linux-kernel - Re: [PATCH v5 5/9] drm/ttm: TTM fault handler helpers

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>] [day] [month] [year] [list]

Date:   Thu, 13 Jun 2019 11:49:54 +0000
From:   Thomas Hellstrom <thellstrom@...are.com>
To:     "hdanton@...a.com" <hdanton@...a.com>
CC:     "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Linux-graphics-maintainer <Linux-graphics-maintainer@...are.com>,
        Pv-drivers <Pv-drivers@...are.com>,
        "christian.koenig@....com" <christian.koenig@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>
Subject: Re: [PATCH v5 5/9] drm/ttm: TTM fault handler helpers

Hi!

On Thu, 2019-06-13 at 12:25 +0800, Hillf Danton wrote:
> Hello Thomas
> 
> On Wed, 12 Jun 2019 08:42:39 +0200 Thomas Hellstrom wrote:
> > From: Thomas Hellstrom <thellstrom@...are.com>
> > 
> > With the vmwgfx dirty tracking, the default TTM fault handler is
> > not
> > completely sufficient (vmwgfx need to modify the vma->vm_flags
> > member,
> > and also needs to restrict the number of prefaults).
> > 
> > We also want to replicate the new ttm_bo_vm_reserve() functionality
> > 
> > So start turning the TTM vm code into helpers:
> > ttm_bo_vm_fault_reserved()
> > and ttm_bo_vm_reserve(), and provide a default TTM fault handler
> > for other
> > drivers to use.
> > 
> > Cc: "Christian König" <christian.koenig@....com>
> > 
> > Signed-off-by: Thomas Hellstrom <thellstrom@...are.com>
> > Reviewed-by: "Christian König" <christian.koenig@....com> #v1
> > ---
> >  drivers/gpu/drm/ttm/ttm_bo_vm.c | 175 +++++++++++++++++++---------
> > ----
> >  include/drm/ttm/ttm_bo_api.h    |  10 ++
> >  2 files changed, 113 insertions(+), 72 deletions(-)
> > 
> > 

...


> > -	/*
> > -	 * Work around locking order reversal in fault / nopfn
> > -	 * between mmap_sem and bo_reserve: Perform a trylock operation
> > -	 * for reserve, and if it fails, retry the fault after waiting
> > -	 * for the buffer to become unreserved.
> > -	 */
> Is it likely to not cut the comment as the trylock is still there?

Yes, I'll re-add that. It was removed in an early version of the patch
when I actually removed the trylock as well, but I changed my mind on
that.

> 
> >  	if (unlikely(!reservation_object_trylock(bo->resv))) {
> >  		if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
> >  			if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
> > @@ -151,14 +148,55 @@ static vm_fault_t ttm_bo_vm_fault(struct
> > vm_fault *vmf)
> >  		return VM_FAULT_NOPAGE;
> >  	}
> >  
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(ttm_bo_vm_reserve);

...


> > 
> > -	if (unlikely(err != 0)) {
> > -		ret = VM_FAULT_SIGBUS;
> > -		goto out_io_unlock;
> > -	}
> > +	if (unlikely(err != 0))
> > +		return VM_FAULT_SIGBUS;
> >  
> Is it likely a typo to skip the io_unlock?
> 
> --
> Hillf


Yes. Good catch. That io_unlock should definitely remain.

I'll respin and resend to dri-devel and lkml only.

Thanks,
Thomas