* Re: [PATCH 5/5] writeback: add more tracepoints
2013-01-12 3:17 ` [PATCH 5/5] writeback: add more tracepoints Fengguang Wu
@ 2013-01-14 13:57 ` Jan Kara
0 siblings, 0 replies; 2+ messages in thread
From: Jan Kara @ 2013-01-14 13:57 UTC (permalink / raw)
To: Fengguang Wu
Cc: Tejun Heo, axboe, linux-kernel, chavey, Jan Kara, linux-fsdevel
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
^ permalink raw reply [flat|nested] 2+ messages in thread