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

[Gregory Price <gourry@gourry.net>]

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.

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.

> 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.

> > >>
> > >>
> > >> 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.

~Gregory