Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1556783 > unrolled thread

[PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks

Started byTim Chen <tim.c.chen@linux.intel.com>
First post2017-01-11 19:00 +0100
Last post2017-01-12 17:50 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks Tim Chen <tim.c.chen@linux.intel.com> - 2017-01-11 19:00 +0100
    Re: [PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks Andrew Morton <akpm@linux-foundation.org> - 2017-01-12 00:10 +0100
      Re: [PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks Andi Kleen <ak@linux.intel.com> - 2017-01-12 00:20 +0100
        Re: [PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks Tim Chen <tim.c.chen@linux.intel.com> - 2017-01-12 17:50 +0100

#1556783 — [PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-01-11 19:00 +0100
Subject[PATCH v5 3/9] mm/swap: Split swap cache into 64MB trunks
Message-ID<sYvlM-1rB-37@gated-at.bofh.it>
From: "Huang, Ying" <ying.huang@intel.com>

The patch is to improve the scalability of the swap out/in via using
fine grained locks for the swap cache.  In current kernel, one address
space will be used for each swap device.  And in the common
configuration, the number of the swap device is very small (one is
typical).  This causes the heavy lock contention on the radix tree of
the address space if multiple tasks swap out/in concurrently.  But in
fact, there is no dependency between pages in the swap cache.  So that,
we can split the one shared address space for each swap device into
several address spaces to reduce the lock contention.  In the patch, the
shared address space is split into 64MB trunks.  64MB is chosen to
balance the memory space usage and effect of lock contention reduction.

The size of struct address_space on x86_64 architecture is 408B, so with
the patch, 6528B more memory will be used for every 1GB swap space on
x86_64 architecture.

One address space is still shared for the swap entries in the same 64M
trunks.  To avoid lock contention for the first round of swap space
allocation, the order of the swap clusters in the initial free clusters
list is changed.  The swap space distance between the consecutive swap
clusters in the free cluster list is at least 64M.  After the first
round of allocation, the swap clusters are expected to be freed
randomly, so the lock contention should be reduced effectively.

Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 include/linux/swap.h | 11 +++++++--
 mm/swap.c            |  6 -----
 mm/swap_state.c      | 68 ++++++++++++++++++++++++++++++++++++++++++----------
 mm/swapfile.c        | 16 +++++++++++--
 4 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 40716b9..31d89c4 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -341,8 +341,13 @@ int generic_swapfile_activate(struct swap_info_struct *, struct file *,
 		sector_t *);
 
 /* linux/mm/swap_state.c */
-extern struct address_space swapper_spaces[];
-#define swap_address_space(entry) (&swapper_spaces[swp_type(entry)])
+/* One swap address space for each 64M swap space */
+#define SWAP_ADDRESS_SPACE_SHIFT	14
+#define SWAP_ADDRESS_SPACE_PAGES	(1 << SWAP_ADDRESS_SPACE_SHIFT)
+extern struct address_space *swapper_spaces[];
+#define swap_address_space(entry)			    \
+	(&swapper_spaces[swp_type(entry)][swp_offset(entry) \
+		>> SWAP_ADDRESS_SPACE_SHIFT])
 extern unsigned long total_swapcache_pages(void);
 extern void show_swap_cache_info(void);
 extern int add_to_swap(struct page *, struct list_head *list);
@@ -396,6 +401,8 @@ extern struct swap_info_struct *page_swap_info(struct page *);
 extern bool reuse_swap_page(struct page *, int *);
 extern int try_to_free_swap(struct page *);
 struct backing_dev_info;
+extern int init_swap_address_space(unsigned int type, unsigned long nr_pages);
+extern void exit_swap_address_space(unsigned int type);
 
 #else /* CONFIG_SWAP */
 
diff --git a/mm/swap.c b/mm/swap.c
index 844baed..aabf2e9 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -971,12 +971,6 @@ EXPORT_SYMBOL(pagevec_lookup_tag);
 void __init swap_setup(void)
 {
 	unsigned long megs = totalram_pages >> (20 - PAGE_SHIFT);
-#ifdef CONFIG_SWAP
-	int i;
-
-	for (i = 0; i < MAX_SWAPFILES; i++)
-		spin_lock_init(&swapper_spaces[i].tree_lock);
-#endif
 
 	/* Use a smaller cluster for small-memory machines */
 	if (megs < 16)
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 35d7e0e..3863acd 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -17,6 +17,7 @@
 #include <linux/blkdev.h>
 #include <linux/pagevec.h>
 #include <linux/migrate.h>
+#include <linux/vmalloc.h>
 
 #include <asm/pgtable.h>
 
@@ -32,15 +33,8 @@ static const struct address_space_operations swap_aops = {
 #endif
 };
 
-struct address_space swapper_spaces[MAX_SWAPFILES] = {
-	[0 ... MAX_SWAPFILES - 1] = {
-		.page_tree	= RADIX_TREE_INIT(GFP_ATOMIC|__GFP_NOWARN),
-		.i_mmap_writable = ATOMIC_INIT(0),
-		.a_ops		= &swap_aops,
-		/* swap cache doesn't use writeback related tags */
-		.flags		= 1 << AS_NO_WRITEBACK_TAGS,
-	}
-};
+struct address_space *swapper_spaces[MAX_SWAPFILES];
+static unsigned int nr_swapper_spaces[MAX_SWAPFILES];
 
 #define INC_CACHE_INFO(x)	do { swap_cache_info.x++; } while (0)
 
@@ -53,11 +47,26 @@ static struct {
 
 unsigned long total_swapcache_pages(void)
 {
-	int i;
+	unsigned int i, j, nr;
 	unsigned long ret = 0;
+	struct address_space *spaces;
 
-	for (i = 0; i < MAX_SWAPFILES; i++)
-		ret += swapper_spaces[i].nrpages;
+	rcu_read_lock();
+	for (i = 0; i < MAX_SWAPFILES; i++) {
+		/*
+		 * The corresponding entries in nr_swapper_spaces and
+		 * swapper_spaces will be reused only after at least
+		 * one grace period.  So it is impossible for them
+		 * belongs to different usage.
+		 */
+		nr = nr_swapper_spaces[i];
+		spaces = rcu_dereference(swapper_spaces[i]);
+		if (!nr || !spaces)
+			continue;
+		for (j = 0; j < nr; j++)
+			ret += spaces[j].nrpages;
+	}
+	rcu_read_unlock();
 	return ret;
 }
 
@@ -505,3 +514,38 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
 skip:
 	return read_swap_cache_async(entry, gfp_mask, vma, addr);
 }
+
+int init_swap_address_space(unsigned int type, unsigned long nr_pages)
+{
+	struct address_space *spaces, *space;
+	unsigned int i, nr;
+
+	nr = DIV_ROUND_UP(nr_pages, SWAP_ADDRESS_SPACE_PAGES);
+	spaces = vzalloc(sizeof(struct address_space) * nr);
+	if (!spaces)
+		return -ENOMEM;
+	for (i = 0; i < nr; i++) {
+		space = spaces + i;
+		INIT_RADIX_TREE(&space->page_tree, GFP_ATOMIC|__GFP_NOWARN);
+		atomic_set(&space->i_mmap_writable, 0);
+		space->a_ops = &swap_aops;
+		/* swap cache doesn't use writeback related tags */
+		mapping_set_no_writeback_tags(space);
+		spin_lock_init(&space->tree_lock);
+	}
+	nr_swapper_spaces[type] = nr;
+	rcu_assign_pointer(swapper_spaces[type], spaces);
+
+	return 0;
+}
+
+void exit_swap_address_space(unsigned int type)
+{
+	struct address_space *spaces;
+
+	spaces = swapper_spaces[type];
+	nr_swapper_spaces[type] = 0;
+	rcu_assign_pointer(swapper_spaces[type], NULL);
+	synchronize_rcu();
+	kvfree(spaces);
+}
diff --git a/mm/swapfile.c b/mm/swapfile.c
index c3ed37e..6f61fb3 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2075,6 +2075,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	vfree(frontswap_map);
 	/* Destroy swap account information */
 	swap_cgroup_swapoff(p->type);
+	exit_swap_address_space(p->type);
 
 	inode = mapping->host;
 	if (S_ISBLK(inode->i_mode)) {
@@ -2398,8 +2399,12 @@ static unsigned long read_swap_header(struct swap_info_struct *p,
 	return maxpages;
 }
 
-#define SWAP_CLUSTER_COLS						\
+#define SWAP_CLUSTER_INFO_COLS						\
 	DIV_ROUND_UP(L1_CACHE_BYTES, sizeof(struct swap_cluster_info))
+#define SWAP_CLUSTER_SPACE_COLS						\
+	DIV_ROUND_UP(SWAP_ADDRESS_SPACE_PAGES, SWAPFILE_CLUSTER)
+#define SWAP_CLUSTER_COLS						\
+	max_t(unsigned int, SWAP_CLUSTER_INFO_COLS, SWAP_CLUSTER_SPACE_COLS)
 
 static int setup_swap_map_and_extents(struct swap_info_struct *p,
 					union swap_header *swap_header,
@@ -2462,7 +2467,10 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p,
 		return nr_extents;
 
 
-	/* Reduce false cache line sharing between cluster_info */
+	/*
+	 * Reduce false cache line sharing between cluster_info and
+	 * sharing same address space.
+	 */
 	for (k = 0; k < SWAP_CLUSTER_COLS; k++) {
 		j = (k + col) % SWAP_CLUSTER_COLS;
 		for (i = 0; i < DIV_ROUND_UP(nr_clusters, SWAP_CLUSTER_COLS); i++) {
@@ -2643,6 +2651,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		}
 	}
 
+	error = init_swap_address_space(p->type, maxpages);
+	if (error)
+		goto bad_swap;
+
 	mutex_lock(&swapon_mutex);
 	prio = -1;
 	if (swap_flags & SWAP_FLAG_PREFER)
-- 
2.5.5

[toc] | [next] | [standalone]


#1557000

FromAndrew Morton <akpm@linux-foundation.org>
Date2017-01-12 00:10 +0100
Message-ID<sYAbM-4Ht-13@gated-at.bofh.it>
In reply to#1556783
On Wed, 11 Jan 2017 09:55:13 -0800 Tim Chen <tim.c.chen@linux.intel.com> wrote:

> The patch is to improve the scalability of the swap out/in via using
> fine grained locks for the swap cache.  In current kernel, one address
> space will be used for each swap device.  And in the common
> configuration, the number of the swap device is very small (one is
> typical).  This causes the heavy lock contention on the radix tree of
> the address space if multiple tasks swap out/in concurrently.  But in
> fact, there is no dependency between pages in the swap cache.  So that,
> we can split the one shared address space for each swap device into
> several address spaces to reduce the lock contention.  In the patch, the
> shared address space is split into 64MB trunks.  64MB is chosen to
> balance the memory space usage and effect of lock contention reduction.
> 
> The size of struct address_space on x86_64 architecture is 408B, so with
> the patch, 6528B more memory will be used for every 1GB swap space on
> x86_64 architecture.
> 
> One address space is still shared for the swap entries in the same 64M
> trunks.  To avoid lock contention for the first round of swap space
> allocation, the order of the swap clusters in the initial free clusters
> list is changed.  The swap space distance between the consecutive swap
> clusters in the free cluster list is at least 64M.  After the first
> round of allocation, the swap clusters are expected to be freed
> randomly, so the lock contention should be reduced effectively.

Switching from a single radix-tree to an array of radix-trees to reduce
contention seems a bit hacky.  That we can do this and have everything
continue to work tells me that we're simply using an inappropriate data
structure to hold this info.

[toc] | [prev] | [next] | [standalone]


#1557003

FromAndi Kleen <ak@linux.intel.com>
Date2017-01-12 00:20 +0100
Message-ID<sYAlr-4Mp-1@gated-at.bofh.it>
In reply to#1557000
> Switching from a single radix-tree to an array of radix-trees to reduce
> contention seems a bit hacky.  That we can do this and have everything
> continue to work tells me that we're simply using an inappropriate data
> structure to hold this info.

What would you use instead?

A tree with fine grained locking?

FWIW too fine grained locking (e.g. on every node) is usually a bad idea: 

it slows down the single thread performance and it causes much more overhead
when there is actual contention because too much time is spent bouncing cache
lines around.

So I actually like the "a little bit more fine grained, but not too much"
approach.

Or a hash table? 

Not sure if this would work here.

-Andi

[toc] | [prev] | [next] | [standalone]


#1557606

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-01-12 17:50 +0100
Message-ID<sYQJA-6gs-21@gated-at.bofh.it>
In reply to#1557003

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jan 11, 2017 at 03:19:37PM -0800, Andi Kleen wrote:
> > Switching from a single radix-tree to an array of radix-trees to reduce
> > contention seems a bit hacky.  That we can do this and have everything
> > continue to work tells me that we're simply using an inappropriate data
> > structure to hold this info.
> 
> What would you use instead?

I agree that this approach is a bit hacky.  However, it is pretty
effective and simple.  If later on we come up with a better solution to
scale modfication of the radix tree, we can collapse the radix trees.

I think developing a scalable radix tree with write modifications will
take quite a while and is a non-trivial effort. With almost memory speed
SSDs coming on the market soon, I think having a workable solution now
and optimizing it for long term is reasonabale.

Tim

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web