writeback: initial tracing support
in [Kernel]
|
Prev: cgroups: Add an API to attach a task to current task's cgroup
Next: perf: Precise task / softirq / hardirq filtered stats/profiles
From: Christoph Hellwig on 21 May 2010 11:10 On Tue, Apr 20, 2010 at 12:41:51PM +1000, Dave Chinner wrote: > From: From: Jens Axboe <jens.axboe(a)oracle.com> > > Trace queue/sched/exec parts of the writeback loop. If you move the CREATE_TRACE_POINTS into fs-writeback.c and include <trace/events/writeback.h> after the sturcture defintions there's no need to move them to a header. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on 25 May 2010 07:20 > +#define CREATE_TRACE_POINTS > +#include <trace/events/writeback.h> It might be worth to add a comment why this is in an unusal place. Otherwise looks good, Reviewed-by: Christoph Hellwig <hch(a)lst.de> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 27 May 2010 17:40 On Tue, 25 May 2010 20:54:07 +1000 Dave Chinner <david(a)fromorbit.com> wrote: > From: From: Jens Axboe <jens.axboe(a)oracle.com> > > Trace queue/sched/exec parts of the writeback loop. It would be most useful if this patchset's description provided sample tracing output, so we can see what the patch is actually providing us. > -#define inode_to_bdi(inode) ((inode)->i_mapping->backing_dev_info) > - > -/* > - * We don't actually have pdflush, but this one is exported though /proc... > - */ > -int nr_pdflush_threads; > - > /* > * Passed into wb_writeback(), essentially a subset of writeback_control > */ > @@ -63,6 +57,16 @@ struct bdi_work { > unsigned long state; /* flag bits, see WS_* */ > }; > > +#define CREATE_TRACE_POINTS > +#include <trace/events/writeback.h> > + > +#define inode_to_bdi(inode) ((inode)->i_mapping->backing_dev_info) Could/should be implemented in C. > +/* > + * We don't actually have pdflush, but this one is exported though /proc... > + */ > +int nr_pdflush_threads; So this is always zero now? We don't want to keep it forever. Add a printk_once("nr_pdflush_threads is deprecated") when someone reads it, remove it in 2014. > > ... > > --- /dev/null > +++ b/include/trace/events/writeback.h > @@ -0,0 +1,171 @@ > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM writeback > + > +#if !defined(_TRACE_WRITEBACK_H) || defined(TRACE_HEADER_MULTI_READ) Seems wrong. If you define TRACE_HEADER_MULTI_READ then include this header twice, things explode. Which negates the purpose of _TRACE_WRITEBACK_H. > +#define _TRACE_WRITEBACK_H > + > +#include <linux/backing-dev.h> > +#include <linux/writeback.h> > + > +TRACE_EVENT(writeback_queue, > + > + TP_PROTO(struct backing_dev_info *bdi, struct wb_writeback_args *args), > + > + TP_ARGS(bdi, args), > + > + TP_STRUCT__entry( > + __array(char, name, 16) > + __field(long, nr_pages) > + __field(int, sb) > + __field(int, sync_mode) > + __field(int, for_kupdate) > + __field(int, range_cyclic) > + __field(int, for_background) > + ), > + > + TP_fast_assign( > + strncpy(__entry->name, dev_name(bdi->dev), 16); > + __entry->nr_pages = args->nr_pages; > + __entry->sb = !!args->sb; > + __entry->for_kupdate = args->for_kupdate; > + __entry->range_cyclic = args->range_cyclic; > + __entry->for_background = args->for_background; > + ), > + > + TP_printk("%s: pages=%ld, sb=%d, kupdate=%d, range_cyclic=%d " > + "for_background=%d", __entry->name, __entry->nr_pages, > + __entry->sb, __entry->for_kupdate, > + __entry->range_cyclic, __entry->for_background) > +); > + -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Dave Chinner on 27 May 2010 20:50 On Thu, May 27, 2010 at 02:32:33PM -0700, Andrew Morton wrote: > On Tue, 25 May 2010 20:54:07 +1000 > Dave Chinner <david(a)fromorbit.com> wrote: > > > From: From: Jens Axboe <jens.axboe(a)oracle.com> > > > > Trace queue/sched/exec parts of the writeback loop. > > It would be most useful if this patchset's description provided sample > tracing output, so we can see what the patch is actually providing us. This is just a forward port of Jen's patch. I guess I'll have to clean it up some more... > > > -#define inode_to_bdi(inode) ((inode)->i_mapping->backing_dev_info) > > - > > -/* > > - * We don't actually have pdflush, but this one is exported though /proc... > > - */ > > -int nr_pdflush_threads; > > - > > /* > > * Passed into wb_writeback(), essentially a subset of writeback_control > > */ > > @@ -63,6 +57,16 @@ struct bdi_work { > > unsigned long state; /* flag bits, see WS_* */ > > }; > > > > +#define CREATE_TRACE_POINTS > > +#include <trace/events/writeback.h> > > + > > +#define inode_to_bdi(inode) ((inode)->i_mapping->backing_dev_info) > > Could/should be implemented in C. OK. > > > +/* > > + * We don't actually have pdflush, but this one is exported though /proc... > > + */ > > +int nr_pdflush_threads; > > So this is always zero now? I guess so. I'd forgotten that this was in the original patch.... > We don't want to keep it forever. Add a > printk_once("nr_pdflush_threads is deprecated") when someone reads it, > remove it in 2014. OK. > > --- /dev/null > > +++ b/include/trace/events/writeback.h > > @@ -0,0 +1,171 @@ > > +#undef TRACE_SYSTEM > > +#define TRACE_SYSTEM writeback > > + > > +#if !defined(_TRACE_WRITEBACK_H) || defined(TRACE_HEADER_MULTI_READ) > > Seems wrong. If you define TRACE_HEADER_MULTI_READ then include this > header twice, things explode. Which negates the purpose of > _TRACE_WRITEBACK_H. Every other trace event header does this, so if it's wrong then the same mistake is in at least 30 files now. I don't know enough about the tracing code to know why this is done, and I'm not keen to address such a mistake here... Cheers, Dave. -- Dave Chinner david(a)fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Steven Rostedt on 27 May 2010 21:20
On Thu, 2010-05-27 at 14:32 -0700, Andrew Morton wrote: > > --- /dev/null > > +++ b/include/trace/events/writeback.h > > @@ -0,0 +1,171 @@ > > +#undef TRACE_SYSTEM > > +#define TRACE_SYSTEM writeback > > + > > +#if !defined(_TRACE_WRITEBACK_H) || defined(TRACE_HEADER_MULTI_READ) > > Seems wrong. If you define TRACE_HEADER_MULTI_READ then include this > header twice, things explode. Which negates the purpose of > _TRACE_WRITEBACK_H. That's intended. It is documented in samples/trace_events/trace-events-samples.h The purpose of the TRACE_HEADER_MULTI_READ is to read the trace header multi times. ;-) You can also read about it here: http://lwn.net/Articles/379903/ here: http://lwn.net/Articles/381064/ and here: http://lwn.net/Articles/383362/ -- Steve > > > +#define _TRACE_WRITEBACK_H > > + > > +#include <linux/backing-dev.h> > > +#include <linux/writeback.h> > > + -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |