Re: [PATCH v3 2/6] mm/hugetlb: take page table lock in follow_huge_(addr|pmd|pud)()

[Hugh Dickins]

On Mon, 8 Sep 2014, Naoya Horiguchi wrote:
> On Mon, Sep 08, 2014 at 12:13:16AM -0700, Hugh Dickins wrote:
> > On Fri, 5 Sep 2014, Naoya Horiguchi wrote:
> > > On Wed, Sep 03, 2014 at 02:17:41PM -0700, Hugh Dickins wrote:
> > > 
> > > > One subtlety to take care over: it's a long time since I've had to
> > > > worry about pmd folding and pud folding (what happens when you only
> > > > have 2 or 3 levels of page table instead of the full 4): macros get
> > > > defined to each other, and levels get optimized out (perhaps
> > > > differently on different architectures).
> > > > 
> > > > So although at first sight the lock to take in follow_huge_pud()
> > > > would seem to be mm->page_table_lock, I am not at this point certain
> > > > that that's necessarily so - sometimes pud_huge might be pmd_huge,
> > > > and the size PMD_SIZE, and pmd_lockptr appropriate at what appears
> > > > to be the pud level.  Maybe: needs checking through the architectures
> > > > and their configs, not obvious to me.
> > > 
> > > I think that every architecture uses mm->page_table_lock for pud-level
> > > locking at least for now, but that could be changed in the future,
> > > for example when 1GB hugepages or pud-based hugepages become common and
> > > someone are interested in splitting lock for pud level.
> > 
> > I'm not convinced by your answer, that you understand the (perhaps
> > imaginary!) issue I'm referring to.  Try grep for __PAGETABLE_P.D_FOLDED.
> > 
> > Our infrastructure allows for 4 levels of pagetable, pgd pud pmd pte,
> > but many architectures/configurations support only 2 or 3 levels.
> > What pud functions and pmd functions work out to be in those
> > configs is confusing, and varies from architecture to architecture.
> > 
> > In particular, pud and pmd may be different expressions of the same
> > thing (with 1 pmd per pud, instead of say 512).  In that case PUD_SIZE
> > will equal PMD_SIZE: and then at the pud level huge_pte_lockptr()
> > will be using split locking instead of mm->page_table_lock.
> 
> Is it a possible problem? It seems to me that in such system no one
> can create pud-based hugepages and care about pud level locking.

Maybe it is not a possible problem, I already said I'm not certain.
(Maybe I just need to try a couple of x86_32 builds with printks,
to find that it is a real problem; but I haven't tried, and x86_32
would not disprove it for the other architectures.)

But again, your answer does not convince me that you begin to understand
the issue: please read again what I wrote.  I am not talking about
pud-based hugepages, I'm talking about pmd-based hugepages when the
pud level is identical to the pmd level.

Hopefully, you're seeing the issue from a different viewpoint than I am,
and from your good viewpoint the answer is obvious, whereas from my
muddled viewpoint it is not; but you're not making that clear to me.

What is certain is that we do not need to worry about this in a
patch fixing follow_huge_pmd() alone: it only becomes an issue in a
patch extending properly locked FOLL_GET support to follow_huge_pud(),
which I think you've decided to set aside for now.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/