Re: [RFC PATCH] mm: mempolicy: remove MPOL_MF_LAZY

[Yang Shi <yang.shi@linux.alibaba.com>]

On 3/21/19 12:24 PM, Mel Gorman wrote:
> On Thu, Mar 21, 2019 at 10:25:08AM -0700, Yang Shi wrote:
>>
>> On 3/21/19 9:51 AM, Michal Hocko wrote:
>>> On Thu 21-03-19 09:21:39, Yang Shi wrote:
>>>> On 3/21/19 7:57 AM, Michal Hocko wrote:
>>>>> On Wed 20-03-19 08:27:39, Yang Shi wrote:
>>>>>> MPOL_MF_LAZY was added by commit b24f53a0bea3 ("mm: mempolicy: Add
>>>>>> MPOL_MF_LAZY"), then it was disabled by commit a720094ded8c ("mm:
>>>>>> mempolicy: Hide MPOL_NOOP and MPOL_MF_LAZY from userspace for now")
>>>>>> right away in 2012.  So, it is never ever exported to userspace.
>>>>>>
>>>>>> And, it looks nobody is interested in revisiting it since it was
>>>>>> disabled 7 years ago.  So, it sounds pointless to still keep it around.
>>>>> The above changelog owes us a lot of explanation about why this is
>>>>> safe and backward compatible. I am also not sure you can change
>>>>> MPOL_MF_INTERNAL because somebody still might use the flag from
>>>>> userspace and we want to guarantee it will have the exact same semantic.
>>>> Since MPOL_MF_LAZY is never exported to userspace (Mel helped to confirm
>>>> this in the other thread), so I'm supposed it should be safe and backward
>>>> compatible to userspace.
>>> You didn't get my point. The flag is exported to the userspace and
>>> nothing in the syscall entry path checks and masks it. So we really have
>>> to preserve the semantic of the flag bit for ever.
>> Thanks, I see you point. Yes, it is exported to userspace in some sense
>> since it is in uapi header. But, it is never documented and MPOL_MF_VALID
>> excludes it. mbind() does check and mask it. It would return -EINVAL if
>> MPOL_MF_LAZY or any other undefined/invalid flag is set. See the below code
>> snippet from do_mbind():
>>
> That does not explain the motivation behind removing it or what we gain.
> Yes, it's undocumented and it's unlikely that anyone will. Any potential
> semantics are almost meaningless with mbind but there are two
> possibilities. One, mbind is relaxed to allow migration within allowed
> nodes and two, interleave could initially interleave but allow migration
> to local node to get a mix of average performance at init and local
> performance over time. No one tried taking that option so far but it
> appears harmless to leave it alone too.

Yes, actually this is what I argued, no one tried taking the flag for 
long time. I also agree it sounds harmless to leave it. I just thought 
it may be dead code, if so why not just remove it.

Thanks,
Yang

>