+ Anshuman Khandual, Catalin Marinas, linux-arm-ker...@lists.infradead.org This series moves GENMASK_U128 out of uapi. ARM is the only proposed user. Add ARM people for visibility.
Thanks, Yury On Sat, Mar 22, 2025 at 07:39:35PM +0900, Vincent Mailhol via B4 Relay wrote: > This is a subset of below series: > > bits: Fixed-type GENMASK_U*() and BIT_U*() > Link: > https://lore.kernel.org/r/20250308-fixed-type-genmasks-v6-0-f59315e73...@wanadoo.fr > > Yury suggested to split the above series in two steps: > > #1 Introduce the new fixed type GENMASK_U*() > #2 Consolidate the existing GENMASK*() > > This new series is the resulting step #2 following the split. > > And thus, this series consolidate all the non-asm GENMASK*() so that > they now all depend on GENMASK_TYPE() which was introduced in step #1. > > To do so, I had to split the definition of the asm and non-asm > GENMASK(). I think this is controversial. So I initially implemented a > first draft in which both the asm and non-asm version would rely on > the same helper macro, i.e. adding this: > > #define __GENMASK_TYPE(t, w, h, l) \ > (((t)~_ULL(0) << (l)) & \ > ((t)~_ULL(0) >> (w - 1 - (h)))) > > to uapi/bits.h. And then, the different GENMASK()s would look like > this: > > #define __GENMASK(h, l) __GENMASK_TYPE(unsigned long, __BITS_PER_LONG, h, l) > > and so on. > > I implemented it, and the final result looked quite ugly. Not only do > we need to manually provide the width each time, the biggest concern > is that adding this to the uapi is asking for trouble. Who knows how > people are going to use this? And once it is in the uapi, there is > virtually no way back. > > Adding to this, that macro can not even be generalized to u128 > integers, whereas after the split, it can. > > And so, after implementing both, the asm and non-asm split seems way > more clean and I think this is the best compromise. > > Aside from the split, the asm's GENMASK() and GENMASK_ULL() are left > untouched. While there are some strong incentives to also simplify > these as pointed by David Laight in this thread: > > https://lore.kernel.org/all/20250309102312.4ff08576@pumpkin/ > > this series deliberately limit its scope to the non-asm variants. > > Here are the bloat-o-meter stats: > > $ ./scripts/bloat-o-meter vmlinux_before.o vmlinux_after.o > add/remove: 0/0 grow/shrink: 4/2 up/down: 5/-4 (1) > Function old new delta > intel_psr_invalidate 666 668 +2 > mst_stream_compute_config 1652 1653 +1 > intel_psr_flush 977 978 +1 > intel_dp_compute_link_config 1327 1328 +1 > cfg80211_inform_bss_data 5109 5108 -1 > intel_drrs_activate 379 376 -3 > Total: Before=22723481, After=22723482, chg +0.00% > > (done with GCC 12.4.1 on an x86_64 defconfig) > > -- > 2.43.0 > > Signed-off-by: Vincent Mailhol <mailhol.vinc...@wanadoo.fr> > --- > Vincent Mailhol (3): > bits: split the definition of the asm and non-asm GENMASK*() > bits: unify the non-asm GENMASK*() > test_bits: add tests for __GENMASK() and __GENMASK_ULL() > > include/linux/bits.h | 29 ++++++----------------------- > lib/test_bits.c | 19 +++++++++++++++++++ > 2 files changed, 25 insertions(+), 23 deletions(-) > --- > base-commit: e3f42c436d7e0cb432935fe3ae275dd8d9b60f71 > change-id: 20250320-consolidate-genmask-6cd02abadf82 > prerequisite-change-id: 20250228-fixed-type-genmasks-8d1a555f34e8:v7 > prerequisite-patch-id: 572c05165229640db7dd8fe4d53e1a33ee5dd586 > prerequisite-patch-id: c16d122a487f83e2866a9a669259db097ef46a70 > prerequisite-patch-id: 35f115c0f1b327f1516cfc38b3076e07713df6cd > prerequisite-patch-id: 5fe7058f6ea73b37df75d5c39ad69a4da928058d > prerequisite-patch-id: 82fb628d052ce9f1efac7f3b61eafb2749f95847 > > Best regards, > -- > Vincent Mailhol <mailhol.vinc...@wanadoo.fr> >
