Re: [PATCH v2 3/4] sched/numa: scan read-only file mappings in tiering mode

[Lorenzo Stoakes (ARM)]

On Fri, Sep 18, 2026 at 11:48:03AM -0400, Gregory Price wrote:
> On Fri, Sep 18, 2026 at 03:53:26PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Fri, Sep 18, 2026 at 03:59:40PM +0200, David Hildenbrand (Arm) wrote:
> > > On 9/18/26 15:57, Gregory Price wrote:
> > > > On Fri, Sep 18, 2026 at 02:58:36PM +0200, David Hildenbrand (Arm) wrote:
> > > >>> +/*
> > > >>> + * Read-only file-backed mappings are expected to be cache replicated between
> > > >>> + * accessor nodes, so they are not worth sampling for placement.  They can
> > > >>> + * still strand on the slow tier like anything else.
> > > >>> + */
> >
> > This is the most specific description ever for such a general condition :)
> >
> > > >>> +static bool vma_is_ro_file(struct vm_area_struct *vma)
> > > >>> +{
> > > >>> +	return vma->vm_file && (vma->vm_flags & (VM_READ | VM_WRITE)) == VM_READ;
> >
> > Firstly you should use the new VMA flags API :)
> >
>
> Please, I beg of you, let us propose clean backportable fixes to handle
> the dumpster fire before we propose setting the entire dump on fire.

Nobody told me it was a hotfix...

>
> I'm not against doing all of this, but this feature is horrendously
> broken and every piece of tiering research that used it since ~6.14
> has just had its data invalidated.

As above...

>
> > But also it seems odd to check VMA_READ_BIT. You can have it cleared but
> > mmap()'ing without PROT_READ but has no material impact on mapping since
> > write implies read for everything afaik (that can have an impact on GUP
> > though).
> >
> > Also note that (well my series changes it hopefully landing for next cycle :)
> > MAP_PRIVATE-/dev/zero which is anon would satisfy this. But anyway :)
> >
> > Anyway in general then I wonder if this shouldn't be vma->vm_file &&
> > !vma_test(vma, VMA_WRITE_BIT), but then it makes me wonder about whether
> > you care if somebody can mprotect() this writable?
> >
> > In which case it'd be vma->vm_file && !vma_test(vma, VMA_MAYWRITE_BIT).
> >
>
> Right, I made no attempt at assessing the correctness the existing vma
> checks - I just moved the existing code to a helper.
>
> I greatly dislike this pattern
>   1) Fix a bug
>   2) While we're here, fix some other subtle hard to explain thing that
>      may or may not change something but certainly is unrelated to the
>      fix and might actually regress something else unexpectedly.
>
> In a single patch.

Well firstly I'm explaining why what you think you are doing isn't
necessarily what you're doing.

Your check as-written includes write-sealed memfd, MAP_PRIVATE file-backed
mappings etc. and you need to figure out if that makes sense or not...

And secondly do not talk about figh... I didn't know it was a hotfix ;)

Anyway, I'd rather you didn't introduce a VMA helper like that here please.

It's not doing what it says it's doing and it might not even be doing what
you think it's doing.

I'd:

	a. figure out whether it matters/you care/etc. about MAP_PRIVATE,
	   write-sealed memfd, etc.

	b. open-code for the hotfix with a comment.

>
> > > >>
> > > >>
> > > >> MAP_PRIVATE can easily map a read-only file with write permissions. So the
> > > >> function name is a bit misleading.
> > > >>
> > > >> This smells like a helper that should go next to other vma helpers and have
> > > >> clear semantics.
> > > >>
> > > >
> > > > No argument here.  Would like to balance improvement vs backportable
> > > > bugfix though.  I broke out the name to try to make it at least a bit
> > > > more readable.
> > >
> > > I understand, but I am not asking about much.
> >
> > It turns out I made it probably too much, or at least too many words :P
> > Sorry.
> >
>
> Can you at least propose a patch on top that adds the cleanup you
> suggest? Much of the VMA stuff is lost on me because I haven't had
> the time to sit down and consume the novel.

What, literally writing the function for you wasn't enough? ;)

I can follow up on it _myself_ if you like + you nag me to (hard to keep
track of things...) good enough? ;)

>
> ~Gregory

--
Cheers, Lorenzo