Re: [PATCH 02/50] make the i_state flags an enum

[Alan Huang <mmpgouride@gmail.com>]

On Aug 22, 2025, at 19:18, Sun YangKai <sunk67188@gmail.com> wrote:
> 
> Hi Josef,
> 
> Sorry for the bothering, and I hope this isn't too far off-topic for the 
> current patch series discussion.
> 
> I recently learned about the x-macro trick and was wondering if it might be 
> suitable for use in this context since we are rewriting this. I'd appreciate 
> any thoughts or feedback on whether this approach could be applied here.

I think x-macro is easy to write, but hard to read or grep.

> 
> Thanks in advance for your insights!
> 
> Below is the patch for reference:
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index d7ab4f96d705..6a766aaa457e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2576,28 +2576,36 @@ static inline void kiocb_clone(struct kiocb *kiocb, 
> struct kiocb *kiocb_src,
>  * __I_{SYNC,NEW,LRU_ISOLATING} are used to derive unique addresses to wait
>  * upon. There's one free address left.
>  */
> -#define __I_NEW 0
> -#define I_NEW (1 << __I_NEW)
> -#define __I_SYNC 1
> -#define I_SYNC (1 << __I_SYNC)
> -#define __I_LRU_ISOLATING 2
> -#define I_LRU_ISOLATING (1 << __I_LRU_ISOLATING)
> -
> -#define I_DIRTY_SYNC (1 << 3)
> -#define I_DIRTY_DATASYNC (1 << 4)
> -#define I_DIRTY_PAGES (1 << 5)
> -#define I_WILL_FREE (1 << 6)
> -#define I_FREEING (1 << 7)
> -#define I_CLEAR (1 << 8)
> -#define I_REFERENCED (1 << 9)
> -#define I_LINKABLE (1 << 10)
> -#define I_DIRTY_TIME (1 << 11)
> -#define I_WB_SWITCH (1 << 12)
> -#define I_OVL_INUSE (1 << 13)
> -#define I_CREATING (1 << 14)
> -#define I_DONTCACHE (1 << 15)
> -#define I_SYNC_QUEUED (1 << 16)
> -#define I_PINNING_NETFS_WB (1 << 17)
> +#define INODE_STATE(X) \

And it should be INODE_STATE().

> + X(I_NEW), \
> + X(I_SYNC), \
> + X(I_LRU_ISOLATING), \
> + X(I_DIRTY_SYNC), \
> + X(I_DIRTY_DATASYNC), \
> + X(I_DIRTY_PAGES), \
> + X(I_WILL_FREE), \
> + X(I_FREEING), \
> + X(I_CLEAR), \
> + X(I_REFERENCED), \
> + X(I_LINKABLE), \
> + X(I_DIRTY_TIME), \
> + X(I_WB_SWITCH), \
> + X(I_OVL_INUSE), \
> + X(I_CREATING), \
> + X(I_DONTCACHE), \
> + X(I_SYNC_QUEUED), \
> + X(I_PINNING_NETFS_WB)
> +
> +enum __inode_state_bits {
> + #define X(state) __##state
> + INODE_STATE(X)
> + #undef X
> +};
> +enum inode_state_bits {
> + #define X(state) state = (1 << __##state)
> + INODE_STATE(X)
> + #undef X
> +};
> 
> #define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
> #define I_DIRTY (I_DIRTY_INODE | I_DIRTY_PAGES)
> diff --git a/include/trace/events/writeback.h b/include/trace/events/
> writeback.h
> index 1e23919c0da9..4c545c72c40a 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -9,26 +9,10 @@
> #include <linux/backing-dev.h>
> #include <linux/writeback.h>
> 
> -#define show_inode_state(state) \
> - __print_flags(state, "|", \
> - {I_DIRTY_SYNC, "I_DIRTY_SYNC"}, \
> - {I_DIRTY_DATASYNC, "I_DIRTY_DATASYNC"}, \
> - {I_DIRTY_PAGES, "I_DIRTY_PAGES"}, \
> - {I_NEW, "I_NEW"}, \
> - {I_WILL_FREE, "I_WILL_FREE"}, \
> - {I_FREEING, "I_FREEING"}, \
> - {I_CLEAR, "I_CLEAR"}, \
> - {I_SYNC, "I_SYNC"}, \
> - {I_DIRTY_TIME, "I_DIRTY_TIME"}, \
> - {I_REFERENCED, "I_REFERENCED"}, \
> - {I_LINKABLE, "I_LINKABLE"}, \
> - {I_WB_SWITCH, "I_WB_SWITCH"}, \
> - {I_OVL_INUSE, "I_OVL_INUSE"}, \
> - {I_CREATING, "I_CREATING"}, \
> - {I_DONTCACHE, "I_DONTCACHE"}, \
> - {I_SYNC_QUEUED, "I_SYNC_QUEUED"}, \
> - {I_PINNING_NETFS_WB, "I_PINNING_NETFS_WB"}, \
> - {I_LRU_ISOLATING, "I_LRU_ISOLATING"} \
> +#define inode_state_name(s) { s, #s }
> +#define show_inode_state(state) \
> + __print_flags(state, "|", \
> + INODE_STATE(inode_state_name) \
> )
> 
> /* enums need to be exported to user space */
> 
> Best regards,
> Sun YangKai
> <x-macro.patch>