Re: [patch] mm, thp: fix defrag setting if newline is not used

From: David Rientjes
Date: Fri Jan 17 2020 - 04:44:04 EST


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.

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.