Re: [PATCH 5/5] writeback: add more tracepoints

[Jan Kara <jack@suse.cz>]

On Sat 12-01-13 11:17:02, Wu Fengguang wrote:
> Patch looks good to me. But CC more people for review.
  The patch looks fine to me as well. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>

> On Fri, Jan 11, 2013 at 01:06:37PM -0800, Tejun Heo wrote:
> > Add tracepoints for page dirtying, writeback_single_inode start, inode
> > dirtying and writeback.  For the latter two inode events, a pair of
> > events are defined to denote start and end of the operations (the
> > starting one has _start suffix and the one w/o suffix happens after
> > the operation is complete).  These inode ops are FS specific and can
> > be non-trivial and having enclosing tracepoints is useful for external
> > tracers.
> > 
> > This is part of tracepoint additions to improve visiblity into
> > dirtying / writeback operations for io tracer and userland.
> > 
> > v2: writeback_dirty_inode[_start] TPs may be called for files on
> >     pseudo FSes w/ unregistered bdi.  Check whether bdi->dev is %NULL
> >     before dereferencing.
> > 
> > v3: buffer dirtying moved to a block TP.
> > 
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > ---
> >  fs/fs-writeback.c                |  16 +++++-
> >  include/trace/events/writeback.h | 116 +++++++++++++++++++++++++++++++++++++++
> >  mm/page-writeback.c              |   2 +
> >  3 files changed, 132 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > index 310972b..359494e 100644
> > --- a/fs/fs-writeback.c
> > +++ b/fs/fs-writeback.c
> > @@ -318,8 +318,14 @@ static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work)
> >  
> >  static int write_inode(struct inode *inode, struct writeback_control *wbc)
> >  {
> > -	if (inode->i_sb->s_op->write_inode && !is_bad_inode(inode))
> > -		return inode->i_sb->s_op->write_inode(inode, wbc);
> > +	int ret;
> > +
> > +	if (inode->i_sb->s_op->write_inode && !is_bad_inode(inode)) {
> > +		trace_writeback_write_inode_start(inode, wbc);
> > +		ret = inode->i_sb->s_op->write_inode(inode, wbc);
> > +		trace_writeback_write_inode(inode, wbc);
> > +		return ret;
> > +	}
> >  	return 0;
> >  }
> >  
> > @@ -450,6 +456,8 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> >  
> >  	WARN_ON(!(inode->i_state & I_SYNC));
> >  
> > +	trace_writeback_single_inode_start(inode, wbc, nr_to_write);
> > +
> >  	ret = do_writepages(mapping, wbc);
> >  
> >  	/*
> > @@ -1150,8 +1158,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> >  	 * dirty the inode itself
> >  	 */
> >  	if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
> > +		trace_writeback_dirty_inode_start(inode, flags);
> > +
> >  		if (sb->s_op->dirty_inode)
> >  			sb->s_op->dirty_inode(inode, flags);
> > +
> > +		trace_writeback_dirty_inode(inode, flags);
> >  	}
> >  
> >  	/*
> > diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> > index b453d92..6a16fd2 100644
> > --- a/include/trace/events/writeback.h
> > +++ b/include/trace/events/writeback.h
> > @@ -32,6 +32,115 @@
> >  
> >  struct wb_writeback_work;
> >  
> > +TRACE_EVENT(writeback_dirty_page,
> > +
> > +	TP_PROTO(struct page *page, struct address_space *mapping),
> > +
> > +	TP_ARGS(page, mapping),
> > +
> > +	TP_STRUCT__entry (
> > +		__array(char, name, 32)
> > +		__field(unsigned long, ino)
> > +		__field(pgoff_t, index)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		strncpy(__entry->name,
> > +			mapping ? dev_name(mapping->backing_dev_info->dev) : "(unknown)", 32);
> > +		__entry->ino = mapping ? mapping->host->i_ino : 0;
> > +		__entry->index = page->index;
> > +	),
> > +
> > +	TP_printk("bdi %s: ino=%lu index=%lu",
> > +		__entry->name,
> > +		__entry->ino,
> > +		__entry->index
> > +	)
> > +);
> > +
> > +DECLARE_EVENT_CLASS(writeback_dirty_inode_template,
> > +
> > +	TP_PROTO(struct inode *inode, int flags),
> > +
> > +	TP_ARGS(inode, flags),
> > +
> > +	TP_STRUCT__entry (
> > +		__array(char, name, 32)
> > +		__field(unsigned long, ino)
> > +		__field(unsigned long, flags)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		struct backing_dev_info *bdi = inode->i_mapping->backing_dev_info;
> > +
> > +		/* may be called for files on pseudo FSes w/ unregistered bdi */
> > +		strncpy(__entry->name,
> > +			bdi->dev ? dev_name(bdi->dev) : "(unknown)", 32);
> > +		__entry->ino		= inode->i_ino;
> > +		__entry->flags		= flags;
> > +	),
> > +
> > +	TP_printk("bdi %s: ino=%lu flags=%s",
> > +		__entry->name,
> > +		__entry->ino,
> > +		show_inode_state(__entry->flags)
> > +	)
> > +);
> > +
> > +DEFINE_EVENT(writeback_dirty_inode_template, writeback_dirty_inode_start,
> > +
> > +	TP_PROTO(struct inode *inode, int flags),
> > +
> > +	TP_ARGS(inode, flags)
> > +);
> > +
> > +DEFINE_EVENT(writeback_dirty_inode_template, writeback_dirty_inode,
> > +
> > +	TP_PROTO(struct inode *inode, int flags),
> > +
> > +	TP_ARGS(inode, flags)
> > +);
> > +
> > +DECLARE_EVENT_CLASS(writeback_write_inode_template,
> > +
> > +	TP_PROTO(struct inode *inode, struct writeback_control *wbc),
> > +
> > +	TP_ARGS(inode, wbc),
> > +
> > +	TP_STRUCT__entry (
> > +		__array(char, name, 32)
> > +		__field(unsigned long, ino)
> > +		__field(int, sync_mode)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		strncpy(__entry->name,
> > +			dev_name(inode->i_mapping->backing_dev_info->dev), 32);
> > +		__entry->ino		= inode->i_ino;
> > +		__entry->sync_mode	= wbc->sync_mode;
> > +	),
> > +
> > +	TP_printk("bdi %s: ino=%lu sync_mode=%d",
> > +		__entry->name,
> > +		__entry->ino,
> > +		__entry->sync_mode
> > +	)
> > +);
> > +
> > +DEFINE_EVENT(writeback_write_inode_template, writeback_write_inode_start,
> > +
> > +	TP_PROTO(struct inode *inode, struct writeback_control *wbc),
> > +
> > +	TP_ARGS(inode, wbc)
> > +);
> > +
> > +DEFINE_EVENT(writeback_write_inode_template, writeback_write_inode,
> > +
> > +	TP_PROTO(struct inode *inode, struct writeback_control *wbc),
> > +
> > +	TP_ARGS(inode, wbc)
> > +);
> > +
> >  DECLARE_EVENT_CLASS(writeback_work_class,
> >  	TP_PROTO(struct backing_dev_info *bdi, struct wb_writeback_work *work),
> >  	TP_ARGS(bdi, work),
> > @@ -479,6 +588,13 @@ DECLARE_EVENT_CLASS(writeback_single_inode_template,
> >  	)
> >  );
> >  
> > +DEFINE_EVENT(writeback_single_inode_template, writeback_single_inode_start,
> > +	TP_PROTO(struct inode *inode,
> > +		 struct writeback_control *wbc,
> > +		 unsigned long nr_to_write),
> > +	TP_ARGS(inode, wbc, nr_to_write)
> > +);
> > +
> >  DEFINE_EVENT(writeback_single_inode_template, writeback_single_inode,
> >  	TP_PROTO(struct inode *inode,
> >  		 struct writeback_control *wbc,
> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> > index 0713bfb..3734cef 100644
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -1982,6 +1982,8 @@ int __set_page_dirty_no_writeback(struct page *page)
> >   */
> >  void account_page_dirtied(struct page *page, struct address_space *mapping)
> >  {
> > +	trace_writeback_dirty_page(page, mapping);
> > +
> >  	if (mapping_cap_account_dirty(mapping)) {
> >  		__inc_zone_page_state(page, NR_FILE_DIRTY);
> >  		__inc_zone_page_state(page, NR_DIRTIED);
> > -- 
> > 1.8.0.2
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR