Hi again, On Sun, Sep 30, 2012 at 11:11:10AM +0900, Itaru Kitayama wrote: > This is the correct one. > > Signed-off-by: Itaru Kitayama <kitay...@cl.bb4.ne.jp> > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index efb044e..c032dbe 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -3712,8 +3712,9 @@ static void shrink_delalloc(struct btrfs_root > *root, u64 to_reclaim, u64 orig, > while (delalloc_bytes && loops < 3) { > max_reclaim = min(delalloc_bytes, to_reclaim); > nr_pages = max_reclaim >> PAGE_CACHE_SHIFT; > - writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages, > - WB_REASON_FS_FREE_SPACE); > + if (!bdi_write_congested(root->fs_info->sb->s_bdi)) > + writeback_inodes_sb_nr_if_idle(root->fs_info, > nr_pages, > + > WB_REASON_FS_FREE_SPACE);
You don't pass the same argument to writeback_inodes_sb_nr_if_idle in the changed code, this would not compile (root->fs_info->sb), but it's a minor thing. I'm more interested in the background motivation of the change, it's clear that it tries to avoid writing data if the devices are congested, have you measured an improvement against original behaviour? writeback_inodes_sb_nr_if_idle checks if writeback is in progress and does not start if this is true. That way this will not hammer the device unnecessarily. Your code tries to avoid even more hammering of the device when the writes do not come from writeback. This may or may not be a good thing (and hard to guess without a few tests, or at least I don't see that). Shrink delalloc starts the writeback for a given number of pages and then hopes they'll be flushed so the reserved space can be reclaimed back. If the device is congested, this will not start the writeback and it would be very unlikely that total delalloc bytes shrinks. The rest of the function relies on asynchronous behaviour, it's even less clear what it would do without the writeback call. In the worst case it could block on 'wait_event' or at 'btrfs_wait_ordered_extents' in some case, though this is just more of a speculation. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html