On Tue, Mar 24, 2020 at 08:37:46AM +0100, Christoph Hellwig wrote:
> On Mon, Mar 23, 2020 at 10:14:55PM -0300, Jason Gunthorpe wrote:
> >     if (pte_none(pte)) {
> >             required_fault = hmm_pte_need_fault(hmm_vma_walk, orig_pfn, 0);
> >             if (required_fault)
> >                     goto fault;
> > +           *pfn = range->values[HMM_PFN_NONE];
> >             return 0;
> >     }
> >  
> > @@ -274,8 +274,10 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, 
> > unsigned long addr,
> >             }
> >  
> >             required_fault = hmm_pte_need_fault(hmm_vma_walk, orig_pfn, 0);
> > -           if (!required_fault)
> > +           if (!required_fault) {
> > +                   *pfn = range->values[HMM_PFN_NONE];
> >                     return 0;
> > +           }
> 
> Maybe throw in a goto hole to consolidaste the set PFN and return
> 0 cases?

Then we have goto fault and goto none both ending in returns. I
generally prefer the goto labels to have a single return

The pte_unmap() before faulting makes this routine twisty and I
haven't thought of a good way to untwist it

Thanks,
Jason
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to