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)
> The layout change can be avoided by make lineno + class explicitly take 3
> bytes.
> Perhaps reduce lineno to u16 and have class be a standalone u8? No kernel
> source
> file (apart from pre-generated AMDGPU headers, which doesn't use dyndbg
> anyway)
> have more than 65535 lines.
>
> Alternatively we can just say the layout change is fine, as for 64-bit
> platforms
> with jump label patching, there's padding in the struct anyway so this doesn't
> grow the full struct size.
I am fine with other solutions if the dynamic debug folks prefer those.
The goal here was to stay as close as possible to the existing code.
> Anyhow this needs to be noted explicitly.
Ack.
> Best,
> Gary
>
> > #ifdef CONFIG_JUMP_LABEL
> > union {
> > struct static_key_true dd_key_true;
