Re: [PATCH v3 1/5] mm/mseal: always define VM_SEALED

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, Jul 24, 2025 at 11:34:31AM -0700, Jeff Xu wrote:
> Hi Lorenzo,
>
> On Wed, Jul 16, 2025 at 10:38 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > There is no reason to treat VM_SEALED in a special way, in each other case
> > in which a VMA flag is unavailable due to configuration, we simply assign
> > that flag to VM_NONE, so make VM_SEALED consistent with all other VMA
> > flags in this respect.
> >
> Originally, I wanted to avoid using VM_NONE for VM_SEALED to catch
> compiler errors in 32-bit builds. This would serve as a safeguard
> against inadvertently using VM_SEALED in code paths shared between
> 32-bit and 64-bit architectures.

I understand why you did it, and I fundamentally disagree.

This would make this the only VMA flag in the entire kernel having this
special treatment for something with very little implementation code, it
simply makes no sense.

The commit message makes this clear.

>
> Take an example of show_smap_vma_flags [1] :
>
> #ifdef CONFIG_64BIT
> [ilog2(VM_SEALED)] = "sl",
> #endif
>
> If a developer forgets to add #ifdef CONFIG_64BIT, the build will fail

This is a really silly thing to worry about.

> for 32-bit systems. With VM_SEALED redefined as VM_NONE, the problem
> will go unnoticed.

What problem exactly?

>
> This coding practice is more defensive and helps catch errors early

It's silly.

> on. It seems that you're working on introducing VM_SEALED for 32-bit
> systems. If that happens, we won't need this safeguard anymore. But
> until then, is it OK to keep it in place?  That said, if VM_SEALED
> support for 32-bit is coming really soon, we can merge this change,
> however, perhaps you could update the description to explain why we're
> removing this safeguard, i.e. due to 32-bit support will soon be in
> place.

No I won't, because that's not why I did it.