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

From: David Rientjes
Date: Fri Jan 06 2017 - 17:21:12 EST


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.