[Cluster-devel] Re: GFS2: Add tracepoints
Christoph Hellwig
hch at infradead.org
Fri May 29 19:23:52 UTC 2009
On Fri, May 29, 2009 at 08:11:00PM +0100, Steven Whitehouse wrote:
> +static inline u8 glock_trace_state(unsigned int state)
> +{
> + switch(state) {
> + case LM_ST_SHARED:
> + return DLM_LOCK_PR;
> + case LM_ST_DEFERRED:
> + return DLM_LOCK_CW;
> + case LM_ST_EXCLUSIVE:
> + return DLM_LOCK_EX;
> + }
> + return DLM_LOCK_NL;
> +}
I think this would be better done using __print_symbolic.
> +static inline const char *glock_trace_name(u8 state)
> +{
> + switch(state) {
> + case DLM_LOCK_PR:
> + return "PR";
> + case DLM_LOCK_CW:
> + return "CW";
> + case DLM_LOCK_EX:
> + return "EX";
> + case DLM_LOCK_NL:
> + return "NL";
> + default:
> + return "--";
> + }
> +}
Same here.
> +static inline const char *gfs2_blkst_name(u8 state)
> +{
> + switch(state) {
> + case GFS2_BLKST_FREE:
> + return "free";
> + case GFS2_BLKST_USED:
> + return "used";
> + case GFS2_BLKST_DINODE:
> + return "dinode";
> + case GFS2_BLKST_UNLINKED:
> + return "unlinked";
> + }
> + return "???";
> +}
Same here.
> + TP_STRUCT__entry(
> + __array( char, devname, BDEVNAME_SIZE )
This is extremly inefficient. We'd be much better off just storing
the dev_t and introducing a __trace_bdevname to expand a bdevname
into the tracer buffer. It's been on my todo list for a while and
I'll look into it next week unless you beat me to it.
> + __entry->gltype = gl->gl_name.ln_type;
> + __entry->cur_state = glock_trace_state(gl->gl_state);
> + __entry->new_state = glock_trace_state(new_state);
> + __entry->tgt_state = glock_trace_state(gl->gl_target);
> + __entry->dmt_state = DLM_LOCK_IV;
> + if (test_bit(GLF_DEMOTE, &gl->gl_flags) ||
> + test_bit(GLF_PENDING_DEMOTE, &gl->gl_flags))
> + __entry->dmt_state = glock_trace_state(gl->gl_demote_state);
Wouldn't it be better to just trace gl_flags and gl_demote_state?
> +);
> +
> +#endif /* _TRACE_GFS2_H */
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH ../../fs/gfs2/
Shouldn't an
#define TRACE_INCLUDE_PATH .
do it? (although that would need -I$(src), not sure how good that is.
More information about the Cluster-devel
mailing list