Messages in this thread | | | Subject | Re: [RFC PATCH] mm: mempolicy: remove MPOL_MF_LAZY | From | Yang Shi <> | Date | Thu, 21 Mar 2019 16:29:32 -0700 |
| |
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
>
| |