Skip to contentSkip to conversation

swap: make each swap partition have one address_space

PATCH In review Shaohua Li 3 participants 3 messages Dec 10, 2012 – Jan 17, 2013

Subsystems (7)
Memory management MM core Proc filesystem Memory mapping +3 more subsystems
lore
Share & tools
lore

Shaohua Li

author
#0 lore raw

PATCH 1/2 swap: make each swap partition have one address_space

When I use several fast SSD to do swap, swapper_space.tree_lock is heavily contended. This makes each swap partition have one address_space to reduce the lock contention. There is an array of address_space for swap. The swap entry type is the index to the array.

In my test with 3 SSD, this increases the swapout throughput 20%.

There are some code here which looks unnecessary, for example, moving some code from swapops.h to swap.h and soem changes in audit_tree.c. Those are to make the code compile.

Signed-off-by Shaohua Li <[email protected]>
---
 fs/proc/meminfo.c       |    4 +--
 include/linux/mm.h      |   11 ++++++---
 include/linux/swap.h    |   55 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/swapops.h |   46 ----------------------------------------
 kernel/audit_tree.c     |   30 +++++++++++++-------------
 mm/memcontrol.c         |    4 +--
 mm/mincore.c            |    5 ++--
 mm/swap.c               |    7 +++++-
 mm/swap_state.c         |   52 +++++++++++++++++++++++++++++----------------
 mm/swapfile.c           |    5 ++--
 10 files changed, 126 insertions(+), 93 deletions(-)
include/linux/mm.h +7 -4
Index: linux/include/linux/mm.h
===================================================================
@@ -17,6 +17,7 @@
 #include <linux/pfn.h>
 #include <linux/bit_spinlock.h>
 #include <linux/shrinker.h>
+#include <linux/swap.h>
 
 struct mempolicy;
 struct anon_vma;

@@ -788,15 +789,17 @@ void page_address_init(void);
 #define PAGE_MAPPING_KSM	2
 #define PAGE_MAPPING_FLAGS	(PAGE_MAPPING_ANON | PAGE_MAPPING_KSM)
 
-extern struct address_space swapper_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
 	VM_BUG_ON(PageSlab(page));
-	if (unlikely(PageSwapCache(page)))
-		mapping = &swapper_space;
-	else if ((unsigned long)mapping & PAGE_MAPPING_ANON)
+	if (unlikely(PageSwapCache(page))) {
+		swp_entry_t entry;
+
+		entry.val = page_private(page);
+		mapping = swap_address_space(entry);
+	} else if ((unsigned long)mapping & PAGE_MAPPING_ANON)
 		mapping = NULL;
 	return mapping;
 }
include/linux/swap.h +53 -2

73 lines not on this page · raw message

