linux-kernel - Re: [patch 1/2] vfs: add/use update_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090218140626.GA13362@wotan.suse.de>
Date:	Wed, 18 Feb 2009 15:06:26 +0100
From:	Nick Piggin <npiggin@...e.de>
To:	Edward Shishkin <edward.shishkin@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, peterz@...radead.org,
	rmh3093@...il.com, randy.dunlap@...cle.com,
	linux-kernel@...r.kernel.org, reiserfs-devel@...r.kernel.org
Subject: Re: [patch 1/2] vfs: add/use update_page_accounting

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@...il.com>
> ---
>  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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/