| Message ID | 20200529201534.474853-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/selftests/mm: reduce per-function stack usage | expand |
Quoting Arnd Bergmann (2020-05-29 21:15:26) > The check_reserve_boundaries() function has a large array on the stack, > over 500 bytes. It gets inlined into __igt_reserve, which has multiple > other large structures as well but stayed just under the stack size > warning limit of 1024 bytes until one more member got added to struct > drm_mm_node, causing a warning: > > drivers/gpu/drm/selftests/test-drm_mm.c:371:12: error: > stack frame size of 1032 bytes in function '__igt_reserve' [-Werror,-Wframe-larger-than=] > > As far as I can tell, this is not nice but will not be called from > a context that is already low for the kernel stack, so just annotate > the inner function as noinline_for_stack to ensure that each function > by itself stays under the warning limit. > > Fixes: 0cdea4455acd ("drm/mm: optimize rb_hole_addr rbtree search") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/gpu/drm/selftests/test-drm_mm.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c > index 9aabe82dcd3a..30108c330db8 100644 > --- a/drivers/gpu/drm/selftests/test-drm_mm.c > +++ b/drivers/gpu/drm/selftests/test-drm_mm.c > @@ -323,9 +323,8 @@ static bool expect_reserve_fail(struct drm_mm *mm, struct drm_mm_node *node) > return false; > } > > -static bool check_reserve_boundaries(struct drm_mm *mm, > - unsigned int count, > - u64 size) > +static noinline_for_stack bool > +check_reserve_boundaries(struct drm_mm *mm, unsigned int count, u64 size) > { > const struct boundary { It's this const [] right? Hmm, if we felt adventurous that could be a small set of multiplication factors (0, -1, 1, count, count+1, ...) and made static. -Chris
On Fri, May 29, 2020 at 10:26 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Arnd Bergmann (2020-05-29 21:15:26) > > > > diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c > > index 9aabe82dcd3a..30108c330db8 100644 > > --- a/drivers/gpu/drm/selftests/test-drm_mm.c > > +++ b/drivers/gpu/drm/selftests/test-drm_mm.c > > @@ -323,9 +323,8 @@ static bool expect_reserve_fail(struct drm_mm *mm, struct drm_mm_node *node) > > return false; > > } > > > > -static bool check_reserve_boundaries(struct drm_mm *mm, > > - unsigned int count, > > - u64 size) > > +static noinline_for_stack bool > > +check_reserve_boundaries(struct drm_mm *mm, unsigned int count, u64 size) > > { > > const struct boundary { > > It's this const [] right? Hmm, if we felt adventurous that could be a > small set of multiplication factors (0, -1, 1, count, count+1, ...) and > made static. That was my first thought, but I couldn't figure out whether 'count' could be replaced by any compile-time constant. Arnd
Quoting Arnd Bergmann (2020-05-29 21:43:47) > On Fri, May 29, 2020 at 10:26 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Arnd Bergmann (2020-05-29 21:15:26) > > > > > > > diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c > > > index 9aabe82dcd3a..30108c330db8 100644 > > > --- a/drivers/gpu/drm/selftests/test-drm_mm.c > > > +++ b/drivers/gpu/drm/selftests/test-drm_mm.c > > > @@ -323,9 +323,8 @@ static bool expect_reserve_fail(struct drm_mm *mm, struct drm_mm_node *node) > > > return false; > > > } > > > > > > -static bool check_reserve_boundaries(struct drm_mm *mm, > > > - unsigned int count, > > > - u64 size) > > > +static noinline_for_stack bool > > > +check_reserve_boundaries(struct drm_mm *mm, unsigned int count, u64 size) > > > { > > > const struct boundary { > > > > It's this const [] right? Hmm, if we felt adventurous that could be a > > small set of multiplication factors (0, -1, 1, count, count+1, ...) and > > made static. > > That was my first thought, but I couldn't figure out whether 'count' > could be replaced by any compile-time constant. I just realised I sent a sketch of a patch to the wrong place. If we replace struct boundary with { int start; int size; const char *name; } that should reduce it from 408 to 272. (Where start, size are the multiples.) Probably not worth the hassle, the saving is too small overall leaving it uncomfortably close to a future warning. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c index 9aabe82dcd3a..30108c330db8 100644 --- a/drivers/gpu/drm/selftests/test-drm_mm.c +++ b/drivers/gpu/drm/selftests/test-drm_mm.c @@ -323,9 +323,8 @@ static bool expect_reserve_fail(struct drm_mm *mm, struct drm_mm_node *node) return false; } -static bool check_reserve_boundaries(struct drm_mm *mm, - unsigned int count, - u64 size) +static noinline_for_stack bool +check_reserve_boundaries(struct drm_mm *mm, unsigned int count, u64 size) { const struct boundary { u64 start, size;
The check_reserve_boundaries() function has a large array on the stack, over 500 bytes. It gets inlined into __igt_reserve, which has multiple other large structures as well but stayed just under the stack size warning limit of 1024 bytes until one more member got added to struct drm_mm_node, causing a warning: drivers/gpu/drm/selftests/test-drm_mm.c:371:12: error: stack frame size of 1032 bytes in function '__igt_reserve' [-Werror,-Wframe-larger-than=] As far as I can tell, this is not nice but will not be called from a context that is already low for the kernel stack, so just annotate the inner function as noinline_for_stack to ensure that each function by itself stays under the warning limit. Fixes: 0cdea4455acd ("drm/mm: optimize rb_hole_addr rbtree search") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/selftests/test-drm_mm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)