On Fri, Feb 12, 2021 at 12:40 PM Peter Xu [off-list ref] wrote:

On Thu, Feb 11, 2021 at 04:19:55PM -0800, Mike Kravetz wrote:

quoted

want_pmd_share() is currently just a check for CONFIG_ARCH_WANT_HUGE_PMD_SHARE.
How about leaving that mostly as is, and adding the new vma checks to
vma_shareable().  vma_shareable() would then be something like:

      if (!(vma->vm_flags & VM_MAYSHARE))
              return false;
#ifdef CONFIG_USERFAULTFD
      if (uffd_disable_huge_pmd_share(vma)
              return false;
#endif
#ifdef /* XXX */
      /* add other checks for things like uffd wp and soft dirty here */
#endif /* XXX */

      if (range_in_vma(vma, base, end)
              return true;
      return false;

Of course, this would require we leave the call to vma_shareable() at the
beginning of huge_pmd_share.  It also means that we are always making a
function call into huge_pmd_share to determine if sharing is possible.
That is not any different than today.  If we do not want to make that extra
function call, then I would suggest putting all that code in want_pmd_share.
It just seems that all the vma checks for sharing should be in one place
if possible.

I don't worry a lot on that since we've already got huge_pte_alloc() which
takes care of huge pmd sharing case, so I don't expect e.g. even most hugetlb
developers to use want_pmd_share() at all, because huge_pte_alloc() will be the
one that frequently got called.

But yeah we can definitely put the check logic into huge_pmd_share() too.
Looking at above code it looks still worth a helper like want_pmd_share() or
with some other name.  Then... instead of making this complicated, how about I
mostly keep this patch but move want_pmd_share() call into huge_pmd_share()
instead?

Btw, Axel, it seems there will still be some respins on the pmd sharing
patches.  Since it turns out it'll be shared by multiple tasks now, do you mind
I pick those out and send them separately?  Then we can consolidate this part
to move on with either the rest of the tasks we've got on hand.

Sounds good to me. :) Thanks Peter + Mike for working on this!

Thanks,

--
Peter Xu