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

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:

quoted

On Sun Oct 11, 2026 at 1:52 PM BST, Thomas Weißschuh wrote:

quoted

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.

quoted

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)

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