On Wed 26-09-18 08:12:47, Michal Hocko wrote:
> On Tue 25-09-18 14:50:52, David Rientjes wrote:
> [...]
> Let's put my general disagreement with the approach asside for a while.
> If this is really the best way forward the is the implementation really
> correct?
>
> > + /*
> > + * Disabling thp is possible through both MADV_NOHUGEPAGE and
> > + * PR_SET_THP_DISABLE. Both historically used VM_NOHUGEPAGE. Since
> > + * the introduction of MMF_DISABLE_THP, however, userspace needs the
> > + * ability to detect vmas where thp is not eligible in the same manner.
> > + */
> > + if (vma->vm_mm && test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)) {
> > + flags &= ~VM_HUGEPAGE;
> > + flags |= VM_NOHUGEPAGE;
> > + }
>
> Do we want to report all vmas nh? Shouldn't we limit that to THP-able
> mappings? It seems quite strange that an application started without
> PR_SET_THP_DISABLE wouldn't report nh for most mappings while it would
> otherwise. Also when can we have vma->vm_mm == NULL?
Hmm, after re-reading your documentation update to "A process mapping
may be advised to not be backed by transparent hugepages by either
madvise(MADV_NOHUGEPAGE) or prctl(PR_SET_THP_DISABLE)." the
implementation matches so scratch my comment.
As I've said, I am not happy about this approach but if there is a
general agreement this is really the best we can do I will not stand in
the way.
--
Michal Hocko
SUSE Labs
