Messages in this thread | | | Subject | Re: [patch] mm, thp: add new background defrag option | From | Vlastimil Babka <> | Date | Fri, 6 Jan 2017 09:41:57 +0100 |
| |
On 01/05/2017 11:54 PM, David Rientjes wrote: > On Thu, 5 Jan 2017, Vlastimil Babka wrote: > >> Hmm that's probably why it's hard to understand, because "madvise >> request" is just setting a vma flag, and the THP allocation (and defrag) >> still happens at fault. >> >> I'm not a fan of either name, so I've tried to implement my own >> suggestion. Turns out it was easier than expected, as there's no kernel >> boot option for "defer", just for "enabled", so that particular worry >> was unfounded. >> >> And personally I think that it's less confusing when one can enable defer >> and madvise together (and not any other combination), than having to dig >> up the difference between "defer" and "background". >> > > I think allowing only two options to be combined amongst four available > solo options is going to be confusing and then even more difficult for the > user to understand what happens when they are combined. Thus, I think
Well, the other options are named "always" and "never", so I wouldn't think so confusing that they can't be combined with anything else. Deciding between "defer" and "background" is however confusing, and also doesn't indicate that the difference is related to madvise.
> these options should only have one settable mode as they have always done. > > The kernel implementation takes less of a priority to userspace > simplicitly, imo, and my patch actually cleans up much of the existing > code and ends up adding fewer lines that yours. I consider it an > improvement in itself. I don't see the benefit of allowing combined > options.
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.
Well, so we'll probably disagree, maybe others can add their opinions.
| |