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