On Fri, Jul 06, 2018 at 04:00:30PM +1000, Michael Ellerman wrote:
> Alexey Kardashevskiy <a...@ozlabs.ru> writes:
> 
> > diff --git a/arch/powerpc/mm/mmu_context_iommu.c 
> > b/arch/powerpc/mm/mmu_context_iommu.c
> > index abb4364..11e1029 100644
> > --- a/arch/powerpc/mm/mmu_context_iommu.c
> > +++ b/arch/powerpc/mm/mmu_context_iommu.c
> > @@ -159,6 +161,7 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long 
> > ua, unsigned long entries,
> >             goto unlock_exit;
> >     }
> >  
> > +   mem->pageshift = __builtin_ctzl(ua | (entries << PAGE_SHIFT));
> 
> __builtin_ctzl(0) is undefined, so are we guaranteed that
> (ua | (entries << PAGE_SHIFT)) is never zero? I couldn't convince
> myself.

We certainly don't need to care about that case, since registering a
zero-length region (entries == 0) is no-op.  But maybe we need to
filter it above.

> > @@ -199,9 +202,17 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long 
> > ua, unsigned long entries,
> >                     }
> >             }
> >  populate:
> > +           pageshift = PAGE_SHIFT;
> > +           if (PageCompound(page))
> > +                   pageshift += compound_order(compound_head(page));
> > +           mem->pageshift = min(mem->pageshift, pageshift);
> >             mem->hpas[i] = page_to_pfn(page) << PAGE_SHIFT;
> >     }
> >  
> > +   /* We have an incomplete huge page, default to PAGE_SHIFT */
> > +   if (head)
> > +           mem->pageshift = PAGE_SHIFT;
> > +
> 
> You never set head AFIACS? (other than in the initialiser)

That looks like a leftover from the previous version.

> 
> cheers
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature

Reply via email to