On Mon, 10 Dec 2012 09:24:39 +0800
Shaohua Li <shli@xxxxxxxxxx> 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?
> --- linux.orig/include/linux/mm.h 2012-12-10 08:51:21.809919763 +0800
> +++ linux/include/linux/mm.h 2012-12-10 09:02:45.029330611 +0800
>
> ...
>
> @@ -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;
> }
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?
--- a/mm/util.c~a
+++ a/mm/util.c
@@ -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);
diff -puN include/linux/mm.h~a include/linux/mm.h
--- a/include/linux/mm.h~a
+++ 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)
>
> ...
>
> void show_swap_cache_info(void)
> {
> - 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);
> + 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.
> if (unlikely(error)) {
> /*
>
> ...
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>