Messages in this thread | | | Subject | Re: [patch] mm, thp: fix defrag setting if newline is not used | From | Vlastimil Babka <> | Date | Fri, 17 Jan 2020 11:12:24 +0100 |
| |
On 1/17/20 10:43 AM, David Rientjes wrote: > On Fri, 17 Jan 2020, Vlastimil Babka wrote: > >>>> If thp defrag setting "defer" is used and a newline is *not* used when >>>> writing to the sysfs file, this is interpreted as the "defer+madvise" >>>> option. >>>> >>>> This is because we do prefix matching and if five characters are written >>>> without a newline, the current code ends up comparing to the first five >>>> bytes of the "defer+madvise" option and using that instead. >>>> >>>> Find the length of what the user is writing and use that to guide our >>>> decision on which string comparison to do. >>> >>> Gee, why is this code so complicated? Can't we just do >>> >>> if (sysfs_streq(buf, "always")) { >>> ... >>> } else if sysfs_streq(buf, "defer+madvise")) { >>> ... >>> } >>> ... >> >> Yeah, if we knew this existed :) >> >> We would lose the prefix matching but hopefully nobody will complain. >> > > I tested Vlastimil's patch and it works as intended so I was about to > modify the changelog and send his patch and ask for a sign-off line > because I think I agree the *partial* prefix matching has ~0.1% chance of > breaking userspace and that 0.1% chance outweighs my desire to make the > code consistent for all options.
If prefix matching worked with "echo alw > /sys..." then I would expect some script out there relies on it, but since it only works with "echo -n alw > /..." then perhaps there's no such script :)
> But if userspace were broken by this, then at least it was already broken > for "defer" depending on newline vs no newline. (What we do know is that > nobody has used "defer" for the past couple years without a newline :). > > If nobody objects, I'll test and send Andrew's version with the changelog > because I think we all agree the risk of breakage here is very minimal and > actually fixes the case for defer.
Agreed.
| |