Messages in this thread | | | Date | Thu, 17 Feb 2022 18:22:33 -0800 | From | Ira Weiny <> | Subject | Re: [PATCH V8 16/44] mm/pkeys: Introduce pks_mk_readwrite() |
| |
On Mon, Jan 31, 2022 at 03:10:39PM -0800, Edgecombe, Rick P wrote: > On Thu, 2022-01-27 at 09:54 -0800, ira.weiny@intel.com wrote: > > +void pks_update_protection(int pkey, u32 protection) > > +{ > > I don't know if this matters too much, but the type of a pkey is either > int or u16 across this series and PKU. But it's only possibly a 4 bit > value.
I was settling on 'int' because the PKRU code uses int a lot.
That said, PKRU is a bit more complicated; x86 is 4 bits, powerpc is 5 bits, and I see 4 different types for pkey [int, u16, u32, s16].
The signed values are used to mean 'key or error' in a couple of places. Which leaves 'int' as a convenient choice over 's16' IMO. The use of u32 and u16 seems arbitrary. Both should be plenty big for generic core code.
> Seems the smallest that would fit is char. Why use one over the > other? > > Also, why u32 for protection here? The whole pkrs value containing the > bits for all keys is 32 bits, but per key there is only room ever for 2 > bits, right?
Correct but I'm not sure anything would be saved by declaring u8. Regardless I've changed it.
> > It would be nice to be consistent and have a reason, but again, I don't > know if makes any real difference.
I was consistent in the core code with 'int'. I'll look at cleaning up some of the PKRU code but I think that is a separate series from this one.
For this series I'll standardize on u8 because u16 is also too big. I have seen one place where it would be nice to have a type of unsigned to check the bounds of the pkey. So you have a valid point that following the PKRU code was less than ideal.
Ira
| |