Re: [PATCH RFC 06/11] dyndbg: make _ddebug::flags its own field

["Thomas Weißschuh" <linux@weissschuh.net>]

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 <linux@weissschuh.net>
> > ---
> >  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;