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 <peterx@xxxxxxxxxx>
>>> 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
