[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <809f552d-3282-4746-ba49-066d2bd8d44f@lucifer.local>
Date: Mon, 13 Oct 2025 12:04:51 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Ye Liu <ye.liu@...ux.dev>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
David Hildenbrand <david@...hat.com>, Ye Liu <liuye@...inos.cn>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC RFC PATCH] mm: convert VM flags from macros to enum
On Sat, Oct 11, 2025 at 05:30:52PM +0800, Ye Liu wrote:
> From: Ye Liu <liuye@...inos.cn>
>
> Hello MM maintainers and drgn community,
>
> This RFC proposes to convert VM_* flags from #define macros to enum
> vm_flags. The motivation comes from recent drgn development where we
> encountered difficulties in implementing VM flag parsing due to the
> current macro-based approach.
This isn't going to work sorry, it's not valid to have flag values as an enum
(they're distinct) and also - importantly - I'm going to be making significant
changes to VMA flags soon (to allow us to have arbitrary number of VMA flags on
_all_ architectures as recently done with my series doing something similar with
mm flags).
HOWEVER.
My upcoming (likely, multiple) series will result in code using VMA flag _bit
numbers_ in preference to raw VMA flag values.
Therefore, these _could_ be enums.
I will note this down as something to look at when I implement the series (it's
going to take a while, given the very extensive usage of VMA flags throughout
the kernel).
Cheers, Lorenzo
>
> Problem:
> - Debugging tools like drgn must hard-code VM flag values
> - No type information available for external tools
> - Maintenance burden when flags change in kernel
> - Error-prone flag interpretation
>
> Solution:
> Convert to enum while preserving all existing values and semantics.
>
> Benefits:
> 1. Better type safety for kernel code
> 2. Enabled dynamic VM flag parsing in debugging tools
> 3. Improved debugger experience (meaningful symbol names)
> 4. Easier maintenance and extensibility
> 5. No functional changes or ABI impact
>
> This change would particularly help with implementing the 'vm -f' option
> in drgn's crash mode, allowing proper VM flag filtering and display
> without hard-coded values.
>
> I'm seeking feedback on:
> - Overall approach acceptability
> - Any concerns about enum usage in this context
> - Potential impacts on other subsystems
> - Suggestions for alternative solutions
> - Whether this should include trace events update
>
> Looking forward to your comments.
>
> Signed-off-by: Ye Liu <liuye@...inos.cn>
> ---
> include/linux/mm.h | 90 +++++++++++++++++++++++---------------------
> include/linux/mman.h | 7 ++--
> 2 files changed, 50 insertions(+), 47 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index d16b33bacc32..7030108c3759 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -268,62 +268,66 @@ extern struct rw_semaphore nommu_region_sem;
>
> extern unsigned int kobjsize(const void *objp);
> #endif
> -
> /*
> * vm_flags in vm_area_struct, see mm_types.h.
> * When changing, update also include/trace/events/mmflags.h
> */
> -#define VM_NONE 0x00000000
> +enum vm_flags {
> + VM_NONE = 0x00000000,
> +
> + VM_READ = BIT(0), /* currently active flags */
> + VM_WRITE = BIT(1),
> + VM_EXEC = BIT(2),
> + VM_SHARED = BIT(3),
>
> -#define VM_READ 0x00000001 /* currently active flags */
> -#define VM_WRITE 0x00000002
> -#define VM_EXEC 0x00000004
> -#define VM_SHARED 0x00000008
> + /* mprotect() hardcodes VM_MAYREAD >> 4 == VM_READ, and so for r/w/x bits. */
> + VM_MAYREAD = BIT(4), /* limits for mprotect() etc */
> + VM_MAYWRITE = BIT(5),
> + VM_MAYEXEC = BIT(6),
> + VM_MAYSHARE = BIT(7),
>
> -/* mprotect() hardcodes VM_MAYREAD >> 4 == VM_READ, and so for r/w/x bits. */
> -#define VM_MAYREAD 0x00000010 /* limits for mprotect() etc */
> -#define VM_MAYWRITE 0x00000020
> -#define VM_MAYEXEC 0x00000040
> -#define VM_MAYSHARE 0x00000080
> + VM_GROWSDOWN = BIT(8), /* general info on the segment */
>
> -#define VM_GROWSDOWN 0x00000100 /* general info on the segment */
> #ifdef CONFIG_MMU
> -#define VM_UFFD_MISSING 0x00000200 /* missing pages tracking */
> -#else /* CONFIG_MMU */
> -#define VM_MAYOVERLAY 0x00000200 /* nommu: R/O MAP_PRIVATE mapping that might overlay a file mapping */
> -#define VM_UFFD_MISSING 0
> -#endif /* CONFIG_MMU */
> -#define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
> -#define VM_UFFD_WP 0x00001000 /* wrprotect pages tracking */
> -
> -#define VM_LOCKED 0x00002000
> -#define VM_IO 0x00004000 /* Memory mapped I/O or similar */
> -
> - /* Used by sys_madvise() */
> -#define VM_SEQ_READ 0x00008000 /* App will access data sequentially */
> -#define VM_RAND_READ 0x00010000 /* App will not benefit from clustered reads */
> -
> -#define VM_DONTCOPY 0x00020000 /* Do not copy this vma on fork */
> -#define VM_DONTEXPAND 0x00040000 /* Cannot expand with mremap() */
> -#define VM_LOCKONFAULT 0x00080000 /* Lock the pages covered when they are faulted in */
> -#define VM_ACCOUNT 0x00100000 /* Is a VM accounted object */
> -#define VM_NORESERVE 0x00200000 /* should the VM suppress accounting */
> -#define VM_HUGETLB 0x00400000 /* Huge TLB Page VM */
> -#define VM_SYNC 0x00800000 /* Synchronous page faults */
> -#define VM_ARCH_1 0x01000000 /* Architecture-specific flag */
> -#define VM_WIPEONFORK 0x02000000 /* Wipe VMA contents in child. */
> -#define VM_DONTDUMP 0x04000000 /* Do not include in the core dump */
> + VM_UFFD_MISSING = BIT(9), /* missing pages tracking */
> +#else
> + /* nommu: R/O MAP_PRIVATE mapping that might overlay a file mapping */
> + VM_MAYOVERLAY = BIT(9),
> + VM_UFFD_MISSING = 0,
> +#endif
> +
> + VM_PFNMAP = BIT(10), /* Page-ranges managed without "struct page", just pure PFN */
> + VM_UFFD_WP = BIT(12), /* wrprotect pages tracking */
> +
> + VM_LOCKED = BIT(13),
> + VM_IO = BIT(14), /* Memory mapped I/O or similar */
> +
> + /* Used by sys_madvise() */
> + VM_SEQ_READ = BIT(15), /* App will access data sequentially */
> + VM_RAND_READ = BIT(16), /* App will not benefit from clustered reads */
> +
> + VM_DONTCOPY = BIT(17), /* Do not copy this vma on fork */
> + VM_DONTEXPAND = BIT(18), /* Cannot expand with mremap() */
> + VM_LOCKONFAULT = BIT(19), /* Lock the pages covered when they are faulted in */
> + VM_ACCOUNT = BIT(20), /* Is a VM accounted object */
> + VM_NORESERVE = BIT(21), /* should the VM suppress accounting */
> + VM_HUGETLB = BIT(22), /* Huge TLB Page VM */
> + VM_SYNC = BIT(23), /* Synchronous page faults */
> + VM_ARCH_1 = BIT(24), /* Architecture-specific flag */
> + VM_WIPEONFORK = BIT(25), /* Wipe VMA contents in child. */
> + VM_DONTDUMP = BIT(26), /* Do not include in the core dump */
>
> #ifdef CONFIG_MEM_SOFT_DIRTY
> -# define VM_SOFTDIRTY 0x08000000 /* Not soft dirty clean area */
> + VM_SOFTDIRTY = BIT(27), /* Not soft dirty clean area */
> #else
> -# define VM_SOFTDIRTY 0
> + VM_SOFTDIRTY = 0,
> #endif
>
> -#define VM_MIXEDMAP 0x10000000 /* Can contain "struct page" and pure PFN pages */
> -#define VM_HUGEPAGE 0x20000000 /* MADV_HUGEPAGE marked this vma */
> -#define VM_NOHUGEPAGE 0x40000000 /* MADV_NOHUGEPAGE marked this vma */
> -#define VM_MERGEABLE BIT(31) /* KSM may merge identical pages */
> + VM_MIXEDMAP = BIT(28), /* Can contain "struct page" and pure PFN pages */
> + VM_HUGEPAGE = BIT(29), /* MADV_HUGEPAGE marked this vma */
> + VM_NOHUGEPAGE = BIT(30), /* MADV_NOHUGEPAGE marked this vma */
> + VM_MERGEABLE = BIT(31), /* KSM may merge identical pages */
> +};
>
> #ifdef CONFIG_ARCH_USES_HIGH_VMA_FLAGS
> #define VM_HIGH_ARCH_BIT_0 32 /* bit only usable on 64-bit architectures */
> diff --git a/include/linux/mman.h b/include/linux/mman.h
> index 0ba8a7e8b90a..4a0adbdeccdd 100644
> --- a/include/linux/mman.h
> +++ b/include/linux/mman.h
> @@ -130,10 +130,9 @@ static inline bool arch_validate_flags(unsigned long flags)
> * ("bit1" and "bit2" must be single bits)
> */
> #define _calc_vm_trans(x, bit1, bit2) \
> - ((!(bit1) || !(bit2)) ? 0 : \
> - ((bit1) <= (bit2) ? ((x) & (bit1)) * ((bit2) / (bit1)) \
> - : ((x) & (bit1)) / ((bit1) / (bit2))))
> -
> + (((bit1) == 0 || (bit2) == 0) ? 0 : \
> + ((bit1) <= (bit2) ? ((x) & (bit1)) * ((bit2) / (bit1)) \
> + : ((x) & (bit1)) / ((bit1) / (bit2))))
> /*
> * Combine the mmap "prot" argument into "vm_flags" used internally.
> */
> --
> 2.43.0
>
Powered by blists - more mailing lists