From: Dave Chinner <david@fromorbit.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Wu Fengguang <fengguang.wu@intel.com>,
linux-fsdevel@vger.kernel.org, Curt Wohlgemuth <curtw@google.com>,
Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@infradead.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC][PATCH] writeback: Unduplicate writeback reason
Date: Wed, 14 Dec 2011 14:28:44 +1100 [thread overview]
Message-ID: <20111214032844.GL3179@dastard> (raw)
In-Reply-To: <1323825240.23971.11.camel@gandalf.stny.rr.com>
On Tue, Dec 13, 2011 at 08:14:00PM -0500, 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.
>
> 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.
I'm not sure this is a pattern we want to repeat all over the place -
print_symbolic() is quite widely used and adding macro redefinitions
all over the place doesn't fill me with joy.
AFAICT this code doesn't need a declared array to work so you can
just use a preprocessor construct like this (as used in XFS):
#define value_1 1
#define value_2 2
.....
or
enum {
value_1 = 1,
value_2 = 2,
.....
}
followed by:
#define VALUES \
{ value_1, "Value 1" }, \
{ value_2, "Value 2" }, \
.....
And it just uses print_symbolic(__entry->value, VALUES); to print
them out.
If this construct does everything requiredi, then I think it is a
much better pattern to use because it's easy to maintain, doesn't
require an array to be declared in a C file and doesn't require
macro tricks to do it's job....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2011-12-14 3:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-14 0:31 [PATCH] writeback: show writeback reason with __print_symbolic Wu Fengguang
2011-12-14 1:14 ` [RFC][PATCH] writeback: Unduplicate writeback reason Steven Rostedt
2011-12-14 1:49 ` Wu Fengguang
2011-12-14 2:56 ` Steven Rostedt
2011-12-14 3:28 ` Dave Chinner [this message]
2011-12-14 13:16 ` Wu Fengguang
2011-12-15 5:24 ` Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111214032844.GL3179@dastard \
--to=david@fromorbit.com \
--cc=curtw@google.com \
--cc=fengguang.wu@intel.com \
--cc=hch@infradead.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.