On 2/12/21 1:18 PM, Peter Xu wrote: > On Fri, Feb 12, 2021 at 10:11:39AM -0800, Mike Kravetz wrote: >> On 2/10/21 1:21 PM, Axel Rasmussen wrote: >>> From: Peter Xu <[email protected]> >>> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h >>> index b8200782dede..ff50c8528113 100644 >>> --- a/include/linux/mmu_notifier.h >>> +++ b/include/linux/mmu_notifier.h >>> @@ -51,6 +51,7 @@ enum mmu_notifier_event { >>> MMU_NOTIFY_SOFT_DIRTY, >>> MMU_NOTIFY_RELEASE, >>> MMU_NOTIFY_MIGRATE, >>> + MMU_NOTIFY_HUGETLB_UNSHARE, >> >> I don't claim to know much about mmu notifiers. Currently, we use other >> event notifiers such as MMU_NOTIFY_CLEAR. I guess we do 'clear' page table >> entries if we unshare. More than happy to have a MMU_NOTIFY_HUGETLB_UNSHARE >> event, but will consumers of the notifications know what this new event type >> means? And, if we introduce this should we use this other places where >> huge_pmd_unshare is called? > > Yeah AFAICT that is a new feature to mmu notifiers and it's not really used a > lot by consumers yet. Hmm... is there really any consumer at all? I simply > grepped MMU_NOTIFY_UNMAP and see no hook took special care of that. So it's > some extra information that the upper layer would like to deliever to the > notifiers, it's just not vastly used so far. > > So far I didn't worry too much on that either. MMU_NOTIFY_HUGETLB_UNSHARE is > introduced here simply because I tried to make it explicit, then it's easy to > be overwritten one day if we think huge pmd unshare is not worth a standalone > flag but reuse some other common one. But I think at least I owe one > documentation of that new enum. :) > > I'm not extremely willing to touch the rest callers of huge pmd unshare yet, > unless I've a solid reason. E.g., one day maybe one mmu notifier hook would > start to monitor some events, then that's a better place imho to change them. > Otherwise any of my future change could be vague, imho. > > For this patch - how about I simply go back to use MMU_NOTIFIER_CLEAR instead?
I'm good with the new MMU_NOTIFY_HUGETLB_UNSHARE and agree with your reasoning for adding it. I really did not know enough about usage which caused me to question. -- Mike Kravetz
