Re: [PATCH V8 16/44] mm/pkeys: Introduce pks_mk_readwrite()

[Ira Weiny]

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@xxxxxxxxx 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