On Wed, 7 Dec 2016 14:51:36 +0100, Sebastian Frias wrote: > On 07/12/16 13:38, Jakub Kicinski wrote: > > On Wed, 7 Dec 2016 12:23:56 +0100, Sebastian Frias wrote: > >> On 07/12/16 12:05, Jakub Kicinski wrote: > >>> On Wed, 7 Dec 2016 11:00:57 +0100, Sebastian Frias wrote: > >>>> On 07/12/16 09:42, Kalle Valo wrote: > >>>>> Sebastian Frias <s...@laposte.net> writes: > >>>>> > >>>>>> Introduce GENVALUE(msb, lsb, value) macro to ease dealing with > >>>>>> continuous bitfields, just as BIT(x) does for single bits. > >>>>>> > >>>>>> GENVALUE_ULL(msb, lsb, value) macro is also added. > >>>>>> > >>>>>> This is useful mostly for creating values to be packed together > >>>>>> via OR operations, ex: > >>>>>> > >>>>>> u32 val = 0x11110000; > >>>>>> val |= GENVALUE(19, 12, 0x5a); > >>>>>> > >>>>>> now 'val = 0x1115a000' > >>>>>> > >>>>>> > >>>>>> Signed-off-by: Sebastian Frias <s...@laposte.net> > >>>>>> Link: https://marc.info/?l=linux-kernel&m=148094498711000&w=2 > >>>>>> --- > >>>>>> > >>>>>> Change in v2: > >>>>>> - rename the macro to GENVALUE as proposed by Linus > >>>>>> - longer comment attempts to show use case for the macro as > >>>>>> proposed by Borislav > >>>>>> > >>>>>> Change in v3: > >>>>>> - use BUILD_BUG_ON_ZERO() to break if some input parameters > >>>>>> (essentially 'lsb' but also 'msb') are not constants as > >>>>>> proposed by Linus. > >>>>>> Indeed, 'lsb' is used twice so it cannot have side-effects; > >>>>>> 'msb' is subjected to same constraints for consistency. > >>>>> > >>>>> (I missed there was v3 already, but I'll repeat what I said in v1.) > >>>>> > >>>>> Please check FIELD_PREP() from include/linux/bitfield.h, doesn't it > >>>>> already do the same? > >>>> > >>>> Indeed, it appears to do the same :-) > >>>> Any reason why "include/linux/bitfield.h" is not included by default in > >>>> bitops.h? > >>> > >>> Hi! > >>> > >>> The code is in a separate header because of circular dependencies > >>> (coming from bug.h including bitops.h, IIRC). You could possibly add an > >>> include of bitfield.h in bitops.h if you're very careful, I haven't > >>> tried TBH :) > >>> > >> > >> Well, the following seems to work just fine: > >> > >> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h > >> index f6505d8..24c7480 100644 > >> --- a/include/linux/bitfield.h > >> +++ b/include/linux/bitfield.h > >> @@ -15,8 +15,6 @@ > >> #ifndef _LINUX_BITFIELD_H > >> #define _LINUX_BITFIELD_H > >> > >> -#include <linux/bug.h> > >> - > > > > That would break users who include bitfield.h directly and don't > > include bug.h, no? > > That's why I was asking if there's a way to verify that I did not break > anything, as it may be simpler to just modify the users. > > Right now bitops.h does not include bug.h yet my patch on this thread > used BUILD_BUG_ON_ZERO() inside bitops.h without issues. > > So it seems like the "proper" include order is already in place. > (even though I agree with you that ideally each header file should include > headers required for it to work properly, regardless of include order) > > Would the following files be the only two users we would need to worry > about? > > $ git grep "linux/bitfield\.h" > drivers/net/ethernet/netronome/nfp/nfp_bpf.h:#include <linux/bitfield.h> > drivers/net/wireless/mediatek/mt7601u/mt7601u.h:#include <linux/bitfield.h>
If you're proposing to require users to have to remember to include bug.h before bitfield.h then I don't think that's acceptable. There are also out-of-tree users like LEDE/OpenWRT who such changes may disturb. BTW I dug out the old conversation: https://lkml.org/lkml/2016/8/19/96 > I can take a look at the underlying include order issue later. I think that would be the only way forward, but is not easy. Is your concern that bitfield.h is hard to discover? Or that users need an extra #include beyond just bitops.h?
