[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20110511145412.GE5057@quack.suse.cz>
Date: Wed, 11 May 2011 16:54:12 +0200
From: Jan Kara <jack@...e.cz>
To: Wu Fengguang <fengguang.wu@...el.com>
Cc: Jan Kara <jack@...e.cz>, Andrew Morton <akpm@...ux-foundation.org>,
LKML <linux-kernel@...r.kernel.org>,
Dave Chinner <david@...morbit.com>,
Christoph Hellwig <hch@...radead.org>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 14/17] writeback: make writeback_control.nr_to_write
straight
On Wed 11-05-11 22:38:07, Wu Fengguang wrote:
> > One more thing that I've noticed while looking through this patch: You
> > are removing several trace points (without actually removing the trace
> > point definitions BTW):
> > trace_wbc_writeback_start(&wbc, wb->bdi);
> > trace_wbc_writeback_written(&wbc, wb->bdi);
> > trace_wbc_writeback_wait(&wbc, wb->bdi);
> > trace_wbc_balance_dirty_start(&wbc, bdi);
> > trace_wbc_balance_dirty_written(&wbc, bdi);
> > trace_wbc_balance_dirty_wait(&wbc, bdi);
> >
> > I guess they should be replaced with equivalent trace points that take
> > struct wb_writeback_work as an argument instead of just removing them?
>
> OK. How about this change?
Great. This should be fine. Thanks.
Honza
>
> ---
> fs/fs-writeback.c | 3 ++
> include/trace/events/writeback.h | 33 +++++++++++++++++++++++------
> mm/page-writeback.c | 3 ++
> 3 files changed, 33 insertions(+), 6 deletions(-)
>
> --- linux-next.orig/fs/fs-writeback.c 2011-05-11 22:19:22.000000000 +0800
> +++ linux-next/fs/fs-writeback.c 2011-05-11 22:20:50.000000000 +0800
> @@ -731,6 +731,7 @@ static long wb_writeback(struct bdi_writ
> }
>
> retry:
> + trace_writeback_start(wb->bdi, work);
> spin_lock(&wb->list_lock);
> if (list_empty(&wb->b_io))
> queue_io(wb, work->older_than_this);
> @@ -739,6 +740,7 @@ retry:
> else
> progress = __writeback_inodes_wb(wb, work);
> spin_unlock(&wb->list_lock);
> + trace_writeback_written(wb->bdi, work);
>
> /*
> * Did we write something? Try for more
> @@ -775,6 +777,7 @@ retry:
> */
> spin_lock(&wb->list_lock);
> if (!list_empty(&wb->b_more_io)) {
> + trace_writeback_wait(wb->bdi, work);
> inode = wb_inode(wb->b_more_io.prev);
> spin_lock(&inode->i_lock);
> inode_wait_for_writeback(inode, wb);
> --- linux-next.orig/mm/page-writeback.c 2011-05-11 22:19:22.000000000 +0800
> +++ linux-next/mm/page-writeback.c 2011-05-11 22:20:50.000000000 +0800
> @@ -555,14 +555,17 @@ static void balance_dirty_pages(struct a
> * threshold otherwise wait until the disk writes catch
> * up.
> */
> + trace_balance_dirty_start(bdi);
> if (bdi_nr_reclaimable > bdi_thresh) {
> pages_written += writeback_inodes_wb(&bdi->wb,
> write_chunk);
> + trace_balance_dirty_written(bdi, pages_written);
> if (pages_written >= write_chunk)
> break; /* We've done our duty */
> }
> __set_current_state(TASK_UNINTERRUPTIBLE);
> io_schedule_timeout(pause);
> + trace_balance_dirty_wait(bdi);
>
> /*
> * Increase the delay for each loop, up to our previous
> --- linux-next.orig/include/trace/events/writeback.h 2011-05-11 22:19:22.000000000 +0800
> +++ linux-next/include/trace/events/writeback.h 2011-05-11 22:22:20.000000000 +0800
> @@ -49,6 +49,9 @@ DEFINE_EVENT(writeback_work_class, name,
> DEFINE_WRITEBACK_WORK_EVENT(writeback_nothread);
> DEFINE_WRITEBACK_WORK_EVENT(writeback_queue);
> DEFINE_WRITEBACK_WORK_EVENT(writeback_exec);
> +DEFINE_WRITEBACK_WORK_EVENT(writeback_start);
> +DEFINE_WRITEBACK_WORK_EVENT(writeback_written);
> +DEFINE_WRITEBACK_WORK_EVENT(writeback_wait);
>
> TRACE_EVENT(writeback_pages_written,
> TP_PROTO(long pages_written),
> @@ -88,6 +91,30 @@ DEFINE_WRITEBACK_EVENT(writeback_bdi_reg
> DEFINE_WRITEBACK_EVENT(writeback_bdi_unregister);
> DEFINE_WRITEBACK_EVENT(writeback_thread_start);
> DEFINE_WRITEBACK_EVENT(writeback_thread_stop);
> +DEFINE_WRITEBACK_EVENT(balance_dirty_start);
> +DEFINE_WRITEBACK_EVENT(balance_dirty_wait);
> +
> +TRACE_EVENT(balance_dirty_written,
> +
> + TP_PROTO(struct backing_dev_info *bdi, int written),
> +
> + TP_ARGS(bdi, written),
> +
> + TP_STRUCT__entry(
> + __array(char, name, 32)
> + __field(int, written)
> + ),
> +
> + TP_fast_assign(
> + strncpy(__entry->name, dev_name(bdi->dev), 32);
> + __entry->written = written;
> + ),
> +
> + TP_printk("bdi %s written %d",
> + __entry->name,
> + __entry->written
> + )
> +);
>
> DECLARE_EVENT_CLASS(wbc_class,
> TP_PROTO(struct writeback_control *wbc, struct backing_dev_info *bdi),
> @@ -137,12 +164,6 @@ DECLARE_EVENT_CLASS(wbc_class,
> DEFINE_EVENT(wbc_class, name, \
> TP_PROTO(struct writeback_control *wbc, struct backing_dev_info *bdi), \
> TP_ARGS(wbc, bdi))
> -DEFINE_WBC_EVENT(wbc_writeback_start);
> -DEFINE_WBC_EVENT(wbc_writeback_written);
> -DEFINE_WBC_EVENT(wbc_writeback_wait);
> -DEFINE_WBC_EVENT(wbc_balance_dirty_start);
> -DEFINE_WBC_EVENT(wbc_balance_dirty_written);
> -DEFINE_WBC_EVENT(wbc_balance_dirty_wait);
> DEFINE_WBC_EVENT(wbc_writepage);
>
> DECLARE_EVENT_CLASS(writeback_congest_waited_template,
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists