On Sun Oct 11, 2026 at 3:47 PM BST, Thomas Weißschuh wrote: > On 2026-10-11 14:23:21+0100, Gary Guo wrote: >> On Sun Oct 11, 2026 at 1:52 PM BST, Thomas Weißschuh wrote: >> > Keeping the flags as part of the bitfield prevents the usage of >> > READ_ONCE()/WRITE_ONCE() with it. It also makes access from Rust >> > awkward. >> > >> > Make it a dedicated field. >> > >> > The new field will stay in the exact same location in the parent >> > structure. >> >> Incorrect. >> >> > >> > No functional change. >> > >> > Signed-off-by: Thomas Weißschuh <[email protected]> >> > --- >> > include/linux/dynamic_debug.h | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h >> > index 0b34343df594..b1ff7c0e9a5f 100644 >> > --- a/include/linux/dynamic_debug.h >> > +++ b/include/linux/dynamic_debug.h >> > @@ -51,7 +51,7 @@ struct _ddebug { >> > #else >> > #define _DPRINTK_FLAGS_DEFAULT 0 >> > #endif >> > - u32 flags:8; >> > + u8 flags; >> >> This will change the layout of the struct, because it'll no longer be merged >> with the previous bitfields. > > It seems to work for me: > > struct _ddebug { > const char * modname; /* 0 8 */ > const char * function; /* 8 8 */ > const char * filename; /* 16 8 */ > const char * format; /* 24 8 */ > u32 lineno:18; /* 32: 0 4 */ > u32 class_id:6; /* 32:18 4 */ > > /* Bitfield combined with next fields */ > > u8 flags; /* 35 1 */ > > /* XXX 4 bytes hole, try to pack */ > > union { > struct static_key_true dd_key_true > __attribute__((__aligned__(8))); /* 40 16 */ > struct static_key_false dd_key_false > __attribute__((__aligned__(8))); /* 40 16 */ > } key __attribute__((__aligned__(8))); /* 40 16 */ > > /* size: 56, cachelines: 1, members: 8 */ > /* sum members: 49, holes: 1, sum holes: 4 */ > /* sum bitfield members: 24 bits (3 bytes) */ > /* forced alignments: 1, forced holes: 1, sum forced holes: 4 */ > /* last cacheline: 56 bytes */ > } __attribute__((__aligned__(8))); > > (x86_64, GCC 16.2.1)
I stand corrected. I thought the type change would force allocation unit to not be shared; turns out that this is only behaviour of some compiler (e.g. MSVC) where in GCC this is not the case. C standard says it's an implementation-defined behaviour, and I guess that's why people don't like using bit fields :) Given kernel assumes GCC (or compatible behaviour) I suppose this is alright and you're right that the layout is unchanged in this case. In that case I think using what you have is fine, but this should still be merged with the following patch that adds READ_ONCE/WRITE_ONCE. Best, Gary
