Re: [PATCH] drm/i915: reduce stack usage in igt_vma_pin1()

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Jun 24, 2025 at 05:14:25PM -0400, Rodrigo Vivi wrote:
> On Fri, Jun 20, 2025 at 01:36:38PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > The igt_vma_pin1() function has a rather high stack usage, which gets
> > in the way of reducing the default warning limit:
> > 
> > In file included from drivers/gpu/drm/i915/i915_vma.c:2285:
> > drivers/gpu/drm/i915/selftests/i915_vma.c:257:12: error: stack frame size (1288) exceeds limit (1280) in 'igt_vma_pin1' [-Werror,-Wframe-larger-than]
> > 
> > There are two things going on here:
> > 
> >  - The on-stack modes[] array is really large itself and gets constructed
> >    for every call, using around 1000 bytes itself depending on the configuration.
> > 
> >  - The call to i915_vma_pin() gets inlined and adds another 200 bytes for
> >    the i915_gem_ww_ctx structure since commit 7d1c2618eac5 ("drm/i915: Take
> >    reservation lock around i915_vma_pin.")
> > 
> > The second one is easy enough to change, by moving the function into the
> > appropriate .c file. Since it is already large enough to not always be
> > inlined, this seems like a good idea regardless, reducing both the code size
> > and the internal stack usage of each of its 67 callers.
> 
> indeed way to much for the inline.
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Also pushing to drm-intel-next right now...

I also pushed this to drm-intel-gt-next...

> 
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/gpu/drm/i915/i915_vma.c | 20 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_vma.h | 22 ++--------------------
> >  2 files changed, 22 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> > index 632e316f8b05..25e97031d76e 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -1607,6 +1607,26 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> >  	return err;
> >  }
> >  
> > +int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
> > +{
> > +	struct i915_gem_ww_ctx ww;
> > +	int err;
> > +
> > +	i915_gem_ww_ctx_init(&ww, true);
> > +retry:
> > +	err = i915_gem_object_lock(vma->obj, &ww);
> > +	if (!err)
> > +		err = i915_vma_pin_ww(vma, &ww, size, alignment, flags);
> > +	if (err == -EDEADLK) {
> > +		err = i915_gem_ww_ctx_backoff(&ww);
> > +		if (!err)
> > +			goto retry;
> > +	}
> > +	i915_gem_ww_ctx_fini(&ww);
> > +
> > +	return err;
> > +}
> > +
> >  static void flush_idle_contexts(struct intel_gt *gt)
> >  {
> >  	struct intel_engine_cs *engine;
> > diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> > index 6a6be8048aa8..14ccbd0636bb 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.h
> > +++ b/drivers/gpu/drm/i915/i915_vma.h
> > @@ -289,26 +289,8 @@ int __must_check
> >  i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> >  		u64 size, u64 alignment, u64 flags);
> >  
> > -static inline int __must_check
> > -i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
> > -{
> > -	struct i915_gem_ww_ctx ww;
> > -	int err;
> > -
> > -	i915_gem_ww_ctx_init(&ww, true);
> > -retry:
> > -	err = i915_gem_object_lock(vma->obj, &ww);
> > -	if (!err)
> > -		err = i915_vma_pin_ww(vma, &ww, size, alignment, flags);
> > -	if (err == -EDEADLK) {
> > -		err = i915_gem_ww_ctx_backoff(&ww);
> > -		if (!err)
> > -			goto retry;
> > -	}
> > -	i915_gem_ww_ctx_fini(&ww);
> > -
> > -	return err;
> > -}
> > +int __must_check
> > +i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags);
> >  
> >  int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> >  		  u32 align, unsigned int flags);
> > -- 
> > 2.39.5
> >