On Thu, 2009-12-17 at 09:40 -0600, Eric Sandeen wrote: > Richard Kennedy wrote: > > ext4: always re-base nr_to_write in ext4_da_writepages > > > > When ext4_da_writepages increases the nr_to_write in writeback_control > > then it must always re-base the return value. > > > > Without this change, when wb_writeback calculates how many pages were > > actually written it can get a negative value and loop more times than > > necessary. In tests I have seen nearly all the dirty pages pushed out to > > writeback due to this issue. > > > > Signed-off-by: Richard Kennedy <richard@xxxxxxxxxxxxxxx> > > > > ---- > > > > patch against 2.6.32 > > tested on x86_64 > > > > wb_writeback calculates (MAX_WRITE_PAGES - nr_to_write) & cannot know > > that the value got changed. > > > > I'm not sure what the test I removed was for. > > Perhaps > > if (nr_to_writebump) > > wbc->nr_to_write -= nr_to_writebump; > > was intended? > > Ted's commit 55138e0b added it (just part of the commit): > > @@ -2914,7 +2994,8 @@ retry: > out_writepages: > if (!no_nrwrite_index_update) > wbc->no_nrwrite_index_update = 0; > - wbc->nr_to_write -= nr_to_writebump; > + if (wbc->nr_to_write > nr_to_writebump) > + wbc->nr_to_write -= nr_to_writebump; > wbc->range_start = range_start; > trace_ext4_da_writepages_result(inode, wbc, ret, pages_written); > return ret; > > so it looks like the intent there was to stop ->nr_to_write from > going negative ... > > wb_writeback is OK with negative, it just needs to know how many pages were written. Then it can decide if it's done the work it was asked to do. balance_dirty_pages uses this throttle a device by asking for writeback on a small number of pages. regards Richard -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
