* [PATCH] [media] vivid: don't use more than 1024 bytes of stack
@ 2015-06-05 10:29 Mauro Carvalho Chehab
2015-06-05 10:33 ` Hans Verkuil
2015-06-05 15:10 ` Hans Verkuil
0 siblings, 2 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 10:29 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil
Remove the following compilation warnings:
drivers/media/platform/vivid/vivid-tpg.c: In function 'tpg_gen_text':
drivers/media/platform/vivid/vivid-tpg.c:1562:1: warning: the frame size of 1308 bytes is larger than 1024 bytes [-Wframe-larger-than=]
}
^
This seems to be due to some bad optimization done by gcc.
Moving the for() loop to happen inside the macro solves the
issue.
While here, fix CodingStyle at the switch().
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/platform/vivid/vivid-tpg.c b/drivers/media/platform/vivid/vivid-tpg.c
index b1147f2df26c..7a3ed580626a 100644
--- a/drivers/media/platform/vivid/vivid-tpg.c
+++ b/drivers/media/platform/vivid/vivid-tpg.c
@@ -1492,12 +1492,10 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
else if (tpg->field == V4L2_FIELD_SEQ_TB || tpg->field == V4L2_FIELD_SEQ_BT)
div = 2;
- for (p = 0; p < tpg->planes; p++) {
- unsigned vdiv = tpg->vdownsampling[p];
- unsigned hdiv = tpg->hdownsampling[p];
-
- /* Print text */
-#define PRINTSTR(PIXTYPE) do { \
+ /* Print text */
+#define PRINTSTR(PIXTYPE) for (p = 0; p < tpg->planes; p++) { \
+ unsigned vdiv = tpg->vdownsampling[p]; \
+ unsigned hdiv = tpg->hdownsampling[p]; \
PIXTYPE fg; \
PIXTYPE bg; \
memcpy(&fg, tpg->textfg[p], sizeof(PIXTYPE)); \
@@ -1548,16 +1546,19 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
} \
} while (0)
- switch (tpg->twopixelsize[p]) {
- case 2:
- PRINTSTR(u8); break;
- case 4:
- PRINTSTR(u16); break;
- case 6:
- PRINTSTR(x24); break;
- case 8:
- PRINTSTR(u32); break;
- }
+ switch (tpg->twopixelsize[p]) {
+ case 2:
+ PRINTSTR(u8);
+ break;
+ case 4:
+ PRINTSTR(u16);
+ break;
+ case 6:
+ PRINTSTR(x24);
+ break;
+ case 8:
+ PRINTSTR(u32);
+ break;
}
}
--
2.4.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] [media] vivid: don't use more than 1024 bytes of stack
2015-06-05 10:29 [PATCH] [media] vivid: don't use more than 1024 bytes of stack Mauro Carvalho Chehab
@ 2015-06-05 10:33 ` Hans Verkuil
2015-06-05 10:57 ` Mauro Carvalho Chehab
2015-06-05 15:10 ` Hans Verkuil
1 sibling, 1 reply; 5+ messages in thread
From: Hans Verkuil @ 2015-06-05 10:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
On 06/05/2015 12:29 PM, Mauro Carvalho Chehab wrote:
> Remove the following compilation warnings:
>
> drivers/media/platform/vivid/vivid-tpg.c: In function 'tpg_gen_text':
> drivers/media/platform/vivid/vivid-tpg.c:1562:1: warning: the frame size of 1308 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^
>
> This seems to be due to some bad optimization done by gcc.
>
> Moving the for() loop to happen inside the macro solves the
> issue.
Huh, interesting. I'd never have guessed that.
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks for looking at this!
Hans
>
> While here, fix CodingStyle at the switch().
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/platform/vivid/vivid-tpg.c b/drivers/media/platform/vivid/vivid-tpg.c
> index b1147f2df26c..7a3ed580626a 100644
> --- a/drivers/media/platform/vivid/vivid-tpg.c
> +++ b/drivers/media/platform/vivid/vivid-tpg.c
> @@ -1492,12 +1492,10 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> else if (tpg->field == V4L2_FIELD_SEQ_TB || tpg->field == V4L2_FIELD_SEQ_BT)
> div = 2;
>
> - for (p = 0; p < tpg->planes; p++) {
> - unsigned vdiv = tpg->vdownsampling[p];
> - unsigned hdiv = tpg->hdownsampling[p];
> -
> - /* Print text */
> -#define PRINTSTR(PIXTYPE) do { \
> + /* Print text */
> +#define PRINTSTR(PIXTYPE) for (p = 0; p < tpg->planes; p++) { \
> + unsigned vdiv = tpg->vdownsampling[p]; \
> + unsigned hdiv = tpg->hdownsampling[p]; \
> PIXTYPE fg; \
> PIXTYPE bg; \
> memcpy(&fg, tpg->textfg[p], sizeof(PIXTYPE)); \
> @@ -1548,16 +1546,19 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> } \
> } while (0)
>
> - switch (tpg->twopixelsize[p]) {
> - case 2:
> - PRINTSTR(u8); break;
> - case 4:
> - PRINTSTR(u16); break;
> - case 6:
> - PRINTSTR(x24); break;
> - case 8:
> - PRINTSTR(u32); break;
> - }
> + switch (tpg->twopixelsize[p]) {
> + case 2:
> + PRINTSTR(u8);
> + break;
> + case 4:
> + PRINTSTR(u16);
> + break;
> + case 6:
> + PRINTSTR(x24);
> + break;
> + case 8:
> + PRINTSTR(u32);
> + break;
> }
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] [media] vivid: don't use more than 1024 bytes of stack
2015-06-05 10:33 ` Hans Verkuil
@ 2015-06-05 10:57 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 10:57 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Em Fri, 05 Jun 2015 12:33:04 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> On 06/05/2015 12:29 PM, Mauro Carvalho Chehab wrote:
> > Remove the following compilation warnings:
> >
> > drivers/media/platform/vivid/vivid-tpg.c: In function 'tpg_gen_text':
> > drivers/media/platform/vivid/vivid-tpg.c:1562:1: warning: the frame size of 1308 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > }
> > ^
> >
> > This seems to be due to some bad optimization done by gcc.
> >
> > Moving the for() loop to happen inside the macro solves the
> > issue.
>
> Huh, interesting. I'd never have guessed that.
Yeah, this fix is not too logical ;) It looks that GCC is doing some
very weird optimizations when the loop is outside the macro.
>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Thanks for looking at this!
This was bothering me for a while. I had to do something ;)
>
> Hans
>
> >
> > While here, fix CodingStyle at the switch().
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >
> > diff --git a/drivers/media/platform/vivid/vivid-tpg.c b/drivers/media/platform/vivid/vivid-tpg.c
> > index b1147f2df26c..7a3ed580626a 100644
> > --- a/drivers/media/platform/vivid/vivid-tpg.c
> > +++ b/drivers/media/platform/vivid/vivid-tpg.c
> > @@ -1492,12 +1492,10 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> > else if (tpg->field == V4L2_FIELD_SEQ_TB || tpg->field == V4L2_FIELD_SEQ_BT)
> > div = 2;
> >
> > - for (p = 0; p < tpg->planes; p++) {
> > - unsigned vdiv = tpg->vdownsampling[p];
> > - unsigned hdiv = tpg->hdownsampling[p];
> > -
> > - /* Print text */
> > -#define PRINTSTR(PIXTYPE) do { \
> > + /* Print text */
> > +#define PRINTSTR(PIXTYPE) for (p = 0; p < tpg->planes; p++) { \
> > + unsigned vdiv = tpg->vdownsampling[p]; \
> > + unsigned hdiv = tpg->hdownsampling[p]; \
> > PIXTYPE fg; \
> > PIXTYPE bg; \
> > memcpy(&fg, tpg->textfg[p], sizeof(PIXTYPE)); \
> > @@ -1548,16 +1546,19 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> > } \
> > } while (0)
> >
> > - switch (tpg->twopixelsize[p]) {
> > - case 2:
> > - PRINTSTR(u8); break;
> > - case 4:
> > - PRINTSTR(u16); break;
> > - case 6:
> > - PRINTSTR(x24); break;
> > - case 8:
> > - PRINTSTR(u32); break;
> > - }
> > + switch (tpg->twopixelsize[p]) {
> > + case 2:
> > + PRINTSTR(u8);
> > + break;
> > + case 4:
> > + PRINTSTR(u16);
> > + break;
> > + case 6:
> > + PRINTSTR(x24);
> > + break;
> > + case 8:
> > + PRINTSTR(u32);
> > + break;
> > }
> > }
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [media] vivid: don't use more than 1024 bytes of stack
2015-06-05 10:29 [PATCH] [media] vivid: don't use more than 1024 bytes of stack Mauro Carvalho Chehab
2015-06-05 10:33 ` Hans Verkuil
@ 2015-06-05 15:10 ` Hans Verkuil
2015-06-05 15:41 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 5+ messages in thread
From: Hans Verkuil @ 2015-06-05 15:10 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
On 06/05/2015 12:29 PM, Mauro Carvalho Chehab wrote:
> Remove the following compilation warnings:
>
> drivers/media/platform/vivid/vivid-tpg.c: In function 'tpg_gen_text':
> drivers/media/platform/vivid/vivid-tpg.c:1562:1: warning: the frame size of 1308 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^
>
> This seems to be due to some bad optimization done by gcc.
>
> Moving the for() loop to happen inside the macro solves the
> issue.
>
> While here, fix CodingStyle at the switch().
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/platform/vivid/vivid-tpg.c b/drivers/media/platform/vivid/vivid-tpg.c
> index b1147f2df26c..7a3ed580626a 100644
> --- a/drivers/media/platform/vivid/vivid-tpg.c
> +++ b/drivers/media/platform/vivid/vivid-tpg.c
> @@ -1492,12 +1492,10 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> else if (tpg->field == V4L2_FIELD_SEQ_TB || tpg->field == V4L2_FIELD_SEQ_BT)
> div = 2;
>
> - for (p = 0; p < tpg->planes; p++) {
> - unsigned vdiv = tpg->vdownsampling[p];
> - unsigned hdiv = tpg->hdownsampling[p];
> -
> - /* Print text */
> -#define PRINTSTR(PIXTYPE) do { \
> + /* Print text */
> +#define PRINTSTR(PIXTYPE) for (p = 0; p < tpg->planes; p++) { \
> + unsigned vdiv = tpg->vdownsampling[p]; \
> + unsigned hdiv = tpg->hdownsampling[p]; \
> PIXTYPE fg; \
> PIXTYPE bg; \
> memcpy(&fg, tpg->textfg[p], sizeof(PIXTYPE)); \
> @@ -1548,16 +1546,19 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> } \
> } while (0)
>
> - switch (tpg->twopixelsize[p]) {
> - case 2:
> - PRINTSTR(u8); break;
> - case 4:
> - PRINTSTR(u16); break;
> - case 6:
> - PRINTSTR(x24); break;
> - case 8:
> - PRINTSTR(u32); break;
> - }
> + switch (tpg->twopixelsize[p]) {
This doesn't work I just discovered. Compiling gives this warning:
drivers/media/platform/vivid/vivid-tpg.c: In function ‘tpg_gen_text’:
drivers/media/platform/vivid/vivid-tpg.c:1549:27: warning: ‘p’ may be used uninitialized in this function [-Wmaybe-uninitialized]
switch (tpg->twopixelsize[p]) {
^
And the value for tpg->twopixelsize[p] will actually differ depending on the value of p.
It's probably best to revert this patch.
The correct approach is likely to make four functions, one for case,
with the macro as the function body.
Regards,
Hans
> + case 2:
> + PRINTSTR(u8);
> + break;
> + case 4:
> + PRINTSTR(u16);
> + break;
> + case 6:
> + PRINTSTR(x24);
> + break;
> + case 8:
> + PRINTSTR(u32);
> + break;
> }
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] [media] vivid: don't use more than 1024 bytes of stack
2015-06-05 15:10 ` Hans Verkuil
@ 2015-06-05 15:41 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 15:41 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Em Fri, 05 Jun 2015 17:10:53 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> On 06/05/2015 12:29 PM, Mauro Carvalho Chehab wrote:
> > Remove the following compilation warnings:
> >
> > drivers/media/platform/vivid/vivid-tpg.c: In function 'tpg_gen_text':
> > drivers/media/platform/vivid/vivid-tpg.c:1562:1: warning: the frame size of 1308 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > }
> > ^
> >
> > This seems to be due to some bad optimization done by gcc.
> >
> > Moving the for() loop to happen inside the macro solves the
> > issue.
> >
> > While here, fix CodingStyle at the switch().
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >
> > diff --git a/drivers/media/platform/vivid/vivid-tpg.c b/drivers/media/platform/vivid/vivid-tpg.c
> > index b1147f2df26c..7a3ed580626a 100644
> > --- a/drivers/media/platform/vivid/vivid-tpg.c
> > +++ b/drivers/media/platform/vivid/vivid-tpg.c
> > @@ -1492,12 +1492,10 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> > else if (tpg->field == V4L2_FIELD_SEQ_TB || tpg->field == V4L2_FIELD_SEQ_BT)
> > div = 2;
> >
> > - for (p = 0; p < tpg->planes; p++) {
> > - unsigned vdiv = tpg->vdownsampling[p];
> > - unsigned hdiv = tpg->hdownsampling[p];
> > -
> > - /* Print text */
> > -#define PRINTSTR(PIXTYPE) do { \
> > + /* Print text */
> > +#define PRINTSTR(PIXTYPE) for (p = 0; p < tpg->planes; p++) { \
> > + unsigned vdiv = tpg->vdownsampling[p]; \
> > + unsigned hdiv = tpg->hdownsampling[p]; \
> > PIXTYPE fg; \
> > PIXTYPE bg; \
> > memcpy(&fg, tpg->textfg[p], sizeof(PIXTYPE)); \
> > @@ -1548,16 +1546,19 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2],
> > } \
> > } while (0)
> >
> > - switch (tpg->twopixelsize[p]) {
> > - case 2:
> > - PRINTSTR(u8); break;
> > - case 4:
> > - PRINTSTR(u16); break;
> > - case 6:
> > - PRINTSTR(x24); break;
> > - case 8:
> > - PRINTSTR(u32); break;
> > - }
> > + switch (tpg->twopixelsize[p]) {
>
> This doesn't work I just discovered. Compiling gives this warning:
>
> drivers/media/platform/vivid/vivid-tpg.c: In function ‘tpg_gen_text’:
> drivers/media/platform/vivid/vivid-tpg.c:1549:27: warning: ‘p’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> switch (tpg->twopixelsize[p]) {
> ^
>
Weird. here (gcc 5.1.1) I didn't get any warning.
> And the value for tpg->twopixelsize[p] will actually differ depending on the value of p.
>
> It's probably best to revert this patch.
>
> The correct approach is likely to make four functions, one for case,
> with the macro as the function body.
I tried that (without the for). The result was worse.
I'll double check it latter today.
>
> Regards,
>
> Hans
>
> > + case 2:
> > + PRINTSTR(u8);
> > + break;
> > + case 4:
> > + PRINTSTR(u16);
> > + break;
> > + case 6:
> > + PRINTSTR(x24);
> > + break;
> > + case 8:
> > + PRINTSTR(u32);
> > + break;
> > }
> > }
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-05 15:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-05 10:29 [PATCH] [media] vivid: don't use more than 1024 bytes of stack Mauro Carvalho Chehab
2015-06-05 10:33 ` Hans Verkuil
2015-06-05 10:57 ` Mauro Carvalho Chehab
2015-06-05 15:10 ` Hans Verkuil
2015-06-05 15:41 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox