On Tue, Nov 22, 2011 at 05:51:46AM +0800, Andrew Morton wrote:
> On Mon, 21 Nov 2011 21:03:46 +0800
> Wu Fengguang <fengguang.wu@xxxxxxxxx> wrote:
>
> > De-account the accumulative dirty counters on page redirty.
> >
> > Page redirties (very common in ext4) will introduce mismatch between
> > counters (a) and (b)
> >
> > a) NR_DIRTIED, BDI_DIRTIED, tsk->nr_dirtied
> > b) NR_WRITTEN, BDI_WRITTEN
> >
> > This will introduce systematic errors in balanced_rate and result in
> > dirty page position errors (ie. the dirty pages are no longer balanced
> > around the global/bdi setpoints).
> >
> > ...
> >
> > --- linux-next.orig/mm/page-writeback.c 2011-11-17 20:57:15.000000000 +0800
> > +++ linux-next/mm/page-writeback.c 2011-11-17 20:57:16.000000000 +0800
> > @@ -1792,6 +1792,17 @@ int __set_page_dirty_nobuffers(struct pa
> > }
> > EXPORT_SYMBOL(__set_page_dirty_nobuffers);
> >
> > +void account_page_redirty(struct page *page)
> > +{
> > + struct address_space *mapping = page->mapping;
> > + if (mapping && mapping_cap_account_dirty(mapping)) {
> > + current->nr_dirtied--;
> > + dec_zone_page_state(page, NR_DIRTIED);
> > + dec_bdi_stat(mapping->backing_dev_info, BDI_DIRTIED);
> > + }
> > +}
>
> Again, writeback doesn't seem to be the best place to be adding
> undocumented code.
Sorry, hope this comment can explain when to call the function and why.
/*
* Call this whenever redirtying a page, to de-account the dirty counters
* (NR_DIRTIED, BDI_DIRTIED, tsk->nr_dirtied), so that they match the written
* counters (NR_WRITTEN, BDI_WRITTEN) in long term. The mismatches will lead to
* systematic errors in balanced_dirty_ratelimit and the dirty pages position
* control.
*/
> > +EXPORT_SYMBOL(account_page_redirty);
>
> The export is unneeded?
btrfs will need to call into it. Here is the patch. I'll include it
in the v2 series.
Thanks,
Fengguang
---
Subject: btrfs: fix dirtied pages accounting on sub-page writes
Date: Mon Aug 08 15:19:47 CST 2011
When doing 1KB sequential writes to the same page,
balance_dirty_pages_ratelimited_nr() should be called once instead of 4
times, the latter makes the dirtier tasks be throttled much too heavy.
Fix it with proper de-accounting on clear_page_dirty_for_io().
CC: Chris Mason <chris.mason@xxxxxxxxxx>
Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
---
fs/btrfs/file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next.orig/fs/btrfs/file.c 2011-11-17 20:13:47.000000000 +0800
+++ linux-next/fs/btrfs/file.c 2011-11-17 20:18:51.000000000 +0800
@@ -1136,7 +1136,8 @@ again:
GFP_NOFS);
}
for (i = 0; i < num_pages; i++) {
- clear_page_dirty_for_io(pages[i]);
+ if (clear_page_dirty_for_io(pages[i]))
+ account_page_redirty(pages[i]);
set_page_extent_mapped(pages[i]);
WARN_ON(!PageLocked(pages[i]));
}
--
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
