[v2] media: staging/intel-ipu3: reduce kernel stack usage

Commit Message

Arnd Bergmann March 5, 2019, 1:26 p.m. UTC

The imgu_css_queue structure is too large to be put on the kernel
stack, as we can see in 32-bit builds:

drivers/staging/media/ipu3/ipu3-css.c: In function 'imgu_css_fmt_try':
drivers/staging/media/ipu3/ipu3-css.c:1863:1: error: the frame size of 1172 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

By dynamically allocating this array, the stack usage goes down to an
acceptable 140 bytes for the same x86-32 configuration.

Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline programming")
Signed-off-by: Arnd Bergmann <[e-mail]>
---
v2: restructure to use 'return -ENOMEM' instead of goto for failed
    allocation.
---
 drivers/staging/media/ipu3/ipu3-css.c | 35 ++++++++++++++++++---------
 1 file changed, 23 insertions(+), 12 deletions(-)

Comments

Sakari Ailus March 5, 2019, 2:17 p.m. UTC | #1

On Tue, Mar 05, 2019 at 02:26:29PM +0100, Arnd Bergmann wrote:
> The imgu_css_queue structure is too large to be put on the kernel
> stack, as we can see in 32-bit builds:
> 
> drivers/staging/media/ipu3/ipu3-css.c: In function 'imgu_css_fmt_try':
> drivers/staging/media/ipu3/ipu3-css.c:1863:1: error: the frame size of 1172 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> 
> By dynamically allocating this array, the stack usage goes down to an
> acceptable 140 bytes for the same x86-32 configuration.
> 
> Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline programming")
> Signed-off-by: Arnd Bergmann <[e-mail]>
> ---
> v2: restructure to use 'return -ENOMEM' instead of goto for failed
>     allocation.

Thanks, Arnd! All three applied.

Patch

diff --git a/drivers/staging/media/ipu3/ipu3-css.c b/drivers/staging/media/ipu3/ipu3-css.c
index 15ab77e4b766..e7f1898874fd 100644
--- a/drivers/staging/media/ipu3/ipu3-css.c
+++ b/drivers/staging/media/ipu3/ipu3-css.c
@@ -3,6 +3,7 @@ 
 
 #include <linux/device.h>
 #include <linux/iopoll.h>
+#include <linux/slab.h>
 
 #include "ipu3-css.h"
 #include "ipu3-css-fw.h"
@@ -1744,15 +1745,18 @@  int imgu_css_fmt_try(struct imgu_css *css,
 	struct v4l2_rect *const bds = &r[IPU3_CSS_RECT_BDS];
 	struct v4l2_rect *const env = &r[IPU3_CSS_RECT_ENVELOPE];
 	struct v4l2_rect *const gdc = &r[IPU3_CSS_RECT_GDC];
-	struct imgu_css_queue q[IPU3_CSS_QUEUES];
-	struct v4l2_pix_format_mplane *const in =
-					&q[IPU3_CSS_QUEUE_IN].fmt.mpix;
-	struct v4l2_pix_format_mplane *const out =
-					&q[IPU3_CSS_QUEUE_OUT].fmt.mpix;
-	struct v4l2_pix_format_mplane *const vf =
-					&q[IPU3_CSS_QUEUE_VF].fmt.mpix;
+	struct imgu_css_queue *q;
+	struct v4l2_pix_format_mplane *in, *out, *vf;
 	int i, s, ret;
 
+	q = kcalloc(IPU3_CSS_QUEUES, sizeof(struct imgu_css_queue), GFP_KERNEL);
+	if (!q)
+		return -ENOMEM;
+
+	in  = &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
+	out = &q[IPU3_CSS_QUEUE_OUT].fmt.mpix;
+	vf  = &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
+
 	/* Adjust all formats, get statistics buffer sizes and formats */
 	for (i = 0; i < IPU3_CSS_QUEUES; i++) {
 		if (fmts[i])
@@ -1766,7 +1770,8 @@  int imgu_css_fmt_try(struct imgu_css *css,
 					IPU3_CSS_QUEUE_TO_FLAGS(i))) {
 			dev_notice(css->dev, "can not initialize queue %s\n",
 				   qnames[i]);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 	}
 	for (i = 0; i < IPU3_CSS_RECTS; i++) {
@@ -1788,7 +1793,8 @@  int imgu_css_fmt_try(struct imgu_css *css,
 	if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_IN]) ||
 	    !imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
 		dev_warn(css->dev, "required queues are disabled\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
@@ -1829,7 +1835,8 @@  int imgu_css_fmt_try(struct imgu_css *css,
 	ret = imgu_css_find_binary(css, pipe, q, r);
 	if (ret < 0) {
 		dev_err(css->dev, "failed to find suitable binary\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 	css->pipes[pipe].bindex = ret;
 
@@ -1843,7 +1850,8 @@  int imgu_css_fmt_try(struct imgu_css *css,
 						IPU3_CSS_QUEUE_TO_FLAGS(i))) {
 				dev_err(css->dev,
 					"final resolution adjustment failed\n");
-				return -EINVAL;
+				ret = -EINVAL;
+				goto out;
 			}
 			*fmts[i] = q[i].fmt.mpix;
 		}
@@ -1859,7 +1867,10 @@  int imgu_css_fmt_try(struct imgu_css *css,
 		 bds->width, bds->height, gdc->width, gdc->height,
 		 out->width, out->height, vf->width, vf->height);
 
-	return 0;
+	ret = 0;
+out:
+	kfree(q);
+	return ret;
 }
 
 int imgu_css_fmt_set(struct imgu_css *css,