On Fri, 4 Jun 2021, Yang Shi wrote: > On Thu, Jun 3, 2021 at 7:23 PM Hugh Dickins <hughd@xxxxxxxxxx> wrote: > > On Thu, 3 Jun 2021, Yang Shi wrote: > > > On Tue, Jun 1, 2021 at 2:05 PM Hugh Dickins <hughd@xxxxxxxxxx> 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
