On Tue, May 10, 2011 at 09:52:00PM +0800, Jan Kara wrote: > On Tue 10-05-11 10:23:36, Wu Fengguang wrote: > > On Fri, May 06, 2011 at 10:36:14PM +0800, Jan Kara wrote: > > > On Fri 06-05-11 11:08:24, Wu Fengguang wrote: > > > > The flusher works on dirty inodes in batches, and may quit prematurely > > > > if the batch of inodes happen to be metadata-only dirtied: in this case > > > > wbc->nr_to_write won't be decreased at all, which stands for "no pages > > > > written" but also mis-interpreted as "no progress". > > > > > > > > So introduce writeback_control.inodes_cleaned to count the inodes get > > > > cleaned. A non-zero value means there are some progress on writeback, > > > > in which case more writeback can be tried. > > > > > > > > about v1: The initial version was to count successful ->write_inode() > > > > calls. However it leads to busy loops for sync() over NFS, because NFS > > > > ridiculously returns 0 (success) while at the same time redirties the > > > > inode. The NFS case can be trivially fixed, however there may be more > > > > hidden bugs in other filesystems.. > > > I think Christoph didn't like this patch > > > (https://lkml.org/lkml/2011/5/4/123) and suggested that inodes_cleaned > > > should remain local to fs-writeback.c... > > > > Yes, he didn't like introducing one more field to writeback_control, > > which will be removed in patch 14. (It will be a lot of fuss to move > > patch 14 here or move this logic after patch 14.) > Yes, I've noticed (after I've sent this email) that you eventually remove > the code later in the series :). So I wonder would it be that much work to > just drop this patch? It's kind of dumb to add a new code which is removed > a few patches later. But I'm stopping bitching about this now ;). OK, thanks. I'm just uncomfortable that something will be broken in between some commits if simply drop this patch :) Thanks, Fengguang -- 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