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
