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: <3b64a3e0659dbfa2c5f819f40f9f0624309d24ed.camel@mailbox.org>
Date: Thu, 08 May 2025 11:13:39 +0200
From: Philipp Stanner <phasta@...lbox.org>
To: Christian König <christian.koenig@....com>, Philipp
 Stanner <phasta@...nel.org>, Lyude Paul <lyude@...hat.com>, Danilo
 Krummrich <dakr@...nel.org>,  David Airlie <airlied@...il.com>, Simona
 Vetter <simona@...ll.ch>, Sumit Semwal <sumit.semwal@...aro.org>
Cc: dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, linux-media@...r.kernel.org, 
	linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

On Mon, 2025-04-28 at 16:45 +0200, Christian König wrote:
> On 4/24/25 15:02, Philipp Stanner wrote:
> > In nouveau_fence_done(), a fence is checked for being signaled by
> > manually evaluating the base fence's bits. This can be done in a
> > canonical manner through dma_fence_is_signaled().
> > 
> > Replace the bit-check with dma_fence_is_signaled().
> > 
> > Signed-off-by: Philipp Stanner <phasta@...nel.org>
> 
> 
> I think the bit check was used here as fast path optimization because
> we later call dma_fence_is_signaled() anyway.

That fast path optimization effectively saves one JMP instruction to
the function.

I'm increasingly of the opinion that we shall work towards all DRM
users only ever using infrastructure through officially documented API
functions, without touching internal data structures.

> Feel free to add my acked-by, but honestly what nouveau does here
> looks rather suspicious to me.

:)


P.

> 
> Regards,
> Christian.
> 
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > index fb9811938c82..d5654e26d5bc 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
> >  	struct nouveau_channel *chan;
> >  	unsigned long flags;
> >  
> > -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
> > >base.flags))
> > +	if (dma_fence_is_signaled(&fence->base))
> >  		return true;
> >  
> >  	spin_lock_irqsave(&fctx->lock, flags);
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