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

[Yang Shi]

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

>