mm/memcontrol.c +2 -2
Index: linux/mm/memcontrol.c
===================================================================
@@ -5166,7 +5166,7 @@ static struct page *mc_handle_swap_pte(s
 	 * Because lookup_swap_cache() updates some statistics counter,
 	 * we call find_get_page() with swapper_space directly.
 	 */
-	page = find_get_page(&swapper_space, ent.val);
+	page = find_get_page(swap_address_space(entry), ent.val);
 	if (do_swap_account)
 		entry->val = ent.val;
 

@@ -5207,7 +5207,7 @@ static struct page *mc_handle_file_pte(s
 		swp_entry_t swap = radix_to_swp_entry(page);
 		if (do_swap_account)
 			*entry = swap;
-		page = find_get_page(&swapper_space, swap.val);
+		page = find_get_page(swap_address_space(swap), swap.val);
 	}
 #endif
 	return page;
mm/mincore.c +3 -2
Index: linux/mm/mincore.c
===================================================================
@@ -75,7 +75,7 @@ static unsigned char mincore_page(struct
 	/* shmem/tmpfs may return swap: account for swapcache page too. */
 	if (radix_tree_exceptional_entry(page)) {
 		swp_entry_t swap = radix_to_swp_entry(page);
-		page = find_get_page(&swapper_space, swap.val);
+		page = find_get_page(swap_address_space(swap), swap.val);
 	}
 #endif
 	if (page) {

@@ -135,7 +135,8 @@ static void mincore_pte_range(struct vm_
 			} else {
 #ifdef CONFIG_SWAP
 				pgoff = entry.val;
-				*vec = mincore_page(&swapper_space, pgoff);
+				*vec = mincore_page(swap_address_space(entry),
+					pgoff);
 #else
 				WARN_ON(1);
 				*vec = 1;
mm/swap.c +6 -1
Index: linux/mm/swap.c
===================================================================
@@ -855,9 +855,14 @@ EXPORT_SYMBOL(pagevec_lookup_tag);
 void __init swap_setup(void)
 {
 	unsigned long megs = totalram_pages >> (20 - PAGE_SHIFT);
+	int i;
 
 #ifdef CONFIG_SWAP
-	bdi_init(swapper_space.backing_dev_info);
+	for (i = 0; i < MAX_SWAPFILES; i++) {
+		bdi_init(swapper_spaces[i].backing_dev_info);
+		spin_lock_init(&swapper_spaces[i].tree_lock);
+		INIT_LIST_HEAD(&swapper_spaces[i].i_mmap_nonlinear);
+	}
 #endif
 
 	/* Use a smaller cluster for small-memory machines */
mm/swap_state.c +34 -18

109 lines not on this page · raw message

mm/swapfile.c +3 -2
Index: linux/mm/swapfile.c
===================================================================
@@ -79,7 +79,7 @@ __try_to_reclaim_swap(struct swap_info_s
 	struct page *page;
 	int ret = 0;
 
-	page = find_get_page(&swapper_space, entry.val);
+	page = find_get_page(swap_address_space(entry), entry.val);
 	if (!page)
 		return 0;
 	/*

@@ -699,7 +699,8 @@ int free_swap_and_cache(swp_entry_t entr
 	p = swap_info_get(entry);
 	if (p) {
 		if (swap_entry_free(p, entry, 1) == SWAP_HAS_CACHE) {
-			page = find_get_page(&swapper_space, entry.val);
+			page = find_get_page(swap_address_space(entry),
+						entry.val);
 			if (page && !trylock_page(page)) {
 				page_cache_release(page);
 				page = NULL;
fs/proc/meminfo.c +2 -2
Index: linux/fs/proc/meminfo.c
===================================================================
@@ -40,7 +40,7 @@ static int meminfo_proc_show(struct seq_
 		* sysctl_overcommit_ratio / 100) + total_swap_pages;
 
 	cached = global_page_state(NR_FILE_PAGES) -
-			total_swapcache_pages - i.bufferram;
+			total_swapcache_pages() - i.bufferram;
 	if (cached < 0)
 		cached = 0;
 

@@ -109,7 +109,7 @@ static int meminfo_proc_show(struct seq_
 		K(i.freeram),
 		K(i.bufferram),
 		K(cached),
-		K(total_swapcache_pages),
+		K(total_swapcache_pages()),
 		K(pages[LRU_ACTIVE_ANON]   + pages[LRU_ACTIVE_FILE]),
 		K(pages[LRU_INACTIVE_ANON] + pages[LRU_INACTIVE_FILE]),
 		K(pages[LRU_ACTIVE_ANON]),
kernel/audit_tree.c +16 -14
Index: linux/kernel/audit_tree.c
===================================================================
@@ -28,7 +28,7 @@ struct audit_chunk {
 	int count;
 	atomic_long_t refs;
 	struct rcu_head head;
-	struct node {
+	struct audit_node {
 		struct list_head list;
 		struct audit_tree *owner;
 		unsigned index;		/* index; upper bit indicates 'will prune' */

@@ -62,7 +62,7 @@ static LIST_HEAD(prune_list);
  * chunk is refcounted by embedded fsnotify_mark + .refs (non-zero refcount
  * of watch contributes 1 to .refs).
  *
- * node.index allows to get from node.list to containing chunk.
+ * audit_node.index allows to get from audit_node.list to containing chunk.
  * MSB of that sucker is stolen to mark taggings that we might have to
  * revert - several operations have very unpleasant cleanup logics and
  * that makes a difference.  Some.

@@ -140,7 +140,8 @@ static struct audit_chunk *alloc_chunk(i
 	size_t size;
 	int i;
 
-	size = offsetof(struct audit_chunk, owners) + count * sizeof(struct node);
+	size = offsetof(struct audit_chunk, owners) +
+			count * sizeof(struct audit_node);
 	chunk = kzalloc(size, GFP_KERNEL);
 	if (!chunk)
 		return NULL;

@@ -206,14 +207,14 @@ int audit_tree_match(struct audit_chunk
 
 /* tagging and untagging inodes with trees */
 
-static struct audit_chunk *find_chunk(struct node *p)
+static struct audit_chunk *find_chunk(struct audit_node *p)
 {
 	int index = p->index & ~(1U<<31);
 	p -= index;
 	return container_of(p, struct audit_chunk, owners[0]);
 }
 
-static void untag_chunk(struct node *p)
+static void untag_chunk(struct audit_node *p)
 {
 	struct audit_chunk *chunk = find_chunk(p);
 	struct fsnotify_mark *entry = &chunk->mark;

@@ -356,7 +357,7 @@ static int tag_chunk(struct inode *inode
 	struct fsnotify_mark *old_entry, *chunk_entry;
 	struct audit_tree *owner;
 	struct audit_chunk *chunk, *old;
-	struct node *p;
+	struct audit_node *p;
 	int n;
 
 	old_entry = fsnotify_find_inode_mark(audit_tree_group, inode);

@@ -484,9 +485,9 @@ static void prune_one(struct audit_tree
 {
 	spin_lock(&hash_lock);
 	while (!list_empty(&victim->chunks)) {
-		struct node *p;
+		struct audit_node *p;
 
-		p = list_entry(victim->chunks.next, struct node, list);
+		p = list_entry(victim->chunks.next, struct audit_node, list);
 
 		untag_chunk(p);
 	}

@@ -506,7 +507,8 @@ static void trim_marked(struct audit_tre
 	}
 	/* reorder */
 	for (p = tree->chunks.next; p != &tree->chunks; p = q) {
-		struct node *node = list_entry(p, struct node, list);
+		struct audit_node *node = list_entry(p, struct audit_node,
+						list);
 		q = p->next;
 		if (node->index & (1U<<31)) {
 			list_del_init(p);

@@ -515,9 +517,9 @@ static void trim_marked(struct audit_tre
 	}
 
 	while (!list_empty(&tree->chunks)) {
-		struct node *node;
+		struct audit_node *node;
 
-		node = list_entry(tree->chunks.next, struct node, list);
+		node = list_entry(tree->chunks.next, struct audit_node, list);
 
 		/* have we run out of marked? */
 		if (!(node->index & (1U<<31)))

@@ -580,7 +582,7 @@ void audit_trim_trees(void)
 		struct audit_tree *tree;
 		struct path path;
 		struct vfsmount *root_mnt;
-		struct node *node;
+		struct audit_node *node;
 		int err;
 
 		tree = container_of(cursor.next, struct audit_tree, list);

@@ -679,7 +681,7 @@ int audit_add_tree_rule(struct audit_kru
 	drop_collected_mounts(mnt);
 
 	if (!err) {
-		struct node *node;
+		struct audit_node *node;
 		spin_lock(&hash_lock);
 		list_for_each_entry(node, &tree->chunks, list)
 			node->index &= ~(1U<<31);

@@ -781,7 +783,7 @@ int audit_tag_tree(char *old, char *new)
 		mutex_unlock(&audit_filter_mutex);
 
 		if (!failed) {
-			struct node *node;
+			struct audit_node *node;
 			spin_lock(&hash_lock);
 			list_for_each_entry(node, &tree->chunks, list)
 				node->index &= ~(1U<<31);
include/linux/swapops.h +0 -46

52 lines not on this page · raw message

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

Reviewed-by on v1 1/2
#1 lore raw
Quoted 5 lines · Shaohua Li

When I use several fast SSD to do swap, swapper_space.tree_lock is heavily contended. This makes each swap partition have one address_space to reduce the lock contention. There is an array of address_space for swap. The swap entry type is the index to the array.

Quoted 7 lines

In my test with 3 SSD, this increases the swapout throughput 20%.

There are some code here which looks unnecessary, for example, moving some code from swapops.h to swap.h and soem changes in audit_tree.c. Those are to make the code compile.

Signed-off-by Shaohua Li <[email protected]>
Reviewed-by Rik van Riel <[email protected]>

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

maintainer
#2 lore raw
In reply to Shaohua Li
Quoted 11 lines · On Mon, 10 Dec 2012 09:24:39 +0800 Shaohua Li <[email protected]> wrote:

When I use several fast SSD to do swap, swapper_space.tree_lock is heavily contended. This makes each swap partition have one address_space to reduce the lock contention. There is an array of address_space for swap. The swap entry type is the index to the array.

In my test with 3 SSD, this increases the swapout throughput 20%.

There are some code here which looks unnecessary, for example, moving some code from swapops.h to swap.h and soem changes in audit_tree.c. Those are to make the code compile.

Although it appears to be pretty mechanical, the patch is rather hard to read due to these irrelevancies. Can it be split up a bit? Surely the audit_tree rename can be done in a preparatory change.

The patches have bitrotted and there's a build error in memcontrol.c (s/entry/*entry).

Refresh, retest and resend, please?

Quoted 17 lines
include/linux/mm.h +0 -3
 
...

@@ -788,15 +789,17 @@ void page_address_init(void);
 #define PAGE_MAPPING_KSM	2
 #define PAGE_MAPPING_FLAGS	(PAGE_MAPPING_ANON | PAGE_MAPPING_KSM)
 
-extern struct address_space swapper_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
 	VM_BUG_ON(PageSlab(page));
-	if (unlikely(PageSwapCache(page)))
-		mapping = &swapper_space;

include/linux/mm.h void page_address_init(void); · line 805

Quoted 10 lines
include/linux/mm.h +6 -1
-	else if ((unsigned long)mapping & PAGE_MAPPING_ANON)
+	if (unlikely(PageSwapCache(page))) {
+		swp_entry_t entry;
+
+		entry.val = page_private(page);
+		mapping = swap_address_space(entry);
+	} else if ((unsigned long)mapping & PAGE_MAPPING_ANON)
 		mapping = NULL;
 	return mapping;
 }

I think that's kinda the last straw for page_mapping(). A quick test here indicates that uninlining page_mapping() saves half a k of text from mm/built-in.o. Wanna include this as #2 in the patch series, please?

mm/util.c renamed from mm/util.c~a +15 -0
@@ -382,6 +382,21 @@ unsigned long vm_mmap(struct file *file,
 }
 EXPORT_SYMBOL(vm_mmap);
 
+struct address_space *page_mapping(struct page *page)
+{
+	struct address_space *mapping = page->mapping;
+
+	VM_BUG_ON(PageSlab(page));
+	if (unlikely(PageSwapCache(page))) {
+		swp_entry_t entry;
+
+		entry.val = page_private(page);
+		mapping = swap_address_space(entry);
+	} else if ((unsigned long)mapping & PAGE_MAPPING_ANON)
+		mapping = NULL;
+	return mapping;
+}
+
 /* Tracepoints definitions. */
 EXPORT_TRACEPOINT_SYMBOL(kmalloc);
 EXPORT_TRACEPOINT_SYMBOL(kmem_cache_alloc);
include/linux/mm.h renamed from include/linux/mm.h~a +1 -14
diff -puN include/linux/mm.h~a include/linux/mm.h
@@ -819,20 +819,7 @@ void page_address_init(void);
 #define PAGE_MAPPING_KSM	2
 #define PAGE_MAPPING_FLAGS	(PAGE_MAPPING_ANON | PAGE_MAPPING_KSM)
 
-static inline struct address_space *page_mapping(struct page *page)
-{
-	struct address_space *mapping = page->mapping;
-
-	VM_BUG_ON(PageSlab(page));
-	if (unlikely(PageSwapCache(page))) {
-		swp_entry_t entry;
-
-		entry.val = page_private(page);
-		mapping = swap_address_space(entry);
-	} else if ((unsigned long)mapping & PAGE_MAPPING_ANON)
-		mapping = NULL;
-	return mapping;
-}
+extern struct address_space *page_mapping(struct page *page);
 
 /* Neutral page->mapping pointer to address_space or anon_vma or other */
 static inline void *page_rmapping(struct page *page)
Quoted 21 lines

...

void show_swap_cache_info(void)
{

include/linux/mm.h +3 -4
-	printk("%lu pages in swap cache\n", total_swapcache_pages);
+	printk("%lu pages in swap cache\n", total_swapcache_pages());
 	printk("Swap cache stats: add %lu, delete %lu, find %lu/%lu\n",
 		swap_cache_info.add_total, swap_cache_info.del_total,
 		swap_cache_info.find_success, swap_cache_info.find_total);

@@ -76,17 +86,18 @@ static int __add_to_swap_cache(struct pa
 	VM_BUG_ON(!PageSwapBacked(page));
 
 	page_cache_get(page);
-	SetPageSwapCache(page);
 	set_page_private(page, entry.val);
+	SetPageSwapCache(page);
 
-	spin_lock_irq(&swapper_space.tree_lock);
-	error = radix_tree_insert(&swapper_space.page_tree, entry.val, page);
+	spin_lock_irq(&swap_address_space(entry)->tree_lock);

include/linux/mm.h static int __add_to_swap_cache(struct pa · line 100

Quoted 10 lines
include/linux/mm.h +4 -2
+	error = radix_tree_insert(&swap_address_space(entry)->page_tree,
+					entry.val, page);
 	if (likely(!error)) {
-		total_swapcache_pages++;
+		swap_address_space(entry)->nrpages++;
 		__inc_zone_page_state(page, NR_FILE_PAGES);
 		INC_CACHE_INFO(add_total);
 	}
-	spin_unlock_irq(&swapper_space.tree_lock);
+	spin_unlock_irq(&swap_address_space(entry)->tree_lock);

Four separate evaluations of swap_address_space(entry). The compiler *should* save the result in a temporary and avoid this, but please check this. If it doesn't, do it manually. Here and in several other places.

include/linux/mm.h static int __add_to_swap_cache(struct pa · line 103

Quoted 5 lines
include/linux/mm.h +0 -0
 	if (unlikely(error)) {
 		/*
 
...

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