Hi Jan:
On Fri, Aug 12, 2011 at 1:43 PM, Jan Kara <jack@xxxxxxx> wrote:
> 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()...
Yeah, good idea.
>> 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?
Oops. This should have been in the "PATCH 2/2" , not this one. v2 of
these patches are in the mail.
Thanks,
Curt
>
> 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
