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

[Lorenzo Stoakes (ARM)]

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 :)

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

Even then things can be weird, as some drivers will clear VMA_MAYWRITE_BIT
for definitely-not-normal-files, though with the intent of disabling
writeability altogether.

For read-only files, as David notes, we do something _weird_:

unsigned long do_mmap(struct file *file, unsigned long addr,
			unsigned long len, unsigned long prot,
			unsigned long flags, vma_flags_t vma_flags,
			unsigned long pgoff, unsigned long *populate,
			struct list_head *uf)
{
	...
	if (file) {
		...
		switch (flags & MAP_TYPE) {
			...
		case MAP_SHARED_VALIDATE:
			...
			if (!(file->f_mode & FMODE_WRITE))
				vma_flags_clear(&vma_flags, VMA_MAYWRITE_BIT,
						VMA_SHARED_BIT);
			...
		}
		...
	}
	...
}

So they become !VMA_SHARED_BIT, !VMA_MAYWRITE_BIT. So it's good you don't
check VMA_SHARED_BIT :)

If you map a read-only file MAP_PRIVATE as readable/writeable they will
actually have VMA_WRITE_BIT, VMA_MAYWRITE_BIT set because the writes CoW
instead.

Anyway, I'm guessing what you want here is:

- Exclude MAP_PRIVATE mappings
- Cannot in any universe write to the damn thing

Which seems like you'd want to test:

In which case the test should be something like:

	return vma_test(vma, VMA_MAYSHARE_BIT) &&
		!vma_test(vma, VMA_MAYWRITE_BIT);

BUT that isn't enough.

Because in actual fact (sigh) some drivers clear VMA_MAYWRITE_BIT (but they
keep VMA_SHARED_BIT) and write-sealing a memfd gives you VMA_SHARED &&
!VMA_MAYWRITE_BIT (which is what vma_is_shared_maywrite() is for for
instance).

So if you _truly_ want to know if something has a _shared_ mapping of a
read-only file It has to be like this:

/**
 * vma_maps_shared_readonly_file() - Is @vma a shared mapping of a read-only
 * file?
 * @vma: The VMA to check.
 *
 * Upon mapping a read-only file with MAP_SHARED[_VALIDATE] mmap() will clear
 * VMA_SHARED_BIT and VMA_MAYWRITE_BIT.
 *
 * The VMA_MAYSHARE_BIT is retained to differentiate against mappings mapped
 * with MAP_PRIVATE.
 *
 * Some drivers clear VMA_MAYWRITE_BIT but by convention retain VMA_SHARED_BIT.
 * This is also true for write-sealed memfd's.
 *
 * Returns: true if the VMA is a shared mapping of a read-only file or false,
 * otherwise.
 */
static inline bool vma_maps_shared_readonly_file(const struct vm_area_struct *vma)
{
	/* Shared mappings of read-only files clear VMA_SHARED_BIT. */
	if (vma_test(vma, VMA_SHARED_BIT))
		return false;

	/* But MAP_SHARED mappings retain VMA_MAYSHARE_BIT. */
	if (!vma_test(vma, VMA_MAYSHARE_BIT))
		return false;

	/* Shared mappings of read-only files also clear VMA_MAYWRITE_BIT. */
	VM_WARN_ON_ONCE(vma_test(vma, VMA_MAYWRITE_BIT));

	return true;
}

The assert here is because nothing else clears VMA_SHARED_BIT like this, but to
protect from future changes that might somehow allow this could be:

	return !vma_test(vma, VMA_MAYWRITE_BIT);

Note that my 40 patch behemoth establishes some actual invariants on this kind
of stuff and introduces some 'VMA checks via semantics' stuff (and eliminates
VM_SPECIAL!) so if adding something like this I'd maybe base it on that.

Obviously if what you need semantically differs from this then do that
instead.

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

>
> Maybe Lorenzo can help us out.
>
> /me summons Lorenzo

Reminds me that I must schlo... write a script to find call-outs in my mail
:P

^^^ see above.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo