linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>,
	"hugh.dickins@tiscali.co.uk" <hugh.dickins@tiscali.co.uk>,
	"hannes@cmpxchg.org" <hannes@cmpxchg.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Subject: Re: [PATCH 1/4] add swap cache interface for swap reference v2 (updated)
Date: Fri, 29 May 2009 15:05:25 +0900	[thread overview]
Message-ID: <20090529150525.cbbb4be1.nishimura@mxp.nes.nec.co.jp> (raw)
In-Reply-To: <20090529143758.4c3db3eb.kamezawa.hiroyu@jp.fujitsu.com>

On Fri, 29 May 2009 14:37:58 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Fri, 29 May 2009 14:08:32 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > IIUC, swap_free() at the end of shmem_writepage() should also be changed to swapcache_free().
> > > 
> > Hmm!. Oh, yes. shmem_writepage()'s error path. Thank you. It will be fixed.
> > 
> here. 
> 
Looks good to me.

	Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>


BTW, I'm now testing(with swap-in/out and swap-on/off) [2/4] of this patch set.
I think this patch set would work well, but it's a big change to swap,
so we should test them very carefully.

Thanks,
Daisuke Nishimura.

> ==
> 
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> In following patch, usage of swap cache will be recorded into swap_map.
> This patch is for necessary interface changes to do that.
> 
> 2 interfaces:
>   - swapcache_prepare()
>   - swapcache_free()
> is added for allocating/freeing refcnt from swap-cache to existing
> swap entries. But implementation itself is not changed under this patch.
> At adding swapcache_free(), memcg's hook code is moved under swapcache_free().
> This is better than using scattered hooks.
> 
> Changelog: v1->v2
>  - fixed shmem_writepage() error path.
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  include/linux/swap.h |    7 +++++++
>  mm/shmem.c           |    2 +-
>  mm/swap_state.c      |   11 +++++------
>  mm/swapfile.c        |   19 +++++++++++++++++++
>  mm/vmscan.c          |    3 +--
>  5 files changed, 33 insertions(+), 9 deletions(-)
> 
> Index: mmotm-2.6.30-May28/include/linux/swap.h
> ===================================================================
> --- mmotm-2.6.30-May28.orig/include/linux/swap.h
> +++ mmotm-2.6.30-May28/include/linux/swap.h
> @@ -282,8 +282,10 @@ extern void si_swapinfo(struct sysinfo *
>  extern swp_entry_t get_swap_page(void);
>  extern swp_entry_t get_swap_page_of_type(int);
>  extern int swap_duplicate(swp_entry_t);
> +extern int swapcache_prepare(swp_entry_t);
>  extern int valid_swaphandles(swp_entry_t, unsigned long *);
>  extern void swap_free(swp_entry_t);
> +extern void swapcache_free(swp_entry_t, struct page *page);
>  extern int free_swap_and_cache(swp_entry_t);
>  extern int swap_type_of(dev_t, sector_t, struct block_device **);
>  extern unsigned int count_swap_pages(int, int);
> @@ -352,11 +354,16 @@ static inline void show_swap_cache_info(
>  
>  #define free_swap_and_cache(swp)	is_migration_entry(swp)
>  #define swap_duplicate(swp)		is_migration_entry(swp)
> +#define swapcache_prepare(swp)		is_migration_entry(swp)
>  
>  static inline void swap_free(swp_entry_t swp)
>  {
>  }
>  
> +static inline void swapcache_free(swp_entry_t swp, struct page *page)
> +{
> +}
> +
>  static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
>  			struct vm_area_struct *vma, unsigned long addr)
>  {
> Index: mmotm-2.6.30-May28/mm/swap_state.c
> ===================================================================
> --- mmotm-2.6.30-May28.orig/mm/swap_state.c
> +++ mmotm-2.6.30-May28/mm/swap_state.c
> @@ -162,11 +162,11 @@ int add_to_swap(struct page *page)
>  			return 1;
>  		case -EEXIST:
>  			/* Raced with "speculative" read_swap_cache_async */
> -			swap_free(entry);
> +			swapcache_free(entry, NULL);
>  			continue;
>  		default:
>  			/* -ENOMEM radix-tree allocation failure */
> -			swap_free(entry);
> +			swapcache_free(entry, NULL);
>  			return 0;
>  		}
>  	}
> @@ -188,8 +188,7 @@ void delete_from_swap_cache(struct page 
>  	__delete_from_swap_cache(page);
>  	spin_unlock_irq(&swapper_space.tree_lock);
>  
> -	mem_cgroup_uncharge_swapcache(page, entry);
> -	swap_free(entry);
> +	swapcache_free(entry, page);
>  	page_cache_release(page);
>  }
>  
> @@ -293,7 +292,7 @@ struct page *read_swap_cache_async(swp_e
>  		/*
>  		 * Swap entry may have been freed since our caller observed it.
>  		 */
> -		if (!swap_duplicate(entry))
> +		if (!swapcache_prepare(entry))
>  			break;
>  
>  		/*
> @@ -317,7 +316,7 @@ struct page *read_swap_cache_async(swp_e
>  		}
>  		ClearPageSwapBacked(new_page);
>  		__clear_page_locked(new_page);
> -		swap_free(entry);
> +		swapcache_free(entry, NULL);
>  	} while (err != -ENOMEM);
>  
>  	if (new_page)
> Index: mmotm-2.6.30-May28/mm/swapfile.c
> ===================================================================
> --- mmotm-2.6.30-May28.orig/mm/swapfile.c
> +++ mmotm-2.6.30-May28/mm/swapfile.c
> @@ -510,6 +510,16 @@ void swap_free(swp_entry_t entry)
>  }
>  
>  /*
> + * Called after dropping swapcache to decrease refcnt to swap entries.
> + */
> +void swapcache_free(swp_entry_t entry, struct page *page)
> +{
> +	if (page)
> +		mem_cgroup_uncharge_swapcache(page, entry);
> +	return swap_free(entry);
> +}
> +
> +/*
>   * How many references to page are currently swapped out?
>   */
>  static inline int page_swapcount(struct page *page)
> @@ -1979,6 +1989,15 @@ bad_file:
>  	goto out;
>  }
>  
> +/*
> + * Called when allocating swap cache for exising swap entry,
> + */
> +int swapcache_prepare(swp_entry_t entry)
> +{
> +	return swap_duplicate(entry);
> +}
> +
> +
>  struct swap_info_struct *
>  get_swap_info_struct(unsigned type)
>  {
> Index: mmotm-2.6.30-May28/mm/vmscan.c
> ===================================================================
> --- mmotm-2.6.30-May28.orig/mm/vmscan.c
> +++ mmotm-2.6.30-May28/mm/vmscan.c
> @@ -477,8 +477,7 @@ static int __remove_mapping(struct addre
>  		swp_entry_t swap = { .val = page_private(page) };
>  		__delete_from_swap_cache(page);
>  		spin_unlock_irq(&mapping->tree_lock);
> -		mem_cgroup_uncharge_swapcache(page, swap);
> -		swap_free(swap);
> +		swapcache_free(swap, page);
>  	} else {
>  		__remove_from_page_cache(page);
>  		spin_unlock_irq(&mapping->tree_lock);
> Index: mmotm-2.6.30-May28/mm/shmem.c
> ===================================================================
> --- mmotm-2.6.30-May28.orig/mm/shmem.c
> +++ mmotm-2.6.30-May28/mm/shmem.c
> @@ -1097,7 +1097,7 @@ static int shmem_writepage(struct page *
>  	shmem_swp_unmap(entry);
>  unlock:
>  	spin_unlock(&info->lock);
> -	swap_free(swap);
> +	swapcache_free(swap, NULL);
>  redirty:
>  	set_page_dirty(page);
>  	if (wbc->for_reclaim)
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2009-05-29  6:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-28  4:54 [PATCH 0/4] memcg fix swap accounting (28/May) KAMEZAWA Hiroyuki
2009-05-28  5:10 ` [PATCH 1/4] add swap cache interface for swap reference KAMEZAWA Hiroyuki
2009-05-29  4:21   ` Daisuke Nishimura
2009-05-29  5:08     ` KAMEZAWA Hiroyuki
2009-05-29  5:37       ` [PATCH 1/4] add swap cache interface for swap reference v2 (updated) KAMEZAWA Hiroyuki
2009-05-29  6:05         ` Daisuke Nishimura [this message]
2009-05-29  6:53           ` KAMEZAWA Hiroyuki
2009-05-30  5:21         ` Balbir Singh
2009-05-28  5:19 ` [PATCH 2/4] modify swap_map and add SWAP_HAS_CACHE flag KAMEZAWA Hiroyuki
2009-05-30  6:10   ` Balbir Singh
2009-05-30 11:16     ` KAMEZAWA Hiroyuki
2009-05-30 11:35       ` Balbir Singh
2009-06-01  7:04   ` Daisuke Nishimura
2009-05-28  5:20 ` [PATCH 3/4] reuse unused swap entry if necessary KAMEZAWA Hiroyuki
2009-05-29 21:55   ` Andrew Morton
2009-05-30 11:11     ` KAMEZAWA Hiroyuki
2009-05-30  6:40   ` Balbir Singh
2009-05-28  5:21 ` [PATCH 4/4] memcg: fix swap accounting KAMEZAWA Hiroyuki
2009-05-30  7:20   ` Balbir Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090529150525.cbbb4be1.nishimura@mxp.nes.nec.co.jp \
    --to=nishimura@mxp.nes.nec.co.jp \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=hannes@cmpxchg.org \
    --cc=hugh.dickins@tiscali.co.uk \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox