On Tue 09-10-18 10:33:26, Michal Hocko wrote:

On Thu 04-10-18 11:34:11, David Rientjes wrote:

quoted

On Thu, 4 Oct 2018, Michal Hocko wrote:

quoted

quoted

And prior to the offending commit, there were three ways to control thp 
but two ways to determine if a mapping was eligible for thp based on the 
implementation detail of one of those ways.

Yes, it is really unfortunate that we have ever allowed to leak such an
internal stuff like VMA flags to userspace.

Right, I don't like userspace dependencies on VmFlags in smaps myself, but 
it's the only way we have available that shows whether a single mapping is 
eligible to be backed by thp :/

Which is not the case due to reasons mentioned earlier. It only speaks
about madvise status on the VMA.

quoted

quoted

quoted

If there are three ways to 
control thp, userspace is still in the dark wrt which takes precedence 
over the other: we have PR_SET_THP_DISABLE but globally sysfs has it set 
to "always", or we have MADV_HUGEPAGE set per smaps but PR_SET_THP_DISABLE 
shown in /proc/pid/status, etc.

Which one is the ultimate authority?

Isn't our documentation good enough? If not then we should document it
properly.

No, because the offending commit actually changed the precedence itself: 
PR_SET_THP_DISABLE used to be honored for future mappings and the commit 
changed that for all current mappings.

Which is the actual and the full point of the fix as described in the
changelog. The original implementation was poor and inconsistent.

quoted

So as a result of the commit 
itself we would have had to change the documentation and userspace can't 
be expected to keep up with yet a fourth variable: kernel version.  It 
really needs to be simpler, just a per-mapping specifier.

As I've said, if you really need a per-vma granularity then make it a
dedicated line in the output with a clear semantic. Do not make VMA
flags even more confusing.

Can we settle with something please?
-- 
Michal Hocko
SUSE Labs