Re: [patch] mm, thp: add new background defrag option

[Vlastimil Babka]

On 01/06/2017 11:20 PM, David Rientjes wrote:
> On Fri, 6 Jan 2017, Vlastimil Babka wrote:
> 
>> Deciding between "defer" and "background" is however confusing, and also
>> doesn't indicate that the difference is related to madvise.
>>
> 
> Any suggestions for a better name for "background" are more than welcome.  

Why not just "madvise+defer"?

>> I don't like bikesheding, but as this is about user-space API, more care
>> should be taken than for implementation details that can change. Even
>> though realistically there will be in 99% of cases only two groups of
>> users setting this
>> - experts like you who know what they are doing, and confusing names
>> won't prevent them from making the right choice
>> - people who will blindly copy/paste from the future cargo-cult websites
>> (if they ever get updated from the enabled="never" recommendations), who
>> likely won't stop and think about the other options.
>>
> 
> I think the far majority will go with a third option: simply use the 
> kernel default and be unaware of other settings or consider it to be the 
> most likely choice solely because it is the kernel default.

Sure, my prediction was only about "users setting this" :) Agreed that
those will be a small minority of all users.

[...]

> So whether it's better to do echo background or echo "madvise defer" is 
> not important to me, I simply imagine that the combination will be more 
> difficult to describe to users.  It would break our userspace to currently 
> tests for "[madvise]" and reports that state as strictly madvise to our 
> mission control, but I can work around that; not sure if others would 
> encounter the same issue (would "[defer madvise]" or "[defer] [madvise]" 
> break fewer userspaces?).

OK, well I'm reluctant to break existing userspace knowingly over such
silliness. Also apparently sysfs files in general should accept only one
value, so I'm not going to push my approach.

> I'd leave it to Andrew to decide whether sysfs files should accept 
> multiple modes or not.  If you are to propose a patch to do so, I'd 
> encourage you to do the same cleanup of triple_flag_store() that I did and 
> make the gfp mask construction more straight-forward.  If you'd like to 
> suggest a different name for "background", I'd be happy to change that if 
> it's more descriptive.

Suggestion is above. I however think your cleanup isn't really needed,
we can simply keep the existing 3 internal flags, and "madvise+defer"
would enable two of them, like in my patch. Nothing says that internally
each option should correspond to exactly one flag.

Vlastimil