Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Simona Vetter <simona.vetter@ffwll.ch>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: "Christian König" <christian.koenig@amd.com>,
	intel-xe@lists.freedesktop.org,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Matthew Auld" <matthew.auld@intel.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure
Date: Mon, 9 Mar 2026 10:36:36 +0100	[thread overview]
Message-ID: <aa6UpF-PlTLlb8Qv@phenom.ffwll.local> (raw)
In-Reply-To: <4e9ac4fe88f4b8aec161d4edb4b4f66e70554ec8.camel@linux.intel.com>

On Mon, Mar 02, 2026 at 10:39:10AM +0100, Thomas Hellström wrote:
> On Mon, 2026-03-02 at 10:02 +0100, Christian König wrote:
> > On 2/27/26 17:00, Thomas Hellström wrote:
> > > If the struct ttm_operation_ctx::gfp_retry_mayfail is true,
> > > buffer object backing store allocation failures are expected to
> > > silently fail with an error code to the caller. But currently an
> > > elaborate warning is printed to the system log.
> > > 
> > > Don't spam the log in this way.
> > 
> > That was intentionally removed or never added because Simona
> > absolutely didn't liked the gfp_retry_mayfail flag at that time.
> > 
> > In general I'm fine with this change since I think we have proved by
> > now that the flag is useful, but that probably need more wider
> > discussion.
> 
> Well for system memory it is a bit questionable to be honest, I think
> mostly because even if we return an error, the OOM killer might be
> invoked on an unrelated allocation immediately afterwards.
> 
> Still, even if the use of gfp_retry_mayfail can be discussed, I'm not
> sure why an error here needs to be printed when there are a number of
> other errors that are not printed or printed only on debug.

Yeah adding the NOWARN makes sense irrespective of the bigger question, so
on that:

Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>

For the mayfail I have honestly no recollection anymore of that, but
making a guess I wasn't a fan because way back it was used to hack around
locking inversions between alloc and reclaim paths. Much more with
dev->struct_mutex drivers before moving to dma_resv for non-ttm drivers
too. And that's not great.

Plus GL userspace did not cope with alloc failures, so punting to the OOM
killer like for everything else made sense. And hence there was really no
use for this. But with vk and other low-level gpu apis that changed, we do
want to just pass ENOMEM to userspace now in many conditions.

I think best would be to add a patch to this series to document when
gfp_retry_mayfail can be used (userspace expects the kernel to pass alloc
failures up the stack) and must not be used (hacking around locking
inversions with reclaim) and then ship this. Might also be a good excuse
to switch the kerneldoc for struct ttm_operation_ctx over to the inline
style so we can be appropriately verbose.

But yeah, going through current users (on a Monday morning without coffee)
I think the flag has solid users by now and there's no fundamental
objections from me.

Cheers, Sima

> 
> Thanks,
> Thomas
> 
> 
> > 
> > Regards,
> > Christian.
> > 
> > > 
> > > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> > > b/drivers/gpu/drm/ttm/ttm_pool.c
> > > index c0d95559197c..8fa9e09f6ee5 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_pool.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> > > @@ -726,7 +726,7 @@ static int __ttm_pool_alloc(struct ttm_pool
> > > *pool, struct ttm_tt *tt,
> > >  		gfp_flags |= __GFP_ZERO;
> > >  
> > >  	if (ctx->gfp_retry_mayfail)
> > > -		gfp_flags |= __GFP_RETRY_MAYFAIL;
> > > +		gfp_flags |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> > >  
> > >  	if (ttm_pool_uses_dma32(pool))
> > >  		gfp_flags |= GFP_DMA32;

-- 
Simona Vetter
Software Engineer
http://blog.ffwll.ch

  reply	other threads:[~2026-03-09  9:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-27 16:00 [PATCH 0/2] drm/ttm: Improve the TTM operation context gfp_retry_mayfail behaviour Thomas Hellström
2026-02-27 16:00 ` [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure Thomas Hellström
2026-02-27 21:01   ` Matthew Brost
2026-03-02  9:02   ` Christian König
2026-03-02  9:39     ` Thomas Hellström
2026-03-09  9:36       ` Simona Vetter [this message]
2026-02-27 16:00 ` [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content Thomas Hellström
2026-03-10 14:10   ` Maarten Lankhorst
2026-02-27 17:06 ` ✓ CI.KUnit: success for drm/ttm: Improve the TTM operation context gfp_retry_mayfail behaviour Patchwork
2026-02-27 17:59 ` ✓ Xe.CI.BAT: " Patchwork
2026-02-28  3:48 ` ✗ Xe.CI.FULL: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aa6UpF-PlTLlb8Qv@phenom.ffwll.local \
    --to=simona.vetter@ffwll.ch \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox