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 <thellst...@vmware.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.koe...@amd.com>
> > 
> > Signed-off-by: Thomas Hellstrom <thellst...@vmware.com>
> > Reviewed-by: "Christian König" <christian.koe...@amd.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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to