Messages in this thread | | | Date | Fri, 6 Jan 2017 14:20:14 -0800 (PST) | From | David Rientjes <> | Subject | Re: [patch] mm, thp: add new background defrag option |
| |
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.
> > 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. >
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.
I think the kernel default could easily be changed to "background" after this and nobody would actually notice, but I don't have a strong preference for that. I think users who notice large thp_fault_fallback and want to get the true "transparent" nature of hugepages will investigate defragmentation behavior and see "background" is exactly what they want. Indeed, I think that the new "background" mode meshes well with the expectation of "transparent" hugepages. I don't foresee any usecase, present or future, for "defer" so I'll simply ignore it.
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?).
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.
| |