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.
