linux-kernel - Re: [PATCH] drm/i915: reduce stack usage in igt_vma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aFsVMcb79rp8kMpF@intel.com>
Date: Tue, 24 Jun 2025 17:14:25 -0400
From: Rodrigo Vivi <rodrigo.vivi@...el.com>
To: Arnd Bergmann <arnd@...nel.org>
CC: Jani Nikula <jani.nikula@...ux.intel.com>, Joonas Lahtinen
	<joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin <tursulin@...ulin.net>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, "Arnd
 Bergmann" <arnd@...db.de>, Nitin Gote <nitin.r.gote@...el.com>, "Chaitanya
 Kumar Borah" <chaitanya.kumar.borah@...el.com>, Krzysztof Niemiec
	<krzysztof.niemiec@...el.com>, Ville Syrjälä
	<ville.syrjala@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>,
	renjun wang <renjunw0@...mail.com>, <intel-gfx@...ts.freedesktop.org>,
	<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/i915: reduce stack usage in igt_vma_pin1()

On Fri, Jun 20, 2025 at 01:36:38PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.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@...el.com>

Also pushing to drm-intel-next right now...

> 
> Signed-off-by: Arnd Bergmann <arnd@...db.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
>