Re: [PATCH 1/9] writeback: make writeback_control.nr_to_write straight

Jan Kara
2011-06-30T16:25:01Z

On Wed 29-06-11 22:52:46, Wu Fengguang wrote:
> Pass struct wb_writeback_work all the way down to writeback_sb_inodes(),
> and initialize the struct writeback_control there.
>
> struct writeback_control is basically designed to control writeback of a
> single file, but we keep abuse it for writing multiple files in
> writeback_sb_inodes() and its callers.
>
> It immediately clean things up, e.g. suddenly wbc.nr_to_write vs
> work->nr_pages starts to make sense, and instead of saving and restoring
> pages_skipped in writeback_sb_inodes it can always start with a clean
> zero value.
>
> It also makes a neat IO pattern change: large dirty files are now
> written in the full 4MB writeback chunk size, rather than whatever
> remained quota in wbc->nr_to_write.
>
> Acked-by: Jan Kara <jack@suse.cz>
> Proposed-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Just one minor nit:

> @@ -570,17 +622,25 @@ static int writeback_sb_inodes(struct su
> iput(inode);
> cond_resched();
> spin_lock(&wb->list_lock);
> - if (wbc->nr_to_write <= 0)
> - return 1;
> + /*
> + * bail out to wb_writeback() often enough to check
> + * background threshold and other termination conditions.
> + */
> + if (wrote) {
> + if (jiffies - start_time > HZ / 10UL)
> + break;
I guess this comparison should use time_before() macro - or maybe even
time_is_before_jiffies().

> + if (work->nr_pages <= 0)
> + break;
> + }
> }
> - /* b_io is empty */
> - return 1;
> + return wrote;
> }
>
> -static void __writeback_inodes_wb(struct bdi_writeback *wb,
> - struct writeback_control *wbc)
> +static long __writeback_inodes_wb(struct bdi_writeback *wb,
> + struct wb_writeback_work *work)
> {
> - int ret = 0;
> + unsigned long start_time = jiffies;
> + long wrote = 0;
>
> while (!list_empty(&wb->b_io)) {
> struct inode *inode = wb_inode(wb->b_io.prev);
> @@ -590,33 +650,37 @@ static void __writeback_inodes_wb(struct
> requeue_io(inode, wb);
> continue;
> }
> - ret = writeback_sb_inodes(sb, wb, wbc, false);
> + wrote += writeback_sb_inodes(sb, wb, work);
> drop_super(sb);
>
> - if (ret)
> - break;
> + /* refer to the same tests at the end of writeback_sb_inodes */
> + if (wrote) {
> + if (jiffies - start_time > HZ / 10UL)
> + break;
And the same here.

> + if (work->nr_pages <= 0)
> + break;
> + }
> }
> /* Leave any unwritten inodes on b_io */
> + return wrote;
> }
>

Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR