On Wed, Mar 05, 2025 at 03:59:28PM +0000, Matthew Wilcox wrote:
> On Wed, Mar 05, 2025 at 02:55:06PM +0000, Lorenzo Stoakes wrote:
> > So adding additional fields is generally unviable, and VMA flags are
> > equally as contended, and prevent VMA merge, further impacting overhead.
> >
> > We can however make use of the time-honoured kernel tradition of grabbing
> > bits where we can.
> >
> > Since we can rely upon anon_vma allocations being at least system
> > word-aligned, we have a handful of bits in the vma->anon_vma available to
> > use as flags.
>
> I'm not a huge fan when there's a much better solution. It's an
> unsigned long, but we can only use the first 32 bits because of 32-bit
> compatibility? This is a noose we've made for our own neck.
Sure, as discussed off-list this is something I'm going to look at, it's
not either/or at all :)
I have in the back of my mind _other uses_ for these flags which isn't
stated here (perhaps should be), perhaps we can have other kinds of data
type we reference here.
But perhaps it's a bit early for that...
Other than that was a morning's work to see if this _could_ work as a
_immediate_ 'what can we do to address this issue?' solution.
The vm_flags solution is viable, as we can modify merging behaviour for a
'sticky' VMA flag that doesn't impact merging, i.e. mask out for merge
compatibility testing, but ensure propagated on split/merge (same behaviour
as what the anon_vma flags achieve).
>
> (there are many more places to fix up; this is illustrative):
>
> diff --git a/include/linux/hugetlb_inline.h b/include/linux/hugetlb_inline.h
> index 0660a03d37d9..c6ea81ff4afe 100644
> --- a/include/linux/hugetlb_inline.h
> +++ b/include/linux/hugetlb_inline.h
> @@ -8,7 +8,7 @@
>
> static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
> {
> - return !!(vma->vm_flags & VM_HUGETLB);
> + return test_bit(VM_HUGETLB, vma->vm_flags);
> }
>
> #else
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 0ca9feec67b8..763210ba70b6 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -571,7 +571,8 @@ static inline void *folio_get_private(struct folio *folio)
> return folio->private;
> }
>
> -typedef unsigned long vm_flags_t;
> +#define VM_FLAGS_COUNT (8 / sizeof(unsigned long))
> +typedef unsigned long vm_flags_t[VM_FLAGS_COUNT];
>
> /*
> * A region containing a mapping of a non-memory backed file under NOMMU
Ohhhh does test_bit() automagically figure things out for sizeof(addr)
including on 32-bit?
Maybe I can quickly slap together something for that quicker than I thought
then? I was _very concerned_ about tearing and that being a total PITA
hence deferring a bit.
But... if you're saying I can churn this to death and it'll 'just work'
then say no more... ;)
Let's put this on hold then until/when I have actual reasons to have
different anon_vma types and I'll look at the 'make vm_flags 64-bit
everywhere' thing instead.
