diff mbox series

media: staging/intel-ipu3: reduce kernel stack usage

Message ID 20190304202758.1802417-1-arnd@arndb.de
State Accepted
Commit c3c2eca87dcded3626bc8f3a1b8bfa4b9f078fb1
Headers show
Series media: staging/intel-ipu3: reduce kernel stack usage | expand

Commit Message

Arnd Bergmann March 4, 2019, 8:27 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 <arnd@arndb.de>

---
 drivers/staging/media/ipu3/ipu3-css.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

-- 
2.20.0

Comments

Arnd Bergmann March 5, 2019, 8:40 a.m. UTC | #1 
On Tue, Mar 5, 2019 at 8:53 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Tue, Mar 05, 2019 at 12:25:18AM +0000, Cao, Bingbu wrote:


> > >     struct v4l2_pix_format_mplane *const in =

> > >                                     &q[IPU3_CSS_QUEUE_IN].fmt.mpix;

> > >     struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@

> > > int imgu_css_fmt_try(struct imgu_css *css,

> > >                                     &q[IPU3_CSS_QUEUE_VF].fmt.mpix;

> > >     int i, s, ret;

> > >

> > > +   if (!q) {

> > > +           ret = -ENOMEM;

> > > +           goto out;

> > > +   }

> > [Cao, Bingbu]

> > The goto here is wrong, you can just report an error, and I prefer it is next to the alloc.

>

> I agree, the goto is just not needed.


Should I remove all the gotos then and do an explicit kfree() in each
error path?

I'd prefer not to mix the two styles, as that can lead to subtle mistakes
when the code is refactored again.

      Arnd
diff mbox series

Patch

diff --git a/drivers/staging/media/ipu3/ipu3-css.c b/drivers/staging/media/ipu3/ipu3-css.c
index 15ab77e4b766..664c14b7a518 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,7 +1745,7 @@  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 imgu_css_queue *q = kcalloc(IPU3_CSS_QUEUES, sizeof(struct imgu_css_queue), GFP_KERNEL);
 	struct v4l2_pix_format_mplane *const in =
 					&q[IPU3_CSS_QUEUE_IN].fmt.mpix;
 	struct v4l2_pix_format_mplane *const out =
@@ -1753,6 +1754,11 @@  int imgu_css_fmt_try(struct imgu_css *css,
 					&q[IPU3_CSS_QUEUE_VF].fmt.mpix;
 	int i, s, ret;
 
+	if (!q) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	/* Adjust all formats, get statistics buffer sizes and formats */
 	for (i = 0; i < IPU3_CSS_QUEUES; i++) {
 		if (fmts[i])
@@ -1766,7 +1772,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 +1795,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 +1837,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 +1852,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 +1869,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,