[PATCHv3 04/10] linux/kernel: introduce lower_48_bits macro

Joe Perches joe at perches.com
Tue Feb 22 12:31:30 PST 2022


On Tue, 2022-02-22 at 20:09 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 22 February 2022 18:43
> > 
> > On Tue, 2022-02-22 at 08:56 -0800, Keith Busch wrote:
> > > On Tue, Feb 22, 2022 at 05:50:45PM +0100, Christoph Hellwig wrote:
> > > > On Tue, Feb 22, 2022 at 08:45:53AM -0800, Joe Perches wrote:
> > > > > On Tue, 2022-02-22 at 08:31 -0800, Keith Busch wrote:
> > > > > > +/ *
> > > > > > + * lower_48_bits - return bits 0-47 of a number
> > > > > > + * @n: the number we're accessing
> > > > > > + */
> > > > > > +#define lower_48_bits(n) ((u64)((n) & 0xffffffffffffull))
> > > > > 
> > > > > why not make this a static inline function?
> > > > 
> > > > Agreed.
> > > 
> > > Sure, that sounds good to me. I only did it this way to match the
> > > existing local convention, but I personally prefer the inline function
> > > too.
> > 
> > The existing convention is used there to allow the compiler to
> > avoid warnings and unnecessary conversions of a u32 to a u64 when
> > shifting by 32 or more bits.
> > 
> > If it's possible to be used with an architecture dependent typedef
> > like dma_addr_t, then perhaps it's reasonable to do something like:
> > 
> > #define lower_48_bits(val)					\
> > ({								\
> > 	typeof(val) high = lower_16_bits(upper_32_bits(val));	\
> > 	typeof(val) low = lower_32_bits(val);			\
> > 								\
> > 	(high << 16 << 16) | low;				\
> > })
> > 
> > and have the compiler have the return value be an appropriate type.
> 
> The compiler could make a real pigs breakfast of that.

Both gcc and clang optimize it just fine.

btw: to return the same type the last line should be:

	(typeof(val))((high << 16 << 16) | low);

otherwise the return is sizeof(int) if typeof(val) is not u64

> Oh, did you look for GENMASK([^,]*,[ 0]*) ?

No, why?  I did look for 47, 0 though.

But it's pretty common really.

$ git grep -P 'GENMASK(?:_ULL)?\s*\(\s*\d+\s*,\s*0\s*\)' | wc -l
6233

> I'd only use something GENMASK() for bit ranges.
> Even then it is often easier to just write the value in hex.

Mostly it's the count of the repeated f that's difficult to
quickly verify visually.

> I think the only time I've written anything like that recently
> (last 30 years) was for some hardware registers when the documentation
> user 'bit 1' for the most significant bit.

Luckily the hardware I've had to deal with never did that.
Not even the least significant bit too.





More information about the Linux-nvme mailing list