Re: [PATCH 2/3] linux/bits.h: Add fixed-width GENMASK and BIT macros

Yury Norov
2023-06-22T15:02:38Z

+ Rasmus Villemoes <linux@rasmusvillemoes.dk>

> > -#define __GENMASK(h, l) \
> > - (((~UL(0)) - (UL(1) << (l)) + 1) & \
> > - (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
> > -#define GENMASK(h, l) \
> > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
> > +#define __GENMASK(t, h, l) \
> > + (GENMASK_INPUT_CHECK(h, l) + \
> > + (((t)~0ULL - ((t)(1) << (l)) + 1) & \
> > + ((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)))))
>
> yeah... forcing the use of ull and then casting to the type is simpler
> and does the job. Checked that it does not break the build if h is
> greater than the type and it works
>
> ../include/linux/bits.h:40:20: error: right shift count >= width of type [-Werror=shift-count-overflow]
> 40 | ((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)))))
> | ^~
>
> However this new version does increase the size. Using i915 module
> to test:
>
> $ size build64/drivers/gpu/drm/i915/i915.ko*
> text data bss dec hex filename
> 4355676 213473 7048 4576197 45d3c5 build64/drivers/gpu/drm/i915/i915.ko
> 4361052 213505 7048 4581605 45e8e5 build64/drivers/gpu/drm/i915/i915.ko.new

It sounds weird because all that should anyways boil down at compile
time...

I enabled DRM_I915 in config and ran bloat-o-meter against today's
master, and I don't see that much difference.

$ size vmlinux vmlinux.new
text data bss dec hex filename
44978613 23962202 3026948 71967763 44a2413 vmlinux
44978653 23966298 3026948 71971899 44a343b vmlinux.new
$ scripts/bloat-o-meter vmlinux vmlinux.new
add/remove: 0/0 grow/shrink: 3/2 up/down: 28/-5 (23)
Function old new delta
kvm_mmu_reset_all_pte_masks 623 639 +16
intel_psr_invalidate 1112 1119 +7
intel_drrs_activate 624 629 +5
intel_psr_flush 1410 1409 -1
clk_fractional_divider_general_approximation 207 203 -4
Total: Before=35398799, After=35398822, chg +0.00%

Can you please check your numbers?

Interestingly, the kvm_mmu_reset_all_pte_masks() uses GENMASK_ULL(),
which should generate the same code across versions. Maybe it's just
a noise? Rasmus, can you please take a look?

Thanks,
Yury