linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: David Hildenbrand <david@redhat.com>
Cc: Usama Arif <usamaarif642@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, hannes@cmpxchg.org, shakeel.butt@linux.dev,
	riel@surriel.com, ziy@nvidia.com, laoar.shao@gmail.com,
	baolin.wang@linux.alibaba.com, Liam.Howlett@oracle.com,
	npache@redhat.com, ryan.roberts@arm.com,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	kernel-team@meta.com
Subject: Re: [PATCH 1/6] prctl: introduce PR_THP_POLICY_DEFAULT_HUGE for the process
Date: Thu, 15 May 2025 21:42:37 +0100	[thread overview]
Message-ID: <5e8c66df-a0d0-4f21-b628-7f2d56f7e595@lucifer.local> (raw)
In-Reply-To: <869607c4-5deb-451e-bd83-fe5890bd54cf@redhat.com>

On Thu, May 15, 2025 at 09:17:28PM +0200, David Hildenbrand wrote:
> On 15.05.25 20:36, Lorenzo Stoakes wrote:

[snip]

> > But I don't think we should use vma_set_thp_policy() in these places, we
> > should just set the flag, to avoid trying to do a write lock etc. etc.,
> > plus we want to set the flag in a place that's not a VMA yet in both cases.
> >
> > So we'd need something like in do_mmap():
> >
> > +	vm_flags |= mm_implied_vma_flags(mm);
> > 	addr = mmap_region(file, addr, len, vm_flags, pgoff, uf);
> >
> > Where mm_implied_vma_flags() reads the MMF flags and sees if any imply VMA
> > flags.
> >
> > But we have something for that already don't we? mm->def_flags.
> >
> > Can't we use that actually? That should work for mmap too?
>
> Yeah, look at
>
> commit 1860033237d4be09c5d7382585f0c7229367a534
> Author: Michal Hocko <mhocko@suse.com>
> Date:   Mon Jul 10 15:48:02 2017 -0700
>
>     mm: make PR_SET_THP_DISABLE immediately active
>
>
> Where we moved away from that. As raised, I am not sure I like what we did
> with PR_SET_THP_DISABLE.
>
> And I don't want any new magical prctl like that that add new magical
> internal toggles.
>
> OTOH, I am completely fine with a prctl that just changes the default for
> new VMAs (just like applying madvise imemdiately afterwards). I'm also fine
> with a prtctl that changes all existing VMAs, but maybe just issuing a
> madvise() is the better solution, to cleanly separate it.
>
> All not too crazy and not too invasive -- piggybagging on VM_HUGEPAGE /
> VM_NOHUGEPAGE.
>
> I can life with that if it solves a use case.

I guess you're not suggesting using an MMF_ in this way which overrides
VMAs, I think the main reason Michael wanted to not use mm->def_flags here
is because doing so doesn't immediately change existing VMAs.

But we're doing things at the VMA level anyway, so we could just set:

1. mm->def_flags accordingly (no new MMF flag needed!)
2. update existing VMAs using (possibly improved) madvise() interface

And all should work.

The get policy stuff could then just check mm->def_flags & VM_HUGEPAGE or
VM_NOHUGEPAGE and use this as state.

This might be the least egregious way of doing this...

Maybe then I could hold my nose and possibly live with truly the most Evil
Interface in the History of Computing (TM), prctl() ;)

>
> --
> Cheers,
>
> David / dhildenb
>

Cheers, Lorenzo


  reply	other threads:[~2025-05-15 20:42 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-15 13:33 [PATCH 0/6] prctl: introduce PR_SET/GET_THP_POLICY Usama Arif
2025-05-15 13:33 ` [PATCH 1/6] prctl: introduce PR_THP_POLICY_DEFAULT_HUGE for the process Usama Arif
2025-05-15 14:40   ` Lorenzo Stoakes
2025-05-15 14:44     ` David Hildenbrand
2025-05-15 14:56       ` Usama Arif
2025-05-15 14:58         ` David Hildenbrand
2025-05-15 15:18           ` Lorenzo Stoakes
2025-05-15 15:45       ` Liam R. Howlett
2025-05-15 15:57         ` David Hildenbrand
2025-05-15 16:38           ` Lorenzo Stoakes
2025-05-15 17:29             ` David Hildenbrand
2025-05-15 18:09               ` Liam R. Howlett
2025-05-15 18:21                 ` Lorenzo Stoakes
2025-05-15 18:42                   ` Zi Yan
2025-05-15 21:04                     ` Lorenzo Stoakes
2025-05-15 18:46                   ` Usama Arif
2025-05-15 19:20                 ` David Hildenbrand
2025-05-15 15:28     ` Usama Arif
2025-05-15 16:06       ` Lorenzo Stoakes
2025-05-15 16:11         ` David Hildenbrand
2025-05-15 18:08           ` Lorenzo Stoakes
2025-05-15 19:12             ` David Hildenbrand
2025-05-15 20:35               ` Lorenzo Stoakes
2025-05-16  7:45                 ` David Hildenbrand
2025-05-16 10:57                   ` Lorenzo Stoakes
2025-05-16 11:24                     ` David Hildenbrand
2025-05-16 12:57                       ` Lorenzo Stoakes
2025-05-16 17:19                         ` Usama Arif
2025-05-16 17:51                           ` Lorenzo Stoakes
2025-05-16 19:34                             ` Usama Arif
2025-05-17 16:20                         ` Is number of process_madvise()-able ranges limited to 8? (was Re: [PATCH 1/6] prctl: introduce PR_THP_POLICY_DEFAULT_HUGE for the process) SeongJae Park
2025-05-17 18:50                           ` Lorenzo Stoakes
2025-05-17 20:25                             ` SeongJae Park
2025-05-17 19:01                         ` [PATCH 1/6] prctl: introduce PR_THP_POLICY_DEFAULT_HUGE for the process Lorenzo Stoakes
2025-05-15 16:47         ` Usama Arif
2025-05-15 18:36           ` Lorenzo Stoakes
2025-05-15 19:17             ` David Hildenbrand
2025-05-15 20:42               ` Lorenzo Stoakes [this message]
2025-05-16  6:12   ` kernel test robot
2025-05-15 13:33 ` [PATCH 2/6] prctl: introduce PR_THP_POLICY_DEFAULT_NOHUGE " Usama Arif
2025-05-16  8:19   ` kernel test robot
2025-05-15 13:33 ` [PATCH 3/6] prctl: introduce PR_THP_POLICY_SYSTEM " Usama Arif
2025-05-15 13:33 ` [PATCH 4/6] selftests: prctl: introduce tests for PR_THP_POLICY_DEFAULT_NOHUGE Usama Arif
2025-05-15 13:33 ` [PATCH 5/6] selftests: prctl: introduce tests for PR_THP_POLICY_DEFAULT_HUGE Usama Arif
2025-05-15 13:33 ` [PATCH 6/6] docs: transhuge: document process level THP controls Usama Arif
2025-05-15 13:55 ` [PATCH 0/6] prctl: introduce PR_SET/GET_THP_POLICY Lorenzo Stoakes
2025-05-15 14:50   ` Usama Arif
2025-05-15 15:15     ` Lorenzo Stoakes
2025-05-15 15:54       ` Usama Arif
2025-05-15 16:04         ` David Hildenbrand
2025-05-15 16:24         ` Lorenzo Stoakes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5e8c66df-a0d0-4f21-b628-7f2d56f7e595@lucifer.local \
    --to=lorenzo.stoakes@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@meta.com \
    --cc=laoar.shao@gmail.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npache@redhat.com \
    --cc=riel@surriel.com \
    --cc=ryan.roberts@arm.com \
    --cc=shakeel.butt@linux.dev \
    --cc=usamaarif642@gmail.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox