Re: [PATCH v2 09/13] mm: update all remaining mmap_prepare users to use vma_flags_t
"Liam R. Howlett" <[email protected]> Mon, 9 Feb 2026 19:27:46 +0000
| Newsgroups | gmane.linux.kernel.aio.general,gmane.linux.kernel,gmane.comp.video.dri.devel,gmane.comp.freedesktop.xorg.drivers.intel,gmane.linux.file-systems,gmane.comp.file-systems.ext4,gmane.linux.kernel.mm,gmane.linux.kernel.lsm |
|---|---|
| Message-ID | <o5efi5bwwqapscipxhmahknjurb7rfx6vitddc7c67hgbocr2k@ndkfnucwv4k2> |
* Lorenzo Stoakes <[email protected]> [260206 20:01]: > On Fri, Feb 06, 2026 at 11:31:53AM -0800, Andrew Morton wrote: > > On Fri, 6 Feb 2026 17:46:36 +0000 Pedro Falcato <[email protected]> wrote: > > > > > > -#define VM_REMAP_FLAGS (VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP) > > > > +#define VMA_REMAP_FLAGS mk_vma_flags(VMA_IO_BIT, VMA_PFNMAP_BIT, \ > > > > + VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT) > > > > > > as a sidenote, these flags are no longer constant expressions and thus > > > > > > static vma_flags_t flags = VMA_REMAP_FLAGS; > > I mean this would be a code smell anyway :) but point taken. > > > > > > > can't compile. > > > > Yup, that isn't nice. An all-caps thing with no () is a compile-time > > constant. > > There is precedence for this, e.g. TASK_SIZE_MAX and other arch defines like > that: > > error: initializer element is not a compile-time constant > 3309 | static unsigned long task_max = TASK_SIZE_MAX; > | ^~~~~~~~~~~~~ > > And this will almost certainly (and certainly in everything I tested) become a > compile-time constant via the optimiser so to all intents and purposes it _is_ > essentially compile-time. > > But the point of doing it this way is to maintain, as much as possible, > one-to-one translation between the previous approach and the new with as little > noise/friction as possible. > > Making this a function makes things really horrible honestly. > > Because vma_remap_flags() suddenly because a vague thing - I'd assume this was a > function doing something. So now do we call it get_vma_remap_flags()? Suddenly > something nice-ish like: > > if (vma_flags_test(flags, VMA_REMAP_FLAGS)) { > ... > } > > Become: > > if (vma_flags_test(flags, get_vma_remap_flags())) { > ... > } > > And now it's SUPER ambiguous as to what you're doing there. I'd assume right > away that get_vma_remap_flags() was going off and doing something or referencing > a static variable or something. > > Given the compile will treat the former _exactly_ as if it were a compile-time > constant it's just adding unnecessary ambiguity. > > So is it something we can live with? How many of these are there going to be? Could we do something like: static inline remap_vma_flags_test(..) { return vma_flags_test_all(vma_flags, ...); } if (remap_vma_flags_test(vma_flags)) { ... } -- To unsubscribe, send a message with 'unsubscribe linux-aio' in the body to [email protected]. For more info on Linux AIO, see: http://www.kvack.org/aio/ Don't email: <a href=mailto:"[email protected]">[email protected]</a>