On Wed, Feb 18, 2009 at 04:27:02PM +0300, Edward Shishkin wrote:
> > > Maybe it makes sense to add comments with warnings
> > > in all such places, or create a header file with a static inline
> > > function update_page_accounting() ?
> >
> > Could just uninline the helper function I guess - if you look, those
> > four statements already involve doing a heck of a lot of stuff.
> >
> > Try it, see how it looks?
> >
>
> Done.
> Please, review.
>
> Add/use a helper function update_page_accounting().
Fine patch, except the name I don't like. account_set_page_dirty, or
account_page_dirtied, or something to hint it is for accounting
dirty.
> Signed-off-by: Edward Shishkin<edward.shishkin@xxxxxxxxx>
> ---
> fs/buffer.c | 9 +--------
> include/linux/mm.h | 1 +
> mm/page-writeback.c | 22 +++++++++++++++-------
> 3 files changed, 17 insertions(+), 15 deletions(-)
>
> --- mmotm.orig/fs/buffer.c
> +++ mmotm/fs/buffer.c
> @@ -803,14 +803,7 @@ static int __set_page_dirty(struct page
> spin_lock_irq(&mapping->tree_lock);
> if (page->mapping) { /* Race with truncate? */
> WARN_ON_ONCE(warn && !PageUptodate(page));
> -
> - if (mapping_cap_account_dirty(mapping)) {
> - __inc_zone_page_state(page, NR_FILE_DIRTY);
> - __inc_bdi_stat(mapping->backing_dev_info,
> - BDI_RECLAIMABLE);
> - task_dirty_inc(current);
> - task_io_account_write(PAGE_CACHE_SIZE);
> - }
> + update_page_accounting(page, mapping);
> radix_tree_tag_set(&mapping->page_tree,
> page_index(page), PAGECACHE_TAG_DIRTY);
> }
> --- mmotm.orig/include/linux/mm.h
> +++ mmotm/include/linux/mm.h
> @@ -839,6 +839,7 @@ int __set_page_dirty_nobuffers(struct pa
> int __set_page_dirty_no_writeback(struct page *page);
> int redirty_page_for_writepage(struct writeback_control *wbc,
> struct page *page);
> +void update_page_accounting(struct page *page, struct address_space *mapping);
> int set_page_dirty(struct page *page);
> int set_page_dirty_lock(struct page *page);
> int clear_page_dirty_for_io(struct page *page);
> --- mmotm.orig/mm/page-writeback.c
> +++ mmotm/mm/page-writeback.c
> @@ -1198,6 +1198,20 @@ int __set_page_dirty_no_writeback(struct
> }
>
> /*
> + * Helper function for set_page_dirty family.
> + * NOTE: This relies on being atomic wrt interrupts.
> + */
> +void update_page_accounting(struct page *page, struct address_space *mapping)
> +{
> + if (mapping_cap_account_dirty(mapping)) {
> + __inc_zone_page_state(page, NR_FILE_DIRTY);
> + __inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> + task_dirty_inc(current);
> + task_io_account_write(PAGE_CACHE_SIZE);
> + }
> +}
> +
> +/*
> * For address_spaces which do not use buffers. Just tag the page as dirty in
> * its radix tree.
> *
> @@ -1226,13 +1240,7 @@ int __set_page_dirty_nobuffers(struct pa
> if (mapping2) { /* Race with truncate? */
> BUG_ON(mapping2 != mapping);
> WARN_ON_ONCE(!PagePrivate(page) && !PageUptodate(page));
> - if (mapping_cap_account_dirty(mapping)) {
> - __inc_zone_page_state(page, NR_FILE_DIRTY);
> - __inc_bdi_stat(mapping->backing_dev_info,
> - BDI_RECLAIMABLE);
> - task_dirty_inc(current);
> - task_io_account_write(PAGE_CACHE_SIZE);
> - }
> + update_page_accounting(page, mapping);
> radix_tree_tag_set(&mapping->page_tree,
> page_index(page), PAGECACHE_TAG_DIRTY);
> }
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
