Re: [RFC][PATCH] writeback: Unduplicate writeback reason

[Wu Fengguang]

Hi Steven,

On Wed, Dec 14, 2011 at 09:14:00AM +0800, Steven Rostedt wrote:
> Names of the writeback reasons are used in both the main kernel as well
> as for parsing the tracepoint format file. Instead of duplicating the
> names in two locations making it likely that they may become out of
> sync, use some macro magic to make sure all the names stay in sync. Any
> update only needs to happen in one spot for it to take place in all
> locations.
 
It looks a bit hacky, but does the nice job of de-duplicating code.
And it compiles. So I like it and would like to take it with the below
rename :-)

> Note, this is an RFC patch, and it probably needs much better comments
> (well, it currently has no comments), and the C() macro probably should
> have a different name too.

C => WB_ENUM_REASONS_ITEM? It may look unpleasantly long, however is
unique enough to make the many #define/#undef safe. 

Thanks!
Fengguang

> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> 
> Index: linux-trace.git/fs/fs-writeback.c
> ===================================================================
> --- linux-trace.git.orig/fs/fs-writeback.c
> +++ linux-trace.git/fs/fs-writeback.c
> @@ -47,15 +47,10 @@ struct wb_writeback_work {
>  	struct completion *done;	/* set if the caller waits */
>  };
>  
> +#undef C
> +#define C(a, b) [a] = b
>  const char *wb_reason_name[] = {
> -	[WB_REASON_BACKGROUND]		= "background",
> -	[WB_REASON_TRY_TO_FREE_PAGES]	= "try_to_free_pages",
> -	[WB_REASON_SYNC]		= "sync",
> -	[WB_REASON_PERIODIC]		= "periodic",
> -	[WB_REASON_LAPTOP_TIMER]	= "laptop_timer",
> -	[WB_REASON_FREE_MORE_MEM]	= "free_more_memory",
> -	[WB_REASON_FS_FREE_SPACE]	= "fs_free_space",
> -	[WB_REASON_FORKER_THREAD]	= "forker_thread"
> +	WB_ENUM_REASONS
>  };
>  
>  /*
> Index: linux-trace.git/include/linux/writeback.h
> ===================================================================
> --- linux-trace.git.orig/include/linux/writeback.h
> +++ linux-trace.git/include/linux/writeback.h
> @@ -38,21 +38,23 @@ enum writeback_sync_modes {
>  	WB_SYNC_ALL,	/* Wait on every mapping */
>  };
>  
> +#define WB_ENUM_REASONS							\
> +		C(WB_REASON_BACKGROUND,		"background"),		\
> +		C(WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"),	\
> +		C(WB_REASON_SYNC,		"sync"),		\
> +		C(WB_REASON_PERIODIC,		"periodic"),		\
> +		C(WB_REASON_LAPTOP_TIMER,	"laptop_timer"),	\
> +		C(WB_REASON_FREE_MORE_MEM,	"free_more_memory"),	\
> +		C(WB_REASON_FS_FREE_SPACE,	"fs_free_space"),	\
> +		C(WB_REASON_FORKER_THREAD,	"forker_thread")
> +
>  /*
>   * why some writeback work was initiated
>   */
> -enum wb_reason {
> -	WB_REASON_BACKGROUND,
> -	WB_REASON_TRY_TO_FREE_PAGES,
> -	WB_REASON_SYNC,
> -	WB_REASON_PERIODIC,
> -	WB_REASON_LAPTOP_TIMER,
> -	WB_REASON_FREE_MORE_MEM,
> -	WB_REASON_FS_FREE_SPACE,
> -	WB_REASON_FORKER_THREAD,
> +#undef C
> +#define C(a, b) a
> +enum wb_reason { WB_ENUM_REASONS, WB_REASONS_MAX };
>  
> -	WB_REASON_MAX,
> -};
>  extern const char *wb_reason_name[];
>  
>  /*
> Index: linux-trace.git/include/trace/events/writeback.h
> ===================================================================
> --- linux-trace.git.orig/include/trace/events/writeback.h
> +++ linux-trace.git/include/trace/events/writeback.h
> @@ -21,16 +21,11 @@
>  		{I_REFERENCED,		"I_REFERENCED"}		\
>  	)
>  
> +#undef C
> +#define C(a, b) {a, b}
>  #define show_work_reason(reason)					\
>  	__print_symbolic(reason,					\
> -		{WB_REASON_BACKGROUND,		"background"},		\
> -		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
> -		{WB_REASON_SYNC,		"sync"},		\
> -		{WB_REASON_PERIODIC,		"periodic"},		\
> -		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
> -		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
> -		{WB_REASON_FS_FREE_SPACE,	"fs_free_space"},	\
> -		{WB_REASON_FORKER_THREAD,	"forker_thread"}	\
> +			 WB_ENUM_REASONS				\
>  	)
>  
>  struct wb_writeback_work;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/