Re: [PATCH] gpu: drm: v3d: use new return type vm_fault_t

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03-Jul-2018 11:12 PM, "Matthew Wilcox" <willy@xxxxxxxxxxxxx> wrote:
>
> On Tue, Jul 03, 2018 at 09:40:43PM +0530, Souptick Joarder wrote:
> > Use new return type vm_fault_t for fault handler. For
> > now, this is just documenting that the function returns
> > a VM_FAULT value rather than an errno. Once all instances
> > are converted, vm_fault_t will become a distinct type.
> >
> > see commit 1c8f422059ae ("mm: change return type to vm_fault_t")
> > for reference.
> >
> > Previously vm_insert_mixed returns err which driver
> > mapped into VM_FAULT_* type. The new function
> > vmf_insert_mixed will replace this inefficiency by
> > returning VM_FAULT_* type.
>
> I really think this changelog could do with more work.  Specifically for
> this patch, I'd just say:
>
> Convert v3d_gem_fault to return vm_fault_t
>
> Instead of converting an errno into a vm_fault_t ourselves, use
> vmf_insert_mixed() which returns a vm_fault_t directly.
>
> >       unsigned long pfn;
> >       pgoff_t pgoff;
> > -     int ret;
> > 
> >       /* We don't use vmf->pgoff since that has the fake offset: */
> >       pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
> >       pfn = page_to_pfn(bo->pages[pgoff]);
> > 
> > -     ret = vm_insert_mixed(vma, vmf->address, __pfn_to_pfn_t(pfn, PFN_DEV));
> > -
> > -     switch (ret) {
> > -     case -EAGAIN:
> > -     case 0:
> > -     case -ERESTARTSYS:
> > -     case -EINTR:
> > -     case -EBUSY:
> > -             /*
> > -              * EBUSY is ok: this just means that another thread
> > -              * already did the job.
> > -              */
> > -             return VM_FAULT_NOPAGE;
> > -     case -ENOMEM:
> > -             return VM_FAULT_OOM;
> > -     default:
> > -             return VM_FAULT_SIGBUS;
> > -     }
> > +     return vmf_insert_mixed(vma, vmf->address,
> > +             __pfn_to_pfn_t(pfn, PFN_DEV));
> >  }
>
> The line-split here is kind of ugly.  I'd do a little more extensive surgery:
>
> -       unsigned long pfn;
> +       pfn_t pfn;
>
> -       pfn = page_to_pfn(bo->pages[pgoff]);
> +       pfn = __pfn_to_pfn_t(page_to_pfn(bo->pages[pgoff]), PFN_DEV);
>
> +       return vmf_insert_mixed(vma, vmf->address, pfn);

Ok, I will add the same and send v2.

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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux