Re: [PATCH 1/7] mm/thp: fix __split_huge_pmd_locked() on shmem migration entry

[Hugh Dickins <hughd@google.com>]

On Fri, 4 Jun 2021, Yang Shi wrote:
> On Thu, Jun 3, 2021 at 7:23 PM Hugh Dickins <hughd@google.com> wrote:
> > On Thu, 3 Jun 2021, Yang Shi wrote:
> > > On Tue, Jun 1, 2021 at 2:05 PM Hugh Dickins <hughd@google.com> wrote:
> >
> > The point here (see commit message above) is that some unrelated pmd
> > migration entry could pass the is_huge_zero_pmd() test, which rushes
> > off to use pmd_page() without even checking pmd_present() first.  And
> > most of its users have, one way or another, checked pmd_present() first;
> > but this place and a couple of others had not.
> 
> Thanks for the elaboration. Wondering whether we'd better add some
> comments in the code? Someone may submit a fix patch by visual
> inspection in the future due to missing these points.

I don't really want to add a comment on this, there in zap_huge_pmd():
I think it would be too much of a distraction from that dense code
sequence.  And the comment will be more obvious in the commit message,
once I split these is_huge_zero_pmd() fixes off from
__split_huge_pmd_locked() as Kirill asked.

But... now I think I'll scrap these parts of the patch, and instead
just add a pmd_present() check into is_huge_zero_pmd() itself.
pmd_present() is quick, but pmd_page() may not be: I may convert it
to use a __read_only huge_pmd_pfn, or may not: I'll see how that goes.

Hugh