On Tue, Aug 12, 2025 at 01:13:26PM -0700, SeongJae Park wrote:
> On Tue, 12 Aug 2025 16:44:09 +0100 Lorenzo Stoakes
> <lorenzo.stoa...@oracle.com> wrote:
>
> > We are currently in the bizarre situation where we are constrained on the
> > number of flags we can set in an mm_struct based on whether this is a
> > 32-bit or 64-bit kernel.
> >
> > This is because mm->flags is an unsigned long field, which is 32-bits on a
> > 32-bit system and 64-bits on a 64-bit system.
> >
> > In order to keep things functional across both architectures, we do not
> > permit mm flag bits to be set above flag 31 (i.e. the 32nd bit).
> >
> > This is a silly situation, especially given how profligate we are in
> > storing metadata in mm_struct, so let's convert mm->flags into a bitmap and
> > allow ourselves as many bits as we like.
>
> I like this conversion.
Thanks!
>
> [...]
> >
> > In order to execute this change, we introduce a new opaque type -
> > mm_flags_t - which wraps a bitmap.
>
> I have no strong opinion here, but I think coding-style.rst[1] has one? To
> quote,
>
> Please don't use things like ``vps_t``.
> It's a **mistake** to use typedef for structures and pointers.
You stopped reading the relevant section in [1] :) Keep going and you see:
Lots of people think that typedefs help readability. Not so. They
are useful only for: totally opaque objects (where the typedef is
actively used to hide what the object is). Example: pte_t
etc. opaque objects that you can only access using the proper
accessor functions.
So this is what this is.
The point is that it's opaque, that is you aren't supposed to know about or
care about what's inside, you use the accessors.
This means we can extend the size of this thing as we like, and can enforce
atomicity through the accessors.
We further highlight the opaqueness through the use of the __private.
>
> checkpatch.pl also complains similarly.
>
> Again, I have no strong opinion, but I think adding a clarification about why
> we use typedef despite of the documented recommendation here might be nice?
I already gave one, I clearly indicate it's opaque.
>
> [...]
> > For mm->flags initialisation on fork, we adjust the logic to ensure all
> > bits are cleared correctly, and then adjust the existing intialisation
>
> Nit. s/intialisation/initialisation/ ?
Ack thanks!
>
> [...]
>
> [1] https://docs.kernel.org/process/coding-style.html#typedefs
>
>
> Thanks,
> SJ
