On Fri 12-08-11 11:45:06, Curt Wohlgemuth wrote:
> This creates a new 'reason' field in a wb_writeback_work
> structure, which unambiguously identifies who initiates
> writeback activity. A 'wb_stats' enumeration has been added
> to writeback.h, to enumerate the possible reasons.
>
> The 'writeback_work_class' tracepoint event class is updated
> to include the symbolic 'reason' in all trace events.
>
> The 'writeback_queue_io' tracepoint now takes a work object,
> in order to print out the 'reason' for queue_io.
>
> And the 'writeback_inodes_sbXXX' family of routines has had
> a wb_stats parameter added to them, so callers can specify
> why writeback is being started.
>
> Signed-off-by: Curt Wohlgemuth <curtw@xxxxxxxxxx>
The patch looks good. Just two minor comments below. So you can
add:
Acked-by: Jan Kara <jack@xxxxxxx>
> @@ -647,11 +651,12 @@ long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
> .nr_pages = nr_pages,
> .sync_mode = WB_SYNC_NONE,
> .range_cyclic = 1,
> + .reason = WB_STAT_BALANCE_DIRTY,
> };
>
> spin_lock(&wb->list_lock);
> if (list_empty(&wb->b_io))
> - queue_io(wb, NULL);
> + queue_io(wb, &work);
> __writeback_inodes_wb(wb, &work);
> spin_unlock(&wb->list_lock);
>
Umm, for consistency it would make more sense for writeback_inodes_wb()
to take reason argument as well. Also strictly speaking, this function has
two callers - balance_dirty_pages() and bdi_forker_thread()...
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index d196074..53c995e 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -737,8 +737,9 @@ static void balance_dirty_pages(struct address_space *mapping,
> */
> trace_balance_dirty_start(bdi);
> if (bdi_nr_reclaimable > task_bdi_thresh) {
> - pages_written += writeback_inodes_wb(&bdi->wb,
> - write_chunk);
> + long wrote;
> + wrote = writeback_inodes_wb(&bdi->wb, write_chunk);
> + pages_written += wrote;
What is this hunk for?
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
