Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCHv11 3/4] zswap: add to mm/

 

 

First page Previous page 1 2 Next page Last page  View All Linux kernel RSS feed   Index | Next | Previous | View Threaded


sjenning at linux

May 13, 2013, 5:40 AM

Post #1 of 28 (623 views)
Permalink
[PATCHv11 3/4] zswap: add to mm/

zswap is a thin compression backend for frontswap. It receives pages from
frontswap and attempts to store them in a compressed memory pool, resulting in
an effective partial memory reclaim and dramatically reduced swap device I/O.

Additionally, in most cases, pages can be retrieved from this compressed store
much more quickly than reading from tradition swap devices resulting in faster
performance for many workloads.

It also has support for evicting swap pages that are currently compressed in
zswap to the swap device on an LRU(ish) basis. This functionality is very
important and make zswap a true cache in that, once the cache is full or can't
grow due to memory pressure, the oldest pages can be moved out of zswap to the
swap device so newer pages can be compressed and stored in zswap.

This patch adds the zswap driver to mm/

Signed-off-by: Seth Jennings <sjenning [at] linux>
---
mm/Kconfig | 15 +
mm/Makefile | 1 +
mm/zswap.c | 952 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 968 insertions(+)
create mode 100644 mm/zswap.c

diff --git a/mm/Kconfig b/mm/Kconfig
index 908f41b..4042e07 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -487,3 +487,18 @@ config ZBUD
While this design limits storage density, it has simple and
deterministic reclaim properties that make it preferable to a higher
density approach when reclaim will be used.
+
+config ZSWAP
+ bool "In-kernel swap page compression"
+ depends on FRONTSWAP && CRYPTO
+ select CRYPTO_LZO
+ select ZBUD
+ default n
+ help
+ Zswap is a backend for the frontswap mechanism in the VMM.
+ It receives pages from frontswap and attempts to store them
+ in a compressed memory pool, resulting in an effective
+ partial memory reclaim. In addition, pages and be retrieved
+ from this compressed store much faster than most tradition
+ swap devices resulting in reduced I/O and faster performance
+ for many workloads.
diff --git a/mm/Makefile b/mm/Makefile
index 95f0197..f008033 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
obj-$(CONFIG_BOUNCE) += bounce.o
obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o
obj-$(CONFIG_FRONTSWAP) += frontswap.o
+obj-$(CONFIG_ZSWAP) += zswap.o
obj-$(CONFIG_HAS_DMA) += dmapool.o
obj-$(CONFIG_HUGETLBFS) += hugetlb.o
obj-$(CONFIG_NUMA) += mempolicy.o
diff --git a/mm/zswap.c b/mm/zswap.c
new file mode 100644
index 0000000..b1070ca
--- /dev/null
+++ b/mm/zswap.c
@@ -0,0 +1,952 @@
+/*
+ * zswap.c - zswap driver file
+ *
+ * zswap is a backend for frontswap that takes pages that are in the
+ * process of being swapped out and attempts to compress them and store
+ * them in a RAM-based memory pool. This results in a significant I/O
+ * reduction on the real swap device and, in the case of a slow swap
+ * device, can also improve workload performance.
+ *
+ * Copyright (C) 2012 Seth Jennings <sjenning [at] linux>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/cpu.h>
+#include <linux/highmem.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/atomic.h>
+#include <linux/frontswap.h>
+#include <linux/rbtree.h>
+#include <linux/swap.h>
+#include <linux/crypto.h>
+#include <linux/mempool.h>
+#include <linux/zbud.h>
+
+#include <linux/mm_types.h>
+#include <linux/page-flags.h>
+#include <linux/swapops.h>
+#include <linux/writeback.h>
+#include <linux/pagemap.h>
+
+/*********************************
+* statistics
+**********************************/
+/* Number of memory pages used by the compressed pool */
+static atomic_t zswap_pool_pages = ATOMIC_INIT(0);
+/* The number of compressed pages currently stored in zswap */
+static atomic_t zswap_stored_pages = ATOMIC_INIT(0);
+
+/*
+ * The statistics below are not protected from concurrent access for
+ * performance reasons so they may not be a 100% accurate. However,
+ * they do provide useful information on roughly how many times a
+ * certain event is occurring.
+*/
+static u64 zswap_pool_limit_hit;
+static u64 zswap_written_back_pages;
+static u64 zswap_reject_reclaim_fail;
+static u64 zswap_reject_compress_poor;
+static u64 zswap_reject_alloc_fail;
+static u64 zswap_reject_kmemcache_fail;
+static u64 zswap_duplicate_entry;
+
+/*********************************
+* tunables
+**********************************/
+/* Enable/disable zswap (disabled by default, fixed at boot for now) */
+static bool zswap_enabled;
+module_param_named(enabled, zswap_enabled, bool, 0);
+
+/* Compressor to be used by zswap (fixed at boot for now) */
+#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
+static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
+module_param_named(compressor, zswap_compressor, charp, 0);
+
+/* The maximum percentage of memory that the compressed pool can occupy */
+static unsigned int zswap_max_pool_percent = 20;
+module_param_named(max_pool_percent,
+ zswap_max_pool_percent, uint, 0644);
+
+/*
+ * Maximum compression ratio, as as percentage, for an acceptable
+ * compressed page. Any pages that do not compress by at least
+ * this ratio will be rejected.
+*/
+static unsigned int zswap_max_compression_ratio = 80;
+module_param_named(max_compression_ratio,
+ zswap_max_compression_ratio, uint, 0644);
+
+/*********************************
+* compression functions
+**********************************/
+/* per-cpu compression transforms */
+static struct crypto_comp * __percpu *zswap_comp_pcpu_tfms;
+
+enum comp_op {
+ ZSWAP_COMPOP_COMPRESS,
+ ZSWAP_COMPOP_DECOMPRESS
+};
+
+static int zswap_comp_op(enum comp_op op, const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ struct crypto_comp *tfm;
+ int ret;
+
+ tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, get_cpu());
+ switch (op) {
+ case ZSWAP_COMPOP_COMPRESS:
+ ret = crypto_comp_compress(tfm, src, slen, dst, dlen);
+ break;
+ case ZSWAP_COMPOP_DECOMPRESS:
+ ret = crypto_comp_decompress(tfm, src, slen, dst, dlen);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ put_cpu();
+ return ret;
+}
+
+static int __init zswap_comp_init(void)
+{
+ if (!crypto_has_comp(zswap_compressor, 0, 0)) {
+ pr_info("%s compressor not available\n", zswap_compressor);
+ /* fall back to default compressor */
+ zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
+ if (!crypto_has_comp(zswap_compressor, 0, 0))
+ /* can't even load the default compressor */
+ return -ENODEV;
+ }
+ pr_info("using %s compressor\n", zswap_compressor);
+
+ /* alloc percpu transforms */
+ zswap_comp_pcpu_tfms = alloc_percpu(struct crypto_comp *);
+ if (!zswap_comp_pcpu_tfms)
+ return -ENOMEM;
+ return 0;
+}
+
+static void zswap_comp_exit(void)
+{
+ /* free percpu transforms */
+ if (zswap_comp_pcpu_tfms)
+ free_percpu(zswap_comp_pcpu_tfms);
+}
+
+/*********************************
+* data structures
+**********************************/
+/*
+ * struct zswap_entry
+ *
+ * This structure contains the metadata for tracking a single compressed
+ * page within zswap.
+ *
+ * rbnode - links the entry into red-black tree for the appropriate swap type
+ * refcount - the number of outstanding reference to the entry. This is needed
+ * to protect against premature freeing of the entry by code
+ * concurent calls to load, invalidate, and writeback. The lock
+ * for the zswap_tree structure that contains the entry must
+ * be held while changing the refcount. Since the lock must
+ * be held, there is no reason to also make refcount atomic.
+ * type - the swap type for the entry. Used to map back to the zswap_tree
+ * structure that contains the entry.
+ * offset - the swap offset for the entry. Index into the red-black tree.
+ * handle - zsmalloc allocation handle that stores the compressed page data
+ * length - the length in bytes of the compressed page data. Needed during
+ * decompression
+ */
+struct zswap_entry {
+ struct rb_node rbnode;
+ pgoff_t offset;
+ int refcount;
+ unsigned int length;
+ unsigned long handle;
+};
+
+struct zswap_header {
+ swp_entry_t swpentry;
+};
+
+/*
+ * The tree lock in the zswap_tree struct protects a few things:
+ * - the rbtree
+ * - the refcount field of each entry in the tree
+ */
+struct zswap_tree {
+ struct rb_root rbroot;
+ spinlock_t lock;
+ struct zbud_pool *pool;
+ unsigned type;
+};
+
+static struct zswap_tree *zswap_trees[MAX_SWAPFILES];
+
+/*********************************
+* zswap entry functions
+**********************************/
+#define ZSWAP_KMEM_CACHE_NAME "zswap_entry_cache"
+static struct kmem_cache *zswap_entry_cache;
+
+static inline int zswap_entry_cache_create(void)
+{
+ zswap_entry_cache =
+ kmem_cache_create(ZSWAP_KMEM_CACHE_NAME,
+ sizeof(struct zswap_entry), 0, 0, NULL);
+ return (zswap_entry_cache == NULL);
+}
+
+static inline void zswap_entry_cache_destory(void)
+{
+ kmem_cache_destroy(zswap_entry_cache);
+}
+
+static inline struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp)
+{
+ struct zswap_entry *entry;
+ entry = kmem_cache_alloc(zswap_entry_cache, gfp);
+ if (!entry)
+ return NULL;
+ entry->refcount = 1;
+ return entry;
+}
+
+static inline void zswap_entry_cache_free(struct zswap_entry *entry)
+{
+ kmem_cache_free(zswap_entry_cache, entry);
+}
+
+static inline void zswap_entry_get(struct zswap_entry *entry)
+{
+ entry->refcount++;
+}
+
+static inline int zswap_entry_put(struct zswap_entry *entry)
+{
+ entry->refcount--;
+ return entry->refcount;
+}
+
+/*********************************
+* rbtree functions
+**********************************/
+static struct zswap_entry *zswap_rb_search(struct rb_root *root, pgoff_t offset)
+{
+ struct rb_node *node = root->rb_node;
+ struct zswap_entry *entry;
+
+ while (node) {
+ entry = rb_entry(node, struct zswap_entry, rbnode);
+ if (entry->offset > offset)
+ node = node->rb_left;
+ else if (entry->offset < offset)
+ node = node->rb_right;
+ else
+ return entry;
+ }
+ return NULL;
+}
+
+/*
+ * In the case that a entry with the same offset is found, it a pointer to
+ * the existing entry is stored in dupentry and the function returns -EEXIST
+*/
+static int zswap_rb_insert(struct rb_root *root, struct zswap_entry *entry,
+ struct zswap_entry **dupentry)
+{
+ struct rb_node **link = &root->rb_node, *parent = NULL;
+ struct zswap_entry *myentry;
+
+ while (*link) {
+ parent = *link;
+ myentry = rb_entry(parent, struct zswap_entry, rbnode);
+ if (myentry->offset > entry->offset)
+ link = &(*link)->rb_left;
+ else if (myentry->offset < entry->offset)
+ link = &(*link)->rb_right;
+ else {
+ *dupentry = myentry;
+ return -EEXIST;
+ }
+ }
+ rb_link_node(&entry->rbnode, parent, link);
+ rb_insert_color(&entry->rbnode, root);
+ return 0;
+}
+
+/*********************************
+* per-cpu code
+**********************************/
+static DEFINE_PER_CPU(u8 *, zswap_dstmem);
+
+static int __zswap_cpu_notifier(unsigned long action, unsigned long cpu)
+{
+ struct crypto_comp *tfm;
+ u8 *dst;
+
+ switch (action) {
+ case CPU_UP_PREPARE:
+ tfm = crypto_alloc_comp(zswap_compressor, 0, 0);
+ if (IS_ERR(tfm)) {
+ pr_err("can't allocate compressor transform\n");
+ return NOTIFY_BAD;
+ }
+ *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = tfm;
+ dst = kmalloc(PAGE_SIZE * 2, GFP_KERNEL);
+ if (!dst) {
+ pr_err("can't allocate compressor buffer\n");
+ crypto_free_comp(tfm);
+ *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
+ return NOTIFY_BAD;
+ }
+ per_cpu(zswap_dstmem, cpu) = dst;
+ break;
+ case CPU_DEAD:
+ case CPU_UP_CANCELED:
+ tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu);
+ if (tfm) {
+ crypto_free_comp(tfm);
+ *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
+ }
+ dst = per_cpu(zswap_dstmem, cpu);
+ kfree(dst);
+ per_cpu(zswap_dstmem, cpu) = NULL;
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static int zswap_cpu_notifier(struct notifier_block *nb,
+ unsigned long action, void *pcpu)
+{
+ unsigned long cpu = (unsigned long)pcpu;
+ return __zswap_cpu_notifier(action, cpu);
+}
+
+static struct notifier_block zswap_cpu_notifier_block = {
+ .notifier_call = zswap_cpu_notifier
+};
+
+static int zswap_cpu_init(void)
+{
+ unsigned long cpu;
+
+ get_online_cpus();
+ for_each_online_cpu(cpu)
+ if (__zswap_cpu_notifier(CPU_UP_PREPARE, cpu) != NOTIFY_OK)
+ goto cleanup;
+ register_cpu_notifier(&zswap_cpu_notifier_block);
+ put_online_cpus();
+ return 0;
+
+cleanup:
+ for_each_online_cpu(cpu)
+ __zswap_cpu_notifier(CPU_UP_CANCELED, cpu);
+ put_online_cpus();
+ return -ENOMEM;
+}
+
+/*********************************
+* helpers
+**********************************/
+static inline bool zswap_is_full(void)
+{
+ int pool_pages = atomic_read(&zswap_pool_pages);
+ return (totalram_pages * zswap_max_pool_percent / 100 < pool_pages);
+}
+
+/*
+ * Carries out the common pattern of freeing and entry's zsmalloc allocation,
+ * freeing the entry itself, and decrementing the number of stored pages.
+ */
+static void zswap_free_entry(struct zswap_tree *tree, struct zswap_entry *entry)
+{
+ zbud_free(tree->pool, entry->handle);
+ zswap_entry_cache_free(entry);
+ atomic_dec(&zswap_stored_pages);
+ atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
+}
+
+/*********************************
+* writeback code
+**********************************/
+/* return enum for zswap_get_swap_cache_page */
+enum zswap_get_swap_ret {
+ ZSWAP_SWAPCACHE_NEW,
+ ZSWAP_SWAPCACHE_EXIST,
+ ZSWAP_SWAPCACHE_NOMEM
+};
+
+/*
+ * zswap_get_swap_cache_page
+ *
+ * This is an adaption of read_swap_cache_async()
+ *
+ * This function tries to find a page with the given swap entry
+ * in the swapper_space address space (the swap cache). If the page
+ * is found, it is returned in retpage. Otherwise, a page is allocated,
+ * added to the swap cache, and returned in retpage.
+ *
+ * If success, the swap cache page is returned in retpage
+ * Returns 0 if page was already in the swap cache, page is not locked
+ * Returns 1 if the new page needs to be populated, page is locked
+ * Returns <0 on error
+ */
+static int zswap_get_swap_cache_page(swp_entry_t entry,
+ struct page **retpage)
+{
+ struct page *found_page, *new_page = NULL;
+ struct address_space *swapper_space = &swapper_spaces[swp_type(entry)];
+ int err;
+
+ *retpage = NULL;
+ do {
+ /*
+ * First check the swap cache. Since this is normally
+ * called after lookup_swap_cache() failed, re-calling
+ * that would confuse statistics.
+ */
+ found_page = find_get_page(swapper_space, entry.val);
+ if (found_page)
+ break;
+
+ /*
+ * Get a new page to read into from swap.
+ */
+ if (!new_page) {
+ new_page = alloc_page(GFP_KERNEL);
+ if (!new_page)
+ break; /* Out of memory */
+ }
+
+ /*
+ * call radix_tree_preload() while we can wait.
+ */
+ err = radix_tree_preload(GFP_KERNEL);
+ if (err)
+ break;
+
+ /*
+ * Swap entry may have been freed since our caller observed it.
+ */
+ err = swapcache_prepare(entry);
+ if (err == -EEXIST) { /* seems racy */
+ radix_tree_preload_end();
+ continue;
+ }
+ if (err) { /* swp entry is obsolete ? */
+ radix_tree_preload_end();
+ break;
+ }
+
+ /* May fail (-ENOMEM) if radix-tree node allocation failed. */
+ __set_page_locked(new_page);
+ SetPageSwapBacked(new_page);
+ err = __add_to_swap_cache(new_page, entry);
+ if (likely(!err)) {
+ radix_tree_preload_end();
+ lru_cache_add_anon(new_page);
+ *retpage = new_page;
+ return ZSWAP_SWAPCACHE_NEW;
+ }
+ radix_tree_preload_end();
+ ClearPageSwapBacked(new_page);
+ __clear_page_locked(new_page);
+ /*
+ * add_to_swap_cache() doesn't return -EEXIST, so we can safely
+ * clear SWAP_HAS_CACHE flag.
+ */
+ swapcache_free(entry, NULL);
+ } while (err != -ENOMEM);
+
+ if (new_page)
+ page_cache_release(new_page);
+ if (!found_page)
+ return ZSWAP_SWAPCACHE_NOMEM;
+ *retpage = found_page;
+ return ZSWAP_SWAPCACHE_EXIST;
+}
+
+/*
+ * Attempts to free and entry by adding a page to the swap cache,
+ * decompressing the entry data into the page, and issuing a
+ * bio write to write the page back to the swap device.
+ *
+ * This can be thought of as a "resumed writeback" of the page
+ * to the swap device. We are basically resuming the same swap
+ * writeback path that was intercepted with the frontswap_store()
+ * in the first place. After the page has been decompressed into
+ * the swap cache, the compressed version stored by zswap can be
+ * freed.
+ */
+static int zswap_writeback_entry(struct zbud_pool *pool, unsigned long handle)
+{
+ struct zswap_header *zhdr;
+ swp_entry_t swpentry;
+ struct zswap_tree *tree;
+ pgoff_t offset;
+ struct zswap_entry *entry;
+ struct page *page;
+ u8 *src, *dst;
+ unsigned int dlen;
+ int ret, refcount;
+ struct writeback_control wbc = {
+ .sync_mode = WB_SYNC_NONE,
+ };
+
+ /* extract swpentry from data */
+ zhdr = zbud_map(pool, handle);
+ swpentry = zhdr->swpentry; /* here */
+ zbud_unmap(pool, handle);
+ tree = zswap_trees[swp_type(swpentry)];
+ offset = swp_offset(swpentry);
+ BUG_ON(pool != tree->pool);
+
+ /* find and ref zswap entry */
+ spin_lock(&tree->lock);
+ entry = zswap_rb_search(&tree->rbroot, offset);
+ if (!entry) {
+ /* entry was invalidated */
+ spin_unlock(&tree->lock);
+ return 0;
+ }
+ zswap_entry_get(entry);
+ spin_unlock(&tree->lock);
+ BUG_ON(offset != entry->offset);
+
+ /* try to allocate swap cache page */
+ switch (zswap_get_swap_cache_page(swpentry, &page)) {
+ case ZSWAP_SWAPCACHE_NOMEM: /* no memory */
+ ret = -ENOMEM;
+ goto fail;
+
+ case ZSWAP_SWAPCACHE_EXIST: /* page is unlocked */
+ /* page is already in the swap cache, ignore for now */
+ page_cache_release(page);
+ ret = -EEXIST;
+ goto fail;
+
+ case ZSWAP_SWAPCACHE_NEW: /* page is locked */
+ /* decompress */
+ dlen = PAGE_SIZE;
+ src = (u8 *)zbud_map(tree->pool, entry->handle) +
+ sizeof(struct zswap_header);
+ dst = kmap_atomic(page);
+ ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src,
+ entry->length, dst, &dlen);
+ kunmap_atomic(dst);
+ zbud_unmap(tree->pool, entry->handle);
+ BUG_ON(ret);
+ BUG_ON(dlen != PAGE_SIZE);
+
+ /* page is up to date */
+ SetPageUptodate(page);
+ }
+
+ /* start writeback */
+ SetPageReclaim(page);
+ __swap_writepage(page, &wbc, end_swap_bio_write);
+ page_cache_release(page);
+ zswap_written_back_pages++;
+
+ spin_lock(&tree->lock);
+
+ /* drop local reference */
+ zswap_entry_put(entry);
+ /* drop the initial reference from entry creation */
+ refcount = zswap_entry_put(entry);
+
+ /*
+ * There are three possible values for refcount here:
+ * (1) refcount is 1, load is in progress, unlink from rbtree,
+ * load will free
+ * (2) refcount is 0, (normal case) entry is valid,
+ * remove from rbtree and free entry
+ * (3) refcount is -1, invalidate happened during writeback,
+ * free entry
+ */
+ if (refcount >= 0) {
+ /* no invalidate yet, remove from rbtree */
+ rb_erase(&entry->rbnode, &tree->rbroot);
+ }
+ spin_unlock(&tree->lock);
+ if (refcount <= 0) {
+ /* free the entry */
+ zswap_free_entry(tree, entry);
+ return 0;
+ }
+ return -EAGAIN;
+
+fail:
+ spin_lock(&tree->lock);
+ zswap_entry_put(entry);
+ spin_unlock(&tree->lock);
+ return ret;
+}
+
+/*********************************
+* frontswap hooks
+**********************************/
+/* attempts to compress and store an single page */
+static int zswap_frontswap_store(unsigned type, pgoff_t offset,
+ struct page *page)
+{
+ struct zswap_tree *tree = zswap_trees[type];
+ struct zswap_entry *entry, *dupentry;
+ int ret;
+ unsigned int dlen = PAGE_SIZE, len;
+ unsigned long handle;
+ char *buf;
+ u8 *src, *dst;
+ struct zswap_header *zhdr;
+
+ if (!tree) {
+ ret = -ENODEV;
+ goto reject;
+ }
+
+ /* reclaim space if needed */
+ if (zswap_is_full()) {
+ zswap_pool_limit_hit++;
+ if (zbud_reclaim_page(tree->pool, 8)) {
+ zswap_reject_reclaim_fail++;
+ ret = -ENOMEM;
+ goto reject;
+ }
+ }
+
+ /* allocate entry */
+ entry = zswap_entry_cache_alloc(GFP_KERNEL);
+ if (!entry) {
+ zswap_reject_kmemcache_fail++;
+ ret = -ENOMEM;
+ goto reject;
+ }
+
+ /* compress */
+ dst = get_cpu_var(zswap_dstmem);
+ src = kmap_atomic(page);
+ ret = zswap_comp_op(ZSWAP_COMPOP_COMPRESS, src, PAGE_SIZE, dst, &dlen);
+ kunmap_atomic(src);
+ if (ret) {
+ ret = -EINVAL;
+ goto freepage;
+ }
+ len = dlen + sizeof(struct zswap_header);
+ if ((len * 100 / PAGE_SIZE) > zswap_max_compression_ratio) {
+ zswap_reject_compress_poor++;
+ ret = -E2BIG;
+ goto freepage;
+ }
+
+ /* store */
+ ret = zbud_alloc(tree->pool, len, __GFP_NORETRY | __GFP_NOWARN,
+ &handle);
+ if (ret) {
+ zswap_reject_alloc_fail++;
+ goto freepage;
+ }
+ zhdr = zbud_map(tree->pool, handle);
+ zhdr->swpentry = swp_entry(type, offset);
+ buf = (u8 *)(zhdr + 1);
+ memcpy(buf, dst, dlen);
+ zbud_unmap(tree->pool, handle);
+ put_cpu_var(zswap_dstmem);
+
+ /* populate entry */
+ entry->offset = offset;
+ entry->handle = handle;
+ entry->length = dlen;
+
+ /* map */
+ spin_lock(&tree->lock);
+ do {
+ ret = zswap_rb_insert(&tree->rbroot, entry, &dupentry);
+ if (ret == -EEXIST) {
+ zswap_duplicate_entry++;
+ /* remove from rbtree */
+ rb_erase(&dupentry->rbnode, &tree->rbroot);
+ if (!zswap_entry_put(dupentry)) {
+ /* free */
+ zswap_free_entry(tree, dupentry);
+ }
+ }
+ } while (ret == -EEXIST);
+ spin_unlock(&tree->lock);
+
+ /* update stats */
+ atomic_inc(&zswap_stored_pages);
+ atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
+
+ return 0;
+
+freepage:
+ put_cpu_var(zswap_dstmem);
+ zswap_entry_cache_free(entry);
+reject:
+ return ret;
+}
+
+/*
+ * returns 0 if the page was successfully decompressed
+ * return -1 on entry not found or error
+*/
+static int zswap_frontswap_load(unsigned type, pgoff_t offset,
+ struct page *page)
+{
+ struct zswap_tree *tree = zswap_trees[type];
+ struct zswap_entry *entry;
+ u8 *src, *dst;
+ unsigned int dlen;
+ int refcount, ret;
+
+ /* find */
+ spin_lock(&tree->lock);
+ entry = zswap_rb_search(&tree->rbroot, offset);
+ if (!entry) {
+ /* entry was written back */
+ spin_unlock(&tree->lock);
+ return -1;
+ }
+ zswap_entry_get(entry);
+ spin_unlock(&tree->lock);
+
+ /* decompress */
+ dlen = PAGE_SIZE;
+ src = (u8 *)zbud_map(tree->pool, entry->handle) +
+ sizeof(struct zswap_header);
+ dst = kmap_atomic(page);
+ ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src, entry->length,
+ dst, &dlen);
+ kunmap_atomic(dst);
+ zbud_unmap(tree->pool, entry->handle);
+ BUG_ON(ret);
+
+ spin_lock(&tree->lock);
+ refcount = zswap_entry_put(entry);
+ if (likely(refcount)) {
+ spin_unlock(&tree->lock);
+ return 0;
+ }
+ spin_unlock(&tree->lock);
+
+ /*
+ * We don't have to unlink from the rbtree because
+ * zswap_writeback_entry() or zswap_frontswap_invalidate page()
+ * has already done this for us if we are the last reference.
+ */
+ /* free */
+
+ zswap_free_entry(tree, entry);
+
+ return 0;
+}
+
+/* invalidates a single page */
+static void zswap_frontswap_invalidate_page(unsigned type, pgoff_t offset)
+{
+ struct zswap_tree *tree = zswap_trees[type];
+ struct zswap_entry *entry;
+ int refcount;
+
+ /* find */
+ spin_lock(&tree->lock);
+ entry = zswap_rb_search(&tree->rbroot, offset);
+ if (!entry) {
+ /* entry was written back */
+ spin_unlock(&tree->lock);
+ return;
+ }
+
+ /* remove from rbtree */
+ rb_erase(&entry->rbnode, &tree->rbroot);
+
+ /* drop the initial reference from entry creation */
+ refcount = zswap_entry_put(entry);
+
+ spin_unlock(&tree->lock);
+
+ if (refcount) {
+ /* writeback in progress, writeback will free */
+ return;
+ }
+
+ /* free */
+ zswap_free_entry(tree, entry);
+}
+
+/* invalidates all pages for the given swap type */
+static void zswap_frontswap_invalidate_area(unsigned type)
+{
+ struct zswap_tree *tree = zswap_trees[type];
+ struct rb_node *node;
+ struct zswap_entry *entry;
+
+ if (!tree)
+ return;
+
+ /* walk the tree and free everything */
+ spin_lock(&tree->lock);
+ /*
+ * TODO: Even though this code should not be executed because
+ * the try_to_unuse() in swapoff should have emptied the tree,
+ * it is very wasteful to rebalance the tree after every
+ * removal when we are freeing the whole tree.
+ *
+ * If post-order traversal code is ever added to the rbtree
+ * implementation, it should be used here.
+ */
+ while ((node = rb_first(&tree->rbroot))) {
+ entry = rb_entry(node, struct zswap_entry, rbnode);
+ rb_erase(&entry->rbnode, &tree->rbroot);
+ zbud_free(tree->pool, entry->handle);
+ zswap_entry_cache_free(entry);
+ atomic_dec(&zswap_stored_pages);
+ }
+ tree->rbroot = RB_ROOT;
+ spin_unlock(&tree->lock);
+}
+
+static struct zbud_ops zswap_zbud_ops = {
+ .evict = zswap_writeback_entry
+};
+
+/* NOTE: this is called in atomic context from swapon and must not sleep */
+static void zswap_frontswap_init(unsigned type)
+{
+ struct zswap_tree *tree;
+
+ tree = kzalloc(sizeof(struct zswap_tree), GFP_ATOMIC);
+ if (!tree)
+ goto err;
+ tree->pool = zbud_create_pool(GFP_NOWAIT, &zswap_zbud_ops);
+ if (!tree->pool)
+ goto freetree;
+ tree->rbroot = RB_ROOT;
+ spin_lock_init(&tree->lock);
+ tree->type = type;
+ zswap_trees[type] = tree;
+ return;
+
+freetree:
+ kfree(tree);
+err:
+ pr_err("alloc failed, zswap disabled for swap type %d\n", type);
+}
+
+static struct frontswap_ops zswap_frontswap_ops = {
+ .store = zswap_frontswap_store,
+ .load = zswap_frontswap_load,
+ .invalidate_page = zswap_frontswap_invalidate_page,
+ .invalidate_area = zswap_frontswap_invalidate_area,
+ .init = zswap_frontswap_init
+};
+
+/*********************************
+* debugfs functions
+**********************************/
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+
+static struct dentry *zswap_debugfs_root;
+
+static int __init zswap_debugfs_init(void)
+{
+ if (!debugfs_initialized())
+ return -ENODEV;
+
+ zswap_debugfs_root = debugfs_create_dir("zswap", NULL);
+ if (!zswap_debugfs_root)
+ return -ENOMEM;
+
+ debugfs_create_u64("pool_limit_hit", S_IRUGO,
+ zswap_debugfs_root, &zswap_pool_limit_hit);
+ debugfs_create_u64("reject_reclaim_fail", S_IRUGO,
+ zswap_debugfs_root, &zswap_reject_reclaim_fail);
+ debugfs_create_u64("reject_alloc_fail", S_IRUGO,
+ zswap_debugfs_root, &zswap_reject_alloc_fail);
+ debugfs_create_u64("reject_kmemcache_fail", S_IRUGO,
+ zswap_debugfs_root, &zswap_reject_kmemcache_fail);
+ debugfs_create_u64("reject_compress_poor", S_IRUGO,
+ zswap_debugfs_root, &zswap_reject_compress_poor);
+ debugfs_create_u64("written_back_pages", S_IRUGO,
+ zswap_debugfs_root, &zswap_written_back_pages);
+ debugfs_create_u64("duplicate_entry", S_IRUGO,
+ zswap_debugfs_root, &zswap_duplicate_entry);
+ debugfs_create_atomic_t("pool_pages", S_IRUGO,
+ zswap_debugfs_root, &zswap_pool_pages);
+ debugfs_create_atomic_t("stored_pages", S_IRUGO,
+ zswap_debugfs_root, &zswap_stored_pages);
+
+ return 0;
+}
+
+static void __exit zswap_debugfs_exit(void)
+{
+ debugfs_remove_recursive(zswap_debugfs_root);
+}
+#else
+static inline int __init zswap_debugfs_init(void)
+{
+ return 0;
+}
+
+static inline void __exit zswap_debugfs_exit(void) { }
+#endif
+
+/*********************************
+* module init and exit
+**********************************/
+static int __init init_zswap(void)
+{
+ if (!zswap_enabled)
+ return 0;
+
+ pr_info("loading zswap\n");
+ if (zswap_entry_cache_create()) {
+ pr_err("entry cache creation failed\n");
+ goto error;
+ }
+ if (zswap_comp_init()) {
+ pr_err("compressor initialization failed\n");
+ goto compfail;
+ }
+ if (zswap_cpu_init()) {
+ pr_err("per-cpu initialization failed\n");
+ goto pcpufail;
+ }
+ frontswap_register_ops(&zswap_frontswap_ops);
+ if (zswap_debugfs_init())
+ pr_warn("debugfs initialization failed\n");
+ return 0;
+pcpufail:
+ zswap_comp_exit();
+compfail:
+ zswap_entry_cache_destory();
+error:
+ return -ENOMEM;
+}
+/* must be late so crypto has time to come up */
+late_initcall(init_zswap);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Seth Jennings <sjenning [at] linux>");
+MODULE_DESCRIPTION("Compressed cache for swap pages");
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 13, 2013, 3:31 PM

Post #2 of 28 (598 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Seth Jennings [mailto:sjenning [at] linux]
> Subject: [PATCHv11 3/4] zswap: add to mm/
>
> zswap is a thin compression backend for frontswap. It receives pages from
> frontswap and attempts to store them in a compressed memory pool, resulting in
> an effective partial memory reclaim and dramatically reduced swap device I/O.
>
> Additionally, in most cases, pages can be retrieved from this compressed store
> much more quickly than reading from tradition swap devices resulting in faster
> performance for many workloads.
>
> It also has support for evicting swap pages that are currently compressed in
> zswap to the swap device on an LRU(ish) basis. This functionality is very
> important and make zswap a true cache in that, once the cache is full or can't
> grow due to memory pressure, the oldest pages can be moved out of zswap to the
> swap device so newer pages can be compressed and stored in zswap.
>
> This patch adds the zswap driver to mm/
>
> Signed-off-by: Seth Jennings <sjenning [at] linux>

A couple of comments below...

> ---
> mm/Kconfig | 15 +
> mm/Makefile | 1 +
> mm/zswap.c | 952 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 968 insertions(+)
> create mode 100644 mm/zswap.c
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 908f41b..4042e07 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -487,3 +487,18 @@ config ZBUD
> While this design limits storage density, it has simple and
> deterministic reclaim properties that make it preferable to a higher
> density approach when reclaim will be used.
> +
> +config ZSWAP
> + bool "In-kernel swap page compression"
> + depends on FRONTSWAP && CRYPTO
> + select CRYPTO_LZO
> + select ZBUD
> + default n
> + help
> + Zswap is a backend for the frontswap mechanism in the VMM.
> + It receives pages from frontswap and attempts to store them
> + in a compressed memory pool, resulting in an effective
> + partial memory reclaim. In addition, pages and be retrieved
> + from this compressed store much faster than most tradition
> + swap devices resulting in reduced I/O and faster performance
> + for many workloads.
> diff --git a/mm/Makefile b/mm/Makefile
> index 95f0197..f008033 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
> obj-$(CONFIG_BOUNCE) += bounce.o
> obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o
> obj-$(CONFIG_FRONTSWAP) += frontswap.o
> +obj-$(CONFIG_ZSWAP) += zswap.o
> obj-$(CONFIG_HAS_DMA) += dmapool.o
> obj-$(CONFIG_HUGETLBFS) += hugetlb.o
> obj-$(CONFIG_NUMA) += mempolicy.o
> diff --git a/mm/zswap.c b/mm/zswap.c
> new file mode 100644
> index 0000000..b1070ca
> --- /dev/null
> +++ b/mm/zswap.c
> @@ -0,0 +1,952 @@
> +/*
> + * zswap.c - zswap driver file
> + *
> + * zswap is a backend for frontswap that takes pages that are in the
> + * process of being swapped out and attempts to compress them and store
> + * them in a RAM-based memory pool. This results in a significant I/O
> + * reduction on the real swap device and, in the case of a slow swap
> + * device, can also improve workload performance.
> + *
> + * Copyright (C) 2012 Seth Jennings <sjenning [at] linux>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +*/
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/highmem.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +#include <linux/atomic.h>
> +#include <linux/frontswap.h>
> +#include <linux/rbtree.h>
> +#include <linux/swap.h>
> +#include <linux/crypto.h>
> +#include <linux/mempool.h>
> +#include <linux/zbud.h>
> +
> +#include <linux/mm_types.h>
> +#include <linux/page-flags.h>
> +#include <linux/swapops.h>
> +#include <linux/writeback.h>
> +#include <linux/pagemap.h>
> +
> +/*********************************
> +* statistics
> +**********************************/
> +/* Number of memory pages used by the compressed pool */
> +static atomic_t zswap_pool_pages = ATOMIC_INIT(0);
> +/* The number of compressed pages currently stored in zswap */
> +static atomic_t zswap_stored_pages = ATOMIC_INIT(0);
> +
> +/*
> + * The statistics below are not protected from concurrent access for
> + * performance reasons so they may not be a 100% accurate. However,
> + * they do provide useful information on roughly how many times a
> + * certain event is occurring.
> +*/
> +static u64 zswap_pool_limit_hit;
> +static u64 zswap_written_back_pages;
> +static u64 zswap_reject_reclaim_fail;
> +static u64 zswap_reject_compress_poor;
> +static u64 zswap_reject_alloc_fail;
> +static u64 zswap_reject_kmemcache_fail;
> +static u64 zswap_duplicate_entry;
> +
> +/*********************************
> +* tunables
> +**********************************/
> +/* Enable/disable zswap (disabled by default, fixed at boot for now) */
> +static bool zswap_enabled;
> +module_param_named(enabled, zswap_enabled, bool, 0);
> +
> +/* Compressor to be used by zswap (fixed at boot for now) */
> +#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
> +static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> +module_param_named(compressor, zswap_compressor, charp, 0);
> +
> +/* The maximum percentage of memory that the compressed pool can occupy */
> +static unsigned int zswap_max_pool_percent = 20;
> +module_param_named(max_pool_percent,
> + zswap_max_pool_percent, uint, 0644);

This limit, along with the code that enforces it (by calling reclaim
when the limit is reached), is IMHO questionable. Is there any
other kernel memory allocation that is constrained by a percentage
of total memory rather than dynamically according to current
system conditions? As Mel pointed out (approx.), if this limit
is reached by a zswap-storm and filled with pages of long-running,
rarely-used processes, 20% of RAM (by default here) becomes forever
clogged.

Zswap reclaim/writeback needs to be cognizant of (and perhaps driven
by) system memory pressure, not some user-settable percentage.
There's some tough policy questions that need to be answered here,
perhaps not before zswap gets merged, but certainly before it
gets enabled by default by distros.

> +/*
> + * Maximum compression ratio, as as percentage, for an acceptable
> + * compressed page. Any pages that do not compress by at least
> + * this ratio will be rejected.
> +*/
> +static unsigned int zswap_max_compression_ratio = 80;
> +module_param_named(max_compression_ratio,
> + zswap_max_compression_ratio, uint, 0644);

Per earlier discussion, this number is actually derived
from a zsmalloc constraint and doesn't necessarily apply
to zbud. And I don't think any mortal user or system
administrator would have any idea what value to change
this to or the potential impact of changing it. IMHO
it should be removed, or at least moved to and enforced
by the specific allocator code.

> +/*********************************
> +* compression functions
> +**********************************/
> +/* per-cpu compression transforms */
> +static struct crypto_comp * __percpu *zswap_comp_pcpu_tfms;
> +
> +enum comp_op {
> + ZSWAP_COMPOP_COMPRESS,
> + ZSWAP_COMPOP_DECOMPRESS
> +};
> +
> +static int zswap_comp_op(enum comp_op op, const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen)
> +{
> + struct crypto_comp *tfm;
> + int ret;
> +
> + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, get_cpu());
> + switch (op) {
> + case ZSWAP_COMPOP_COMPRESS:
> + ret = crypto_comp_compress(tfm, src, slen, dst, dlen);
> + break;
> + case ZSWAP_COMPOP_DECOMPRESS:
> + ret = crypto_comp_decompress(tfm, src, slen, dst, dlen);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + put_cpu();
> + return ret;
> +}
> +
> +static int __init zswap_comp_init(void)
> +{
> + if (!crypto_has_comp(zswap_compressor, 0, 0)) {
> + pr_info("%s compressor not available\n", zswap_compressor);
> + /* fall back to default compressor */
> + zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> + if (!crypto_has_comp(zswap_compressor, 0, 0))
> + /* can't even load the default compressor */
> + return -ENODEV;
> + }
> + pr_info("using %s compressor\n", zswap_compressor);
> +
> + /* alloc percpu transforms */
> + zswap_comp_pcpu_tfms = alloc_percpu(struct crypto_comp *);
> + if (!zswap_comp_pcpu_tfms)
> + return -ENOMEM;
> + return 0;
> +}
> +
> +static void zswap_comp_exit(void)
> +{
> + /* free percpu transforms */
> + if (zswap_comp_pcpu_tfms)
> + free_percpu(zswap_comp_pcpu_tfms);
> +}
> +
> +/*********************************
> +* data structures
> +**********************************/
> +/*
> + * struct zswap_entry
> + *
> + * This structure contains the metadata for tracking a single compressed
> + * page within zswap.
> + *
> + * rbnode - links the entry into red-black tree for the appropriate swap type
> + * refcount - the number of outstanding reference to the entry. This is needed
> + * to protect against premature freeing of the entry by code
> + * concurent calls to load, invalidate, and writeback. The lock
> + * for the zswap_tree structure that contains the entry must
> + * be held while changing the refcount. Since the lock must
> + * be held, there is no reason to also make refcount atomic.
> + * type - the swap type for the entry. Used to map back to the zswap_tree
> + * structure that contains the entry.
> + * offset - the swap offset for the entry. Index into the red-black tree.
> + * handle - zsmalloc allocation handle that stores the compressed page data
> + * length - the length in bytes of the compressed page data. Needed during
> + * decompression
> + */
> +struct zswap_entry {
> + struct rb_node rbnode;
> + pgoff_t offset;
> + int refcount;
> + unsigned int length;
> + unsigned long handle;
> +};
> +
> +struct zswap_header {
> + swp_entry_t swpentry;
> +};
> +
> +/*
> + * The tree lock in the zswap_tree struct protects a few things:
> + * - the rbtree
> + * - the refcount field of each entry in the tree
> + */
> +struct zswap_tree {
> + struct rb_root rbroot;
> + spinlock_t lock;
> + struct zbud_pool *pool;
> + unsigned type;
> +};
> +
> +static struct zswap_tree *zswap_trees[MAX_SWAPFILES];
> +
> +/*********************************
> +* zswap entry functions
> +**********************************/
> +#define ZSWAP_KMEM_CACHE_NAME "zswap_entry_cache"
> +static struct kmem_cache *zswap_entry_cache;
> +
> +static inline int zswap_entry_cache_create(void)
> +{
> + zswap_entry_cache =
> + kmem_cache_create(ZSWAP_KMEM_CACHE_NAME,
> + sizeof(struct zswap_entry), 0, 0, NULL);
> + return (zswap_entry_cache == NULL);
> +}
> +
> +static inline void zswap_entry_cache_destory(void)
> +{
> + kmem_cache_destroy(zswap_entry_cache);
> +}
> +
> +static inline struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp)
> +{
> + struct zswap_entry *entry;
> + entry = kmem_cache_alloc(zswap_entry_cache, gfp);
> + if (!entry)
> + return NULL;
> + entry->refcount = 1;
> + return entry;
> +}
> +
> +static inline void zswap_entry_cache_free(struct zswap_entry *entry)
> +{
> + kmem_cache_free(zswap_entry_cache, entry);
> +}
> +
> +static inline void zswap_entry_get(struct zswap_entry *entry)
> +{
> + entry->refcount++;
> +}
> +
> +static inline int zswap_entry_put(struct zswap_entry *entry)
> +{
> + entry->refcount--;
> + return entry->refcount;
> +}
> +
> +/*********************************
> +* rbtree functions
> +**********************************/
> +static struct zswap_entry *zswap_rb_search(struct rb_root *root, pgoff_t offset)
> +{
> + struct rb_node *node = root->rb_node;
> + struct zswap_entry *entry;
> +
> + while (node) {
> + entry = rb_entry(node, struct zswap_entry, rbnode);
> + if (entry->offset > offset)
> + node = node->rb_left;
> + else if (entry->offset < offset)
> + node = node->rb_right;
> + else
> + return entry;
> + }
> + return NULL;
> +}
> +
> +/*
> + * In the case that a entry with the same offset is found, it a pointer to
> + * the existing entry is stored in dupentry and the function returns -EEXIST
> +*/
> +static int zswap_rb_insert(struct rb_root *root, struct zswap_entry *entry,
> + struct zswap_entry **dupentry)
> +{
> + struct rb_node **link = &root->rb_node, *parent = NULL;
> + struct zswap_entry *myentry;
> +
> + while (*link) {
> + parent = *link;
> + myentry = rb_entry(parent, struct zswap_entry, rbnode);
> + if (myentry->offset > entry->offset)
> + link = &(*link)->rb_left;
> + else if (myentry->offset < entry->offset)
> + link = &(*link)->rb_right;
> + else {
> + *dupentry = myentry;
> + return -EEXIST;
> + }
> + }
> + rb_link_node(&entry->rbnode, parent, link);
> + rb_insert_color(&entry->rbnode, root);
> + return 0;
> +}
> +
> +/*********************************
> +* per-cpu code
> +**********************************/
> +static DEFINE_PER_CPU(u8 *, zswap_dstmem);
> +
> +static int __zswap_cpu_notifier(unsigned long action, unsigned long cpu)
> +{
> + struct crypto_comp *tfm;
> + u8 *dst;
> +
> + switch (action) {
> + case CPU_UP_PREPARE:
> + tfm = crypto_alloc_comp(zswap_compressor, 0, 0);
> + if (IS_ERR(tfm)) {
> + pr_err("can't allocate compressor transform\n");
> + return NOTIFY_BAD;
> + }
> + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = tfm;
> + dst = kmalloc(PAGE_SIZE * 2, GFP_KERNEL);
> + if (!dst) {
> + pr_err("can't allocate compressor buffer\n");
> + crypto_free_comp(tfm);
> + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> + return NOTIFY_BAD;
> + }
> + per_cpu(zswap_dstmem, cpu) = dst;
> + break;
> + case CPU_DEAD:
> + case CPU_UP_CANCELED:
> + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu);
> + if (tfm) {
> + crypto_free_comp(tfm);
> + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> + }
> + dst = per_cpu(zswap_dstmem, cpu);
> + kfree(dst);
> + per_cpu(zswap_dstmem, cpu) = NULL;
> + break;
> + default:
> + break;
> + }
> + return NOTIFY_OK;
> +}
> +
> +static int zswap_cpu_notifier(struct notifier_block *nb,
> + unsigned long action, void *pcpu)
> +{
> + unsigned long cpu = (unsigned long)pcpu;
> + return __zswap_cpu_notifier(action, cpu);
> +}
> +
> +static struct notifier_block zswap_cpu_notifier_block = {
> + .notifier_call = zswap_cpu_notifier
> +};
> +
> +static int zswap_cpu_init(void)
> +{
> + unsigned long cpu;
> +
> + get_online_cpus();
> + for_each_online_cpu(cpu)
> + if (__zswap_cpu_notifier(CPU_UP_PREPARE, cpu) != NOTIFY_OK)
> + goto cleanup;
> + register_cpu_notifier(&zswap_cpu_notifier_block);
> + put_online_cpus();
> + return 0;
> +
> +cleanup:
> + for_each_online_cpu(cpu)
> + __zswap_cpu_notifier(CPU_UP_CANCELED, cpu);
> + put_online_cpus();
> + return -ENOMEM;
> +}
> +
> +/*********************************
> +* helpers
> +**********************************/
> +static inline bool zswap_is_full(void)
> +{
> + int pool_pages = atomic_read(&zswap_pool_pages);
> + return (totalram_pages * zswap_max_pool_percent / 100 < pool_pages);
> +}
> +
> +/*
> + * Carries out the common pattern of freeing and entry's zsmalloc allocation,
> + * freeing the entry itself, and decrementing the number of stored pages.
> + */
> +static void zswap_free_entry(struct zswap_tree *tree, struct zswap_entry *entry)
> +{
> + zbud_free(tree->pool, entry->handle);
> + zswap_entry_cache_free(entry);
> + atomic_dec(&zswap_stored_pages);
> + atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
> +}
> +
> +/*********************************
> +* writeback code
> +**********************************/
> +/* return enum for zswap_get_swap_cache_page */
> +enum zswap_get_swap_ret {
> + ZSWAP_SWAPCACHE_NEW,
> + ZSWAP_SWAPCACHE_EXIST,
> + ZSWAP_SWAPCACHE_NOMEM
> +};
> +
> +/*
> + * zswap_get_swap_cache_page
> + *
> + * This is an adaption of read_swap_cache_async()
> + *
> + * This function tries to find a page with the given swap entry
> + * in the swapper_space address space (the swap cache). If the page
> + * is found, it is returned in retpage. Otherwise, a page is allocated,
> + * added to the swap cache, and returned in retpage.
> + *
> + * If success, the swap cache page is returned in retpage
> + * Returns 0 if page was already in the swap cache, page is not locked
> + * Returns 1 if the new page needs to be populated, page is locked
> + * Returns <0 on error
> + */
> +static int zswap_get_swap_cache_page(swp_entry_t entry,
> + struct page **retpage)
> +{
> + struct page *found_page, *new_page = NULL;
> + struct address_space *swapper_space = &swapper_spaces[swp_type(entry)];
> + int err;
> +
> + *retpage = NULL;
> + do {
> + /*
> + * First check the swap cache. Since this is normally
> + * called after lookup_swap_cache() failed, re-calling
> + * that would confuse statistics.
> + */
> + found_page = find_get_page(swapper_space, entry.val);
> + if (found_page)
> + break;
> +
> + /*
> + * Get a new page to read into from swap.
> + */
> + if (!new_page) {
> + new_page = alloc_page(GFP_KERNEL);
> + if (!new_page)
> + break; /* Out of memory */
> + }
> +
> + /*
> + * call radix_tree_preload() while we can wait.
> + */
> + err = radix_tree_preload(GFP_KERNEL);
> + if (err)
> + break;
> +
> + /*
> + * Swap entry may have been freed since our caller observed it.
> + */
> + err = swapcache_prepare(entry);
> + if (err == -EEXIST) { /* seems racy */
> + radix_tree_preload_end();
> + continue;
> + }
> + if (err) { /* swp entry is obsolete ? */
> + radix_tree_preload_end();
> + break;
> + }
> +
> + /* May fail (-ENOMEM) if radix-tree node allocation failed. */
> + __set_page_locked(new_page);
> + SetPageSwapBacked(new_page);
> + err = __add_to_swap_cache(new_page, entry);
> + if (likely(!err)) {
> + radix_tree_preload_end();
> + lru_cache_add_anon(new_page);
> + *retpage = new_page;
> + return ZSWAP_SWAPCACHE_NEW;
> + }
> + radix_tree_preload_end();
> + ClearPageSwapBacked(new_page);
> + __clear_page_locked(new_page);
> + /*
> + * add_to_swap_cache() doesn't return -EEXIST, so we can safely
> + * clear SWAP_HAS_CACHE flag.
> + */
> + swapcache_free(entry, NULL);
> + } while (err != -ENOMEM);
> +
> + if (new_page)
> + page_cache_release(new_page);
> + if (!found_page)
> + return ZSWAP_SWAPCACHE_NOMEM;
> + *retpage = found_page;
> + return ZSWAP_SWAPCACHE_EXIST;
> +}
> +
> +/*
> + * Attempts to free and entry by adding a page to the swap cache,
> + * decompressing the entry data into the page, and issuing a
> + * bio write to write the page back to the swap device.
> + *
> + * This can be thought of as a "resumed writeback" of the page
> + * to the swap device. We are basically resuming the same swap
> + * writeback path that was intercepted with the frontswap_store()
> + * in the first place. After the page has been decompressed into
> + * the swap cache, the compressed version stored by zswap can be
> + * freed.
> + */
> +static int zswap_writeback_entry(struct zbud_pool *pool, unsigned long handle)
> +{
> + struct zswap_header *zhdr;
> + swp_entry_t swpentry;
> + struct zswap_tree *tree;
> + pgoff_t offset;
> + struct zswap_entry *entry;
> + struct page *page;
> + u8 *src, *dst;
> + unsigned int dlen;
> + int ret, refcount;
> + struct writeback_control wbc = {
> + .sync_mode = WB_SYNC_NONE,
> + };
> +
> + /* extract swpentry from data */
> + zhdr = zbud_map(pool, handle);
> + swpentry = zhdr->swpentry; /* here */
> + zbud_unmap(pool, handle);
> + tree = zswap_trees[swp_type(swpentry)];
> + offset = swp_offset(swpentry);
> + BUG_ON(pool != tree->pool);
> +
> + /* find and ref zswap entry */
> + spin_lock(&tree->lock);
> + entry = zswap_rb_search(&tree->rbroot, offset);
> + if (!entry) {
> + /* entry was invalidated */
> + spin_unlock(&tree->lock);
> + return 0;
> + }
> + zswap_entry_get(entry);
> + spin_unlock(&tree->lock);
> + BUG_ON(offset != entry->offset);
> +
> + /* try to allocate swap cache page */
> + switch (zswap_get_swap_cache_page(swpentry, &page)) {
> + case ZSWAP_SWAPCACHE_NOMEM: /* no memory */
> + ret = -ENOMEM;
> + goto fail;
> +
> + case ZSWAP_SWAPCACHE_EXIST: /* page is unlocked */
> + /* page is already in the swap cache, ignore for now */
> + page_cache_release(page);
> + ret = -EEXIST;
> + goto fail;
> +
> + case ZSWAP_SWAPCACHE_NEW: /* page is locked */
> + /* decompress */
> + dlen = PAGE_SIZE;
> + src = (u8 *)zbud_map(tree->pool, entry->handle) +
> + sizeof(struct zswap_header);
> + dst = kmap_atomic(page);
> + ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src,
> + entry->length, dst, &dlen);
> + kunmap_atomic(dst);
> + zbud_unmap(tree->pool, entry->handle);
> + BUG_ON(ret);
> + BUG_ON(dlen != PAGE_SIZE);
> +
> + /* page is up to date */
> + SetPageUptodate(page);
> + }
> +
> + /* start writeback */
> + SetPageReclaim(page);
> + __swap_writepage(page, &wbc, end_swap_bio_write);
> + page_cache_release(page);
> + zswap_written_back_pages++;
> +
> + spin_lock(&tree->lock);
> +
> + /* drop local reference */
> + zswap_entry_put(entry);
> + /* drop the initial reference from entry creation */
> + refcount = zswap_entry_put(entry);
> +
> + /*
> + * There are three possible values for refcount here:
> + * (1) refcount is 1, load is in progress, unlink from rbtree,
> + * load will free
> + * (2) refcount is 0, (normal case) entry is valid,
> + * remove from rbtree and free entry
> + * (3) refcount is -1, invalidate happened during writeback,
> + * free entry
> + */
> + if (refcount >= 0) {
> + /* no invalidate yet, remove from rbtree */
> + rb_erase(&entry->rbnode, &tree->rbroot);
> + }
> + spin_unlock(&tree->lock);
> + if (refcount <= 0) {
> + /* free the entry */
> + zswap_free_entry(tree, entry);
> + return 0;
> + }
> + return -EAGAIN;
> +
> +fail:
> + spin_lock(&tree->lock);
> + zswap_entry_put(entry);
> + spin_unlock(&tree->lock);
> + return ret;
> +}
> +
> +/*********************************
> +* frontswap hooks
> +**********************************/
> +/* attempts to compress and store an single page */
> +static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> + struct page *page)
> +{
> + struct zswap_tree *tree = zswap_trees[type];
> + struct zswap_entry *entry, *dupentry;
> + int ret;
> + unsigned int dlen = PAGE_SIZE, len;
> + unsigned long handle;
> + char *buf;
> + u8 *src, *dst;
> + struct zswap_header *zhdr;
> +
> + if (!tree) {
> + ret = -ENODEV;
> + goto reject;
> + }
> +
> + /* reclaim space if needed */
> + if (zswap_is_full()) {
> + zswap_pool_limit_hit++;
> + if (zbud_reclaim_page(tree->pool, 8)) {
> + zswap_reject_reclaim_fail++;
> + ret = -ENOMEM;
> + goto reject;
> + }
> + }

See comment above about enforcing "full".

(No further comments below... Thanks, Dan)

> + /* allocate entry */
> + entry = zswap_entry_cache_alloc(GFP_KERNEL);
> + if (!entry) {
> + zswap_reject_kmemcache_fail++;
> + ret = -ENOMEM;
> + goto reject;
> + }
> +
> + /* compress */
> + dst = get_cpu_var(zswap_dstmem);
> + src = kmap_atomic(page);
> + ret = zswap_comp_op(ZSWAP_COMPOP_COMPRESS, src, PAGE_SIZE, dst, &dlen);
> + kunmap_atomic(src);
> + if (ret) {
> + ret = -EINVAL;
> + goto freepage;
> + }
> + len = dlen + sizeof(struct zswap_header);
> + if ((len * 100 / PAGE_SIZE) > zswap_max_compression_ratio) {
> + zswap_reject_compress_poor++;
> + ret = -E2BIG;
> + goto freepage;
> + }
> +
> + /* store */
> + ret = zbud_alloc(tree->pool, len, __GFP_NORETRY | __GFP_NOWARN,
> + &handle);
> + if (ret) {
> + zswap_reject_alloc_fail++;
> + goto freepage;
> + }
> + zhdr = zbud_map(tree->pool, handle);
> + zhdr->swpentry = swp_entry(type, offset);
> + buf = (u8 *)(zhdr + 1);
> + memcpy(buf, dst, dlen);
> + zbud_unmap(tree->pool, handle);
> + put_cpu_var(zswap_dstmem);
> +
> + /* populate entry */
> + entry->offset = offset;
> + entry->handle = handle;
> + entry->length = dlen;
> +
> + /* map */
> + spin_lock(&tree->lock);
> + do {
> + ret = zswap_rb_insert(&tree->rbroot, entry, &dupentry);
> + if (ret == -EEXIST) {
> + zswap_duplicate_entry++;
> + /* remove from rbtree */
> + rb_erase(&dupentry->rbnode, &tree->rbroot);
> + if (!zswap_entry_put(dupentry)) {
> + /* free */
> + zswap_free_entry(tree, dupentry);
> + }
> + }
> + } while (ret == -EEXIST);
> + spin_unlock(&tree->lock);
> +
> + /* update stats */
> + atomic_inc(&zswap_stored_pages);
> + atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
> +
> + return 0;
> +
> +freepage:
> + put_cpu_var(zswap_dstmem);
> + zswap_entry_cache_free(entry);
> +reject:
> + return ret;
> +}
> +
> +/*
> + * returns 0 if the page was successfully decompressed
> + * return -1 on entry not found or error
> +*/
> +static int zswap_frontswap_load(unsigned type, pgoff_t offset,
> + struct page *page)
> +{
> + struct zswap_tree *tree = zswap_trees[type];
> + struct zswap_entry *entry;
> + u8 *src, *dst;
> + unsigned int dlen;
> + int refcount, ret;
> +
> + /* find */
> + spin_lock(&tree->lock);
> + entry = zswap_rb_search(&tree->rbroot, offset);
> + if (!entry) {
> + /* entry was written back */
> + spin_unlock(&tree->lock);
> + return -1;
> + }
> + zswap_entry_get(entry);
> + spin_unlock(&tree->lock);
> +
> + /* decompress */
> + dlen = PAGE_SIZE;
> + src = (u8 *)zbud_map(tree->pool, entry->handle) +
> + sizeof(struct zswap_header);
> + dst = kmap_atomic(page);
> + ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src, entry->length,
> + dst, &dlen);
> + kunmap_atomic(dst);
> + zbud_unmap(tree->pool, entry->handle);
> + BUG_ON(ret);
> +
> + spin_lock(&tree->lock);
> + refcount = zswap_entry_put(entry);
> + if (likely(refcount)) {
> + spin_unlock(&tree->lock);
> + return 0;
> + }
> + spin_unlock(&tree->lock);
> +
> + /*
> + * We don't have to unlink from the rbtree because
> + * zswap_writeback_entry() or zswap_frontswap_invalidate page()
> + * has already done this for us if we are the last reference.
> + */
> + /* free */
> +
> + zswap_free_entry(tree, entry);
> +
> + return 0;
> +}
> +
> +/* invalidates a single page */
> +static void zswap_frontswap_invalidate_page(unsigned type, pgoff_t offset)
> +{
> + struct zswap_tree *tree = zswap_trees[type];
> + struct zswap_entry *entry;
> + int refcount;
> +
> + /* find */
> + spin_lock(&tree->lock);
> + entry = zswap_rb_search(&tree->rbroot, offset);
> + if (!entry) {
> + /* entry was written back */
> + spin_unlock(&tree->lock);
> + return;
> + }
> +
> + /* remove from rbtree */
> + rb_erase(&entry->rbnode, &tree->rbroot);
> +
> + /* drop the initial reference from entry creation */
> + refcount = zswap_entry_put(entry);
> +
> + spin_unlock(&tree->lock);
> +
> + if (refcount) {
> + /* writeback in progress, writeback will free */
> + return;
> + }
> +
> + /* free */
> + zswap_free_entry(tree, entry);
> +}
> +
> +/* invalidates all pages for the given swap type */
> +static void zswap_frontswap_invalidate_area(unsigned type)
> +{
> + struct zswap_tree *tree = zswap_trees[type];
> + struct rb_node *node;
> + struct zswap_entry *entry;
> +
> + if (!tree)
> + return;
> +
> + /* walk the tree and free everything */
> + spin_lock(&tree->lock);
> + /*
> + * TODO: Even though this code should not be executed because
> + * the try_to_unuse() in swapoff should have emptied the tree,
> + * it is very wasteful to rebalance the tree after every
> + * removal when we are freeing the whole tree.
> + *
> + * If post-order traversal code is ever added to the rbtree
> + * implementation, it should be used here.
> + */
> + while ((node = rb_first(&tree->rbroot))) {
> + entry = rb_entry(node, struct zswap_entry, rbnode);
> + rb_erase(&entry->rbnode, &tree->rbroot);
> + zbud_free(tree->pool, entry->handle);
> + zswap_entry_cache_free(entry);
> + atomic_dec(&zswap_stored_pages);
> + }
> + tree->rbroot = RB_ROOT;
> + spin_unlock(&tree->lock);
> +}
> +
> +static struct zbud_ops zswap_zbud_ops = {
> + .evict = zswap_writeback_entry
> +};
> +
> +/* NOTE: this is called in atomic context from swapon and must not sleep */
> +static void zswap_frontswap_init(unsigned type)
> +{
> + struct zswap_tree *tree;
> +
> + tree = kzalloc(sizeof(struct zswap_tree), GFP_ATOMIC);
> + if (!tree)
> + goto err;
> + tree->pool = zbud_create_pool(GFP_NOWAIT, &zswap_zbud_ops);
> + if (!tree->pool)
> + goto freetree;
> + tree->rbroot = RB_ROOT;
> + spin_lock_init(&tree->lock);
> + tree->type = type;
> + zswap_trees[type] = tree;
> + return;
> +
> +freetree:
> + kfree(tree);
> +err:
> + pr_err("alloc failed, zswap disabled for swap type %d\n", type);
> +}
> +
> +static struct frontswap_ops zswap_frontswap_ops = {
> + .store = zswap_frontswap_store,
> + .load = zswap_frontswap_load,
> + .invalidate_page = zswap_frontswap_invalidate_page,
> + .invalidate_area = zswap_frontswap_invalidate_area,
> + .init = zswap_frontswap_init
> +};
> +
> +/*********************************
> +* debugfs functions
> +**********************************/
> +#ifdef CONFIG_DEBUG_FS
> +#include <linux/debugfs.h>
> +
> +static struct dentry *zswap_debugfs_root;
> +
> +static int __init zswap_debugfs_init(void)
> +{
> + if (!debugfs_initialized())
> + return -ENODEV;
> +
> + zswap_debugfs_root = debugfs_create_dir("zswap", NULL);
> + if (!zswap_debugfs_root)
> + return -ENOMEM;
> +
> + debugfs_create_u64("pool_limit_hit", S_IRUGO,
> + zswap_debugfs_root, &zswap_pool_limit_hit);
> + debugfs_create_u64("reject_reclaim_fail", S_IRUGO,
> + zswap_debugfs_root, &zswap_reject_reclaim_fail);
> + debugfs_create_u64("reject_alloc_fail", S_IRUGO,
> + zswap_debugfs_root, &zswap_reject_alloc_fail);
> + debugfs_create_u64("reject_kmemcache_fail", S_IRUGO,
> + zswap_debugfs_root, &zswap_reject_kmemcache_fail);
> + debugfs_create_u64("reject_compress_poor", S_IRUGO,
> + zswap_debugfs_root, &zswap_reject_compress_poor);
> + debugfs_create_u64("written_back_pages", S_IRUGO,
> + zswap_debugfs_root, &zswap_written_back_pages);
> + debugfs_create_u64("duplicate_entry", S_IRUGO,
> + zswap_debugfs_root, &zswap_duplicate_entry);
> + debugfs_create_atomic_t("pool_pages", S_IRUGO,
> + zswap_debugfs_root, &zswap_pool_pages);
> + debugfs_create_atomic_t("stored_pages", S_IRUGO,
> + zswap_debugfs_root, &zswap_stored_pages);
> +
> + return 0;
> +}
> +
> +static void __exit zswap_debugfs_exit(void)
> +{
> + debugfs_remove_recursive(zswap_debugfs_root);
> +}
> +#else
> +static inline int __init zswap_debugfs_init(void)
> +{
> + return 0;
> +}
> +
> +static inline void __exit zswap_debugfs_exit(void) { }
> +#endif
> +
> +/*********************************
> +* module init and exit
> +**********************************/
> +static int __init init_zswap(void)
> +{
> + if (!zswap_enabled)
> + return 0;
> +
> + pr_info("loading zswap\n");
> + if (zswap_entry_cache_create()) {
> + pr_err("entry cache creation failed\n");
> + goto error;
> + }
> + if (zswap_comp_init()) {
> + pr_err("compressor initialization failed\n");
> + goto compfail;
> + }
> + if (zswap_cpu_init()) {
> + pr_err("per-cpu initialization failed\n");
> + goto pcpufail;
> + }
> + frontswap_register_ops(&zswap_frontswap_ops);
> + if (zswap_debugfs_init())
> + pr_warn("debugfs initialization failed\n");
> + return 0;
> +pcpufail:
> + zswap_comp_exit();
> +compfail:
> + zswap_entry_cache_destory();
> +error:
> + return -ENOMEM;
> +}
> +/* must be late so crypto has time to come up */
> +late_initcall(init_zswap);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Seth Jennings <sjenning [at] linux>");
> +MODULE_DESCRIPTION("Compressed cache for swap pages");
> --
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bob.liu at oracle

May 14, 2013, 2:19 AM

Post #3 of 28 (598 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

Hi Seth,

On 05/13/2013 08:40 PM, Seth Jennings wrote:
> zswap is a thin compression backend for frontswap. It receives pages from
> frontswap and attempts to store them in a compressed memory pool, resulting in
> an effective partial memory reclaim and dramatically reduced swap device I/O.
>
> Additionally, in most cases, pages can be retrieved from this compressed store
> much more quickly than reading from tradition swap devices resulting in faster
> performance for many workloads.
>
> It also has support for evicting swap pages that are currently compressed in
> zswap to the swap device on an LRU(ish) basis. This functionality is very
> important and make zswap a true cache in that, once the cache is full or can't
> grow due to memory pressure, the oldest pages can be moved out of zswap to the
> swap device so newer pages can be compressed and stored in zswap.
>
> This patch adds the zswap driver to mm/
>
> Signed-off-by: Seth Jennings <sjenning [at] linux>

It seems that you didn't address some comments from Mel in
[PATCHv9 4/8] zswap: add to mm/

> ---
> mm/Kconfig | 15 +
> mm/Makefile | 1 +
> mm/zswap.c | 952 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 968 insertions(+)
> create mode 100644 mm/zswap.c
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 908f41b..4042e07 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -487,3 +487,18 @@ config ZBUD
> While this design limits storage density, it has simple and
> deterministic reclaim properties that make it preferable to a higher
> density approach when reclaim will be used.
> +
> +config ZSWAP
> + bool "In-kernel swap page compression"
> + depends on FRONTSWAP && CRYPTO
> + select CRYPTO_LZO
> + select ZBUD
> + default n
> + help
> + Zswap is a backend for the frontswap mechanism in the VMM.
> + It receives pages from frontswap and attempts to store them
> + in a compressed memory pool, resulting in an effective
> + partial memory reclaim. In addition, pages and be retrieved
> + from this compressed store much faster than most tradition
> + swap devices resulting in reduced I/O and faster performance
> + for many workloads.
> diff --git a/mm/Makefile b/mm/Makefile
> index 95f0197..f008033 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
> obj-$(CONFIG_BOUNCE) += bounce.o
> obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o
> obj-$(CONFIG_FRONTSWAP) += frontswap.o
> +obj-$(CONFIG_ZSWAP) += zswap.o
> obj-$(CONFIG_HAS_DMA) += dmapool.o
> obj-$(CONFIG_HUGETLBFS) += hugetlb.o
> obj-$(CONFIG_NUMA) += mempolicy.o
> diff --git a/mm/zswap.c b/mm/zswap.c
> new file mode 100644
> index 0000000..b1070ca
> --- /dev/null
> +++ b/mm/zswap.c
> @@ -0,0 +1,952 @@
> +/*
> + * zswap.c - zswap driver file
> + *
> + * zswap is a backend for frontswap that takes pages that are in the
> + * process of being swapped out and attempts to compress them and store
> + * them in a RAM-based memory pool. This results in a significant I/O
> + * reduction on the real swap device and, in the case of a slow swap
> + * device, can also improve workload performance.
> + *
> + * Copyright (C) 2012 Seth Jennings <sjenning [at] linux>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +*/
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/highmem.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +#include <linux/atomic.h>
> +#include <linux/frontswap.h>
> +#include <linux/rbtree.h>
> +#include <linux/swap.h>
> +#include <linux/crypto.h>
> +#include <linux/mempool.h>
> +#include <linux/zbud.h>
> +
> +#include <linux/mm_types.h>
> +#include <linux/page-flags.h>
> +#include <linux/swapops.h>
> +#include <linux/writeback.h>
> +#include <linux/pagemap.h>
> +
> +/*********************************
> +* statistics
> +**********************************/
> +/* Number of memory pages used by the compressed pool */
> +static atomic_t zswap_pool_pages = ATOMIC_INIT(0);
> +/* The number of compressed pages currently stored in zswap */
> +static atomic_t zswap_stored_pages = ATOMIC_INIT(0);
> +
> +/*
> + * The statistics below are not protected from concurrent access for
> + * performance reasons so they may not be a 100% accurate. However,
> + * they do provide useful information on roughly how many times a
> + * certain event is occurring.
> +*/
> +static u64 zswap_pool_limit_hit;
> +static u64 zswap_written_back_pages;
> +static u64 zswap_reject_reclaim_fail;
> +static u64 zswap_reject_compress_poor;
> +static u64 zswap_reject_alloc_fail;
> +static u64 zswap_reject_kmemcache_fail;
> +static u64 zswap_duplicate_entry;
> +
> +/*********************************
> +* tunables
> +**********************************/
> +/* Enable/disable zswap (disabled by default, fixed at boot for now) */
> +static bool zswap_enabled;
> +module_param_named(enabled, zswap_enabled, bool, 0);
> +
> +/* Compressor to be used by zswap (fixed at boot for now) */
> +#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
> +static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> +module_param_named(compressor, zswap_compressor, charp, 0);
> +
> +/* The maximum percentage of memory that the compressed pool can occupy */
> +static unsigned int zswap_max_pool_percent = 20;
> +module_param_named(max_pool_percent,
> + zswap_max_pool_percent, uint, 0644);
> +

I think it's reasonable but... see comments in zbud_reclaim_page().

> +/*
> + * Maximum compression ratio, as as percentage, for an acceptable
> + * compressed page. Any pages that do not compress by at least
> + * this ratio will be rejected.
> +*/
> +static unsigned int zswap_max_compression_ratio = 80;
> +module_param_named(max_compression_ratio,
> + zswap_max_compression_ratio, uint, 0644);
> +

Prefer not export it, it's hard for use to know what value should set to.

> +/*********************************
> +* compression functions
> +**********************************/
> +/* per-cpu compression transforms */
> +static struct crypto_comp * __percpu *zswap_comp_pcpu_tfms;
> +
> +enum comp_op {
> + ZSWAP_COMPOP_COMPRESS,
> + ZSWAP_COMPOP_DECOMPRESS
> +};
> +
> +static int zswap_comp_op(enum comp_op op, const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen)
> +{
> + struct crypto_comp *tfm;
> + int ret;
> +
> + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, get_cpu());
> + switch (op) {
> + case ZSWAP_COMPOP_COMPRESS:
> + ret = crypto_comp_compress(tfm, src, slen, dst, dlen);
> + break;
> + case ZSWAP_COMPOP_DECOMPRESS:
> + ret = crypto_comp_decompress(tfm, src, slen, dst, dlen);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + put_cpu();
> + return ret;
> +}
> +
> +static int __init zswap_comp_init(void)
> +{
> + if (!crypto_has_comp(zswap_compressor, 0, 0)) {
> + pr_info("%s compressor not available\n", zswap_compressor);
> + /* fall back to default compressor */
> + zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> + if (!crypto_has_comp(zswap_compressor, 0, 0))
> + /* can't even load the default compressor */
> + return -ENODEV;
> + }
> + pr_info("using %s compressor\n", zswap_compressor);
> +
> + /* alloc percpu transforms */
> + zswap_comp_pcpu_tfms = alloc_percpu(struct crypto_comp *);
> + if (!zswap_comp_pcpu_tfms)
> + return -ENOMEM;
> + return 0;
> +}
> +
> +static void zswap_comp_exit(void)
> +{
> + /* free percpu transforms */
> + if (zswap_comp_pcpu_tfms)
> + free_percpu(zswap_comp_pcpu_tfms);
> +}
> +
> +/*********************************
> +* data structures
> +**********************************/
> +/*
> + * struct zswap_entry
> + *
> + * This structure contains the metadata for tracking a single compressed
> + * page within zswap.
> + *
> + * rbnode - links the entry into red-black tree for the appropriate swap type
> + * refcount - the number of outstanding reference to the entry. This is needed
> + * to protect against premature freeing of the entry by code
> + * concurent calls to load, invalidate, and writeback. The lock
> + * for the zswap_tree structure that contains the entry must
> + * be held while changing the refcount. Since the lock must
> + * be held, there is no reason to also make refcount atomic.
> + * type - the swap type for the entry. Used to map back to the zswap_tree
> + * structure that contains the entry.
> + * offset - the swap offset for the entry. Index into the red-black tree.
> + * handle - zsmalloc allocation handle that stores the compressed page data
> + * length - the length in bytes of the compressed page data. Needed during
> + * decompression

The sequence is different from the struct define?

> + */
> +struct zswap_entry {
> + struct rb_node rbnode;
> + pgoff_t offset;
> + int refcount;
> + unsigned int length;
> + unsigned long handle;
> +};
> +
> +struct zswap_header {
> + swp_entry_t swpentry;
> +};
> +
> +/*
> + * The tree lock in the zswap_tree struct protects a few things:
> + * - the rbtree
> + * - the refcount field of each entry in the tree
> + */
> +struct zswap_tree {
> + struct rb_root rbroot;
> + spinlock_t lock;
> + struct zbud_pool *pool;
> + unsigned type;

It seems that zswap_tree->type have no usage for zswap.

> +};
> +
> +static struct zswap_tree *zswap_trees[MAX_SWAPFILES];
> +
> +/*********************************
> +* zswap entry functions
> +**********************************/
> +#define ZSWAP_KMEM_CACHE_NAME "zswap_entry_cache"
> +static struct kmem_cache *zswap_entry_cache;
> +
> +static inline int zswap_entry_cache_create(void)
> +{
> + zswap_entry_cache =
> + kmem_cache_create(ZSWAP_KMEM_CACHE_NAME,
> + sizeof(struct zswap_entry), 0, 0, NULL);
> + return (zswap_entry_cache == NULL);
> +}
> +
> +static inline void zswap_entry_cache_destory(void)
> +{
> + kmem_cache_destroy(zswap_entry_cache);
> +}
> +
> +static inline struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp)
> +{
> + struct zswap_entry *entry;
> + entry = kmem_cache_alloc(zswap_entry_cache, gfp);
> + if (!entry)
> + return NULL;
> + entry->refcount = 1;
> + return entry;
> +}
> +
> +static inline void zswap_entry_cache_free(struct zswap_entry *entry)
> +{
> + kmem_cache_free(zswap_entry_cache, entry);
> +}
> +
> +static inline void zswap_entry_get(struct zswap_entry *entry)
> +{
> + entry->refcount++;
> +}
> +
> +static inline int zswap_entry_put(struct zswap_entry *entry)
> +{
> + entry->refcount--;
> + return entry->refcount;
> +}

Better if have lock comments here.

> +
> +/*********************************
> +* rbtree functions
> +**********************************/
> +static struct zswap_entry *zswap_rb_search(struct rb_root *root, pgoff_t offset)
> +{
> + struct rb_node *node = root->rb_node;
> + struct zswap_entry *entry;
> +
> + while (node) {
> + entry = rb_entry(node, struct zswap_entry, rbnode);
> + if (entry->offset > offset)
> + node = node->rb_left;
> + else if (entry->offset < offset)
> + node = node->rb_right;
> + else
> + return entry;
> + }
> + return NULL;
> +}
> +
> +/*
> + * In the case that a entry with the same offset is found, it a pointer to
> + * the existing entry is stored in dupentry and the function returns -EEXIST
> +*/
> +static int zswap_rb_insert(struct rb_root *root, struct zswap_entry *entry,
> + struct zswap_entry **dupentry)
> +{
> + struct rb_node **link = &root->rb_node, *parent = NULL;
> + struct zswap_entry *myentry;
> +
> + while (*link) {
> + parent = *link;
> + myentry = rb_entry(parent, struct zswap_entry, rbnode);
> + if (myentry->offset > entry->offset)
> + link = &(*link)->rb_left;
> + else if (myentry->offset < entry->offset)
> + link = &(*link)->rb_right;
> + else {
> + *dupentry = myentry;
> + return -EEXIST;
> + }
> + }
> + rb_link_node(&entry->rbnode, parent, link);
> + rb_insert_color(&entry->rbnode, root);
> + return 0;
> +}
> +
> +/*********************************
> +* per-cpu code
> +**********************************/
> +static DEFINE_PER_CPU(u8 *, zswap_dstmem);
> +
> +static int __zswap_cpu_notifier(unsigned long action, unsigned long cpu)
> +{
> + struct crypto_comp *tfm;
> + u8 *dst;
> +
> + switch (action) {
> + case CPU_UP_PREPARE:
> + tfm = crypto_alloc_comp(zswap_compressor, 0, 0);
> + if (IS_ERR(tfm)) {
> + pr_err("can't allocate compressor transform\n");
> + return NOTIFY_BAD;
> + }
> + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = tfm;
> + dst = kmalloc(PAGE_SIZE * 2, GFP_KERNEL);
> + if (!dst) {
> + pr_err("can't allocate compressor buffer\n");
> + crypto_free_comp(tfm);
> + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> + return NOTIFY_BAD;
> + }
> + per_cpu(zswap_dstmem, cpu) = dst;
> + break;
> + case CPU_DEAD:
> + case CPU_UP_CANCELED:
> + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu);
> + if (tfm) {
> + crypto_free_comp(tfm);
> + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> + }
> + dst = per_cpu(zswap_dstmem, cpu);
> + kfree(dst);
> + per_cpu(zswap_dstmem, cpu) = NULL;
> + break;
> + default:
> + break;
> + }
> + return NOTIFY_OK;
> +}
> +
> +static int zswap_cpu_notifier(struct notifier_block *nb,
> + unsigned long action, void *pcpu)
> +{
> + unsigned long cpu = (unsigned long)pcpu;
> + return __zswap_cpu_notifier(action, cpu);
> +}
> +
> +static struct notifier_block zswap_cpu_notifier_block = {
> + .notifier_call = zswap_cpu_notifier
> +};
> +
> +static int zswap_cpu_init(void)
> +{
> + unsigned long cpu;
> +
> + get_online_cpus();
> + for_each_online_cpu(cpu)
> + if (__zswap_cpu_notifier(CPU_UP_PREPARE, cpu) != NOTIFY_OK)
> + goto cleanup;
> + register_cpu_notifier(&zswap_cpu_notifier_block);
> + put_online_cpus();
> + return 0;
> +
> +cleanup:
> + for_each_online_cpu(cpu)
> + __zswap_cpu_notifier(CPU_UP_CANCELED, cpu);
> + put_online_cpus();
> + return -ENOMEM;
> +}
> +
> +/*********************************
> +* helpers
> +**********************************/
> +static inline bool zswap_is_full(void)
> +{
> + int pool_pages = atomic_read(&zswap_pool_pages);
> + return (totalram_pages * zswap_max_pool_percent / 100 < pool_pages);
> +}
> +
> +/*
> + * Carries out the common pattern of freeing and entry's zsmalloc allocation,
> + * freeing the entry itself, and decrementing the number of stored pages.
> + */
> +static void zswap_free_entry(struct zswap_tree *tree, struct zswap_entry *entry)
> +{
> + zbud_free(tree->pool, entry->handle);
> + zswap_entry_cache_free(entry);
> + atomic_dec(&zswap_stored_pages);
> + atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
> +}
> +
> +/*********************************
> +* writeback code
> +**********************************/
> +/* return enum for zswap_get_swap_cache_page */
> +enum zswap_get_swap_ret {
> + ZSWAP_SWAPCACHE_NEW,
> + ZSWAP_SWAPCACHE_EXIST,
> + ZSWAP_SWAPCACHE_NOMEM
> +};
> +
> +/*
> + * zswap_get_swap_cache_page
> + *
> + * This is an adaption of read_swap_cache_async()
> + *
> + * This function tries to find a page with the given swap entry
> + * in the swapper_space address space (the swap cache). If the page
> + * is found, it is returned in retpage. Otherwise, a page is allocated,
> + * added to the swap cache, and returned in retpage.
> + *
> + * If success, the swap cache page is returned in retpage
> + * Returns 0 if page was already in the swap cache, page is not locked
> + * Returns 1 if the new page needs to be populated, page is locked
> + * Returns <0 on error
> + */
> +static int zswap_get_swap_cache_page(swp_entry_t entry,
> + struct page **retpage)
> +{
> + struct page *found_page, *new_page = NULL;
> + struct address_space *swapper_space = &swapper_spaces[swp_type(entry)];
> + int err;
> +
> + *retpage = NULL;
> + do {
> + /*
> + * First check the swap cache. Since this is normally
> + * called after lookup_swap_cache() failed, re-calling
> + * that would confuse statistics.
> + */
> + found_page = find_get_page(swapper_space, entry.val);
> + if (found_page)
> + break;
> +
> + /*
> + * Get a new page to read into from swap.
> + */
> + if (!new_page) {
> + new_page = alloc_page(GFP_KERNEL);
> + if (!new_page)
> + break; /* Out of memory */
> + }
> +
> + /*
> + * call radix_tree_preload() while we can wait.
> + */
> + err = radix_tree_preload(GFP_KERNEL);
> + if (err)
> + break;
> +
> + /*
> + * Swap entry may have been freed since our caller observed it.
> + */
> + err = swapcache_prepare(entry);
> + if (err == -EEXIST) { /* seems racy */
> + radix_tree_preload_end();
> + continue;
> + }
> + if (err) { /* swp entry is obsolete ? */
> + radix_tree_preload_end();
> + break;
> + }
> +
> + /* May fail (-ENOMEM) if radix-tree node allocation failed. */
> + __set_page_locked(new_page);
> + SetPageSwapBacked(new_page);
> + err = __add_to_swap_cache(new_page, entry);
> + if (likely(!err)) {
> + radix_tree_preload_end();
> + lru_cache_add_anon(new_page);
> + *retpage = new_page;
> + return ZSWAP_SWAPCACHE_NEW;
> + }
> + radix_tree_preload_end();
> + ClearPageSwapBacked(new_page);
> + __clear_page_locked(new_page);
> + /*
> + * add_to_swap_cache() doesn't return -EEXIST, so we can safely
> + * clear SWAP_HAS_CACHE flag.
> + */
> + swapcache_free(entry, NULL);
> + } while (err != -ENOMEM);
> +
> + if (new_page)
> + page_cache_release(new_page);
> + if (!found_page)
> + return ZSWAP_SWAPCACHE_NOMEM;
> + *retpage = found_page;
> + return ZSWAP_SWAPCACHE_EXIST;
> +}
> +
> +/*
> + * Attempts to free and entry by adding a page to the swap cache,
> + * decompressing the entry data into the page, and issuing a
> + * bio write to write the page back to the swap device.
> + *
> + * This can be thought of as a "resumed writeback" of the page
> + * to the swap device. We are basically resuming the same swap
> + * writeback path that was intercepted with the frontswap_store()
> + * in the first place. After the page has been decompressed into
> + * the swap cache, the compressed version stored by zswap can be
> + * freed.
> + */
> +static int zswap_writeback_entry(struct zbud_pool *pool, unsigned long handle)
> +{
> + struct zswap_header *zhdr;
> + swp_entry_t swpentry;
> + struct zswap_tree *tree;
> + pgoff_t offset;
> + struct zswap_entry *entry;
> + struct page *page;
> + u8 *src, *dst;
> + unsigned int dlen;
> + int ret, refcount;
> + struct writeback_control wbc = {
> + .sync_mode = WB_SYNC_NONE,
> + };
> +
> + /* extract swpentry from data */
> + zhdr = zbud_map(pool, handle);
> + swpentry = zhdr->swpentry; /* here */
> + zbud_unmap(pool, handle);
> + tree = zswap_trees[swp_type(swpentry)];
> + offset = swp_offset(swpentry);
> + BUG_ON(pool != tree->pool);
> +
> + /* find and ref zswap entry */
> + spin_lock(&tree->lock);
> + entry = zswap_rb_search(&tree->rbroot, offset);
> + if (!entry) {
> + /* entry was invalidated */
> + spin_unlock(&tree->lock);
> + return 0;
> + }
> + zswap_entry_get(entry);
> + spin_unlock(&tree->lock);
> + BUG_ON(offset != entry->offset);
> +
> + /* try to allocate swap cache page */
> + switch (zswap_get_swap_cache_page(swpentry, &page)) {
> + case ZSWAP_SWAPCACHE_NOMEM: /* no memory */
> + ret = -ENOMEM;
> + goto fail;
> +
> + case ZSWAP_SWAPCACHE_EXIST: /* page is unlocked */
> + /* page is already in the swap cache, ignore for now */
> + page_cache_release(page);
> + ret = -EEXIST;
> + goto fail;
> +
> + case ZSWAP_SWAPCACHE_NEW: /* page is locked */
> + /* decompress */
> + dlen = PAGE_SIZE;
> + src = (u8 *)zbud_map(tree->pool, entry->handle) +
> + sizeof(struct zswap_header);
> + dst = kmap_atomic(page);
> + ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src,
> + entry->length, dst, &dlen);
> + kunmap_atomic(dst);
> + zbud_unmap(tree->pool, entry->handle);
> + BUG_ON(ret);
> + BUG_ON(dlen != PAGE_SIZE);
> +
> + /* page is up to date */
> + SetPageUptodate(page);
> + }
> +
> + /* start writeback */
> + SetPageReclaim(page);
> + __swap_writepage(page, &wbc, end_swap_bio_write);
> + page_cache_release(page);
> + zswap_written_back_pages++;
> +
> + spin_lock(&tree->lock);
> +
> + /* drop local reference */
> + zswap_entry_put(entry);
> + /* drop the initial reference from entry creation */
> + refcount = zswap_entry_put(entry);
> +
> + /*
> + * There are three possible values for refcount here:
> + * (1) refcount is 1, load is in progress, unlink from rbtree,
> + * load will free
> + * (2) refcount is 0, (normal case) entry is valid,
> + * remove from rbtree and free entry
> + * (3) refcount is -1, invalidate happened during writeback,
> + * free entry
> + */
> + if (refcount >= 0) {
> + /* no invalidate yet, remove from rbtree */
> + rb_erase(&entry->rbnode, &tree->rbroot);
> + }
> + spin_unlock(&tree->lock);
> + if (refcount <= 0) {
> + /* free the entry */
> + zswap_free_entry(tree, entry);
> + return 0;
> + }
> + return -EAGAIN;
> +
> +fail:
> + spin_lock(&tree->lock);
> + zswap_entry_put(entry);
> + spin_unlock(&tree->lock);
> + return ret;
> +}
> +
> +/*********************************
> +* frontswap hooks
> +**********************************/
> +/* attempts to compress and store an single page */
> +static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> + struct page *page)
> +{
> + struct zswap_tree *tree = zswap_trees[type];
> + struct zswap_entry *entry, *dupentry;
> + int ret;
> + unsigned int dlen = PAGE_SIZE, len;
> + unsigned long handle;
> + char *buf;
> + u8 *src, *dst;
> + struct zswap_header *zhdr;
> +
> + if (!tree) {
> + ret = -ENODEV;
> + goto reject;
> + }
> +
> + /* reclaim space if needed */
> + if (zswap_is_full()) {
> + zswap_pool_limit_hit++;
> + if (zbud_reclaim_page(tree->pool, 8)) {

My idea is to wake up a kernel thread here to do the reclaim.
Once zswap is full(20% percent of total mem currently), the kernel
thread should reclaim pages from it. Not only reclaim one page, it
should depend on the current memory pressure.
And then the API in zbud may like this:
zbud_reclaim_page(pool, nr_pages_to_reclaim, nr_retry);


> + zswap_reject_reclaim_fail++;
> + ret = -ENOMEM;
> + goto reject;
> + }
> + }
> +


--
Regards,
-Bob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sjenning at linux

May 14, 2013, 9:00 AM

Post #4 of 28 (594 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On Tue, May 14, 2013 at 05:19:19PM +0800, Bob Liu wrote:
> Hi Seth,

Hi Bob, thanks for the review!

>
> On 05/13/2013 08:40 PM, Seth Jennings wrote:
> > zswap is a thin compression backend for frontswap. It receives pages from
> > frontswap and attempts to store them in a compressed memory pool, resulting in
> > an effective partial memory reclaim and dramatically reduced swap device I/O.
> >
> > Additionally, in most cases, pages can be retrieved from this compressed store
> > much more quickly than reading from tradition swap devices resulting in faster
> > performance for many workloads.
> >
> > It also has support for evicting swap pages that are currently compressed in
> > zswap to the swap device on an LRU(ish) basis. This functionality is very
> > important and make zswap a true cache in that, once the cache is full or can't
> > grow due to memory pressure, the oldest pages can be moved out of zswap to the
> > swap device so newer pages can be compressed and stored in zswap.
> >
> > This patch adds the zswap driver to mm/
> >
> > Signed-off-by: Seth Jennings <sjenning [at] linux>
>
> It seems that you didn't address some comments from Mel in
> [PATCHv9 4/8] zswap: add to mm/
>
> > ---
> > mm/Kconfig | 15 +
> > mm/Makefile | 1 +
> > mm/zswap.c | 952 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 968 insertions(+)
> > create mode 100644 mm/zswap.c
> >
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 908f41b..4042e07 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -487,3 +487,18 @@ config ZBUD
> > While this design limits storage density, it has simple and
> > deterministic reclaim properties that make it preferable to a higher
> > density approach when reclaim will be used.
> > +
> > +config ZSWAP
> > + bool "In-kernel swap page compression"
> > + depends on FRONTSWAP && CRYPTO
> > + select CRYPTO_LZO
> > + select ZBUD
> > + default n
> > + help
> > + Zswap is a backend for the frontswap mechanism in the VMM.
> > + It receives pages from frontswap and attempts to store them
> > + in a compressed memory pool, resulting in an effective
> > + partial memory reclaim. In addition, pages and be retrieved
> > + from this compressed store much faster than most tradition
> > + swap devices resulting in reduced I/O and faster performance
> > + for many workloads.
> > diff --git a/mm/Makefile b/mm/Makefile
> > index 95f0197..f008033 100644
> > --- a/mm/Makefile
> > +++ b/mm/Makefile
> > @@ -32,6 +32,7 @@ obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
> > obj-$(CONFIG_BOUNCE) += bounce.o
> > obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o
> > obj-$(CONFIG_FRONTSWAP) += frontswap.o
> > +obj-$(CONFIG_ZSWAP) += zswap.o
> > obj-$(CONFIG_HAS_DMA) += dmapool.o
> > obj-$(CONFIG_HUGETLBFS) += hugetlb.o
> > obj-$(CONFIG_NUMA) += mempolicy.o
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > new file mode 100644
> > index 0000000..b1070ca
> > --- /dev/null
> > +++ b/mm/zswap.c
> > @@ -0,0 +1,952 @@
> > +/*
> > + * zswap.c - zswap driver file
> > + *
> > + * zswap is a backend for frontswap that takes pages that are in the
> > + * process of being swapped out and attempts to compress them and store
> > + * them in a RAM-based memory pool. This results in a significant I/O
> > + * reduction on the real swap device and, in the case of a slow swap
> > + * device, can also improve workload performance.
> > + *
> > + * Copyright (C) 2012 Seth Jennings <sjenning [at] linux>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version 2
> > + * of the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > +*/
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/module.h>
> > +#include <linux/cpu.h>
> > +#include <linux/highmem.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> > +#include <linux/atomic.h>
> > +#include <linux/frontswap.h>
> > +#include <linux/rbtree.h>
> > +#include <linux/swap.h>
> > +#include <linux/crypto.h>
> > +#include <linux/mempool.h>
> > +#include <linux/zbud.h>
> > +
> > +#include <linux/mm_types.h>
> > +#include <linux/page-flags.h>
> > +#include <linux/swapops.h>
> > +#include <linux/writeback.h>
> > +#include <linux/pagemap.h>
> > +
> > +/*********************************
> > +* statistics
> > +**********************************/
> > +/* Number of memory pages used by the compressed pool */
> > +static atomic_t zswap_pool_pages = ATOMIC_INIT(0);
> > +/* The number of compressed pages currently stored in zswap */
> > +static atomic_t zswap_stored_pages = ATOMIC_INIT(0);
> > +
> > +/*
> > + * The statistics below are not protected from concurrent access for
> > + * performance reasons so they may not be a 100% accurate. However,
> > + * they do provide useful information on roughly how many times a
> > + * certain event is occurring.
> > +*/
> > +static u64 zswap_pool_limit_hit;
> > +static u64 zswap_written_back_pages;
> > +static u64 zswap_reject_reclaim_fail;
> > +static u64 zswap_reject_compress_poor;
> > +static u64 zswap_reject_alloc_fail;
> > +static u64 zswap_reject_kmemcache_fail;
> > +static u64 zswap_duplicate_entry;
> > +
> > +/*********************************
> > +* tunables
> > +**********************************/
> > +/* Enable/disable zswap (disabled by default, fixed at boot for now) */
> > +static bool zswap_enabled;
> > +module_param_named(enabled, zswap_enabled, bool, 0);
> > +
> > +/* Compressor to be used by zswap (fixed at boot for now) */
> > +#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
> > +static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> > +module_param_named(compressor, zswap_compressor, charp, 0);
> > +
> > +/* The maximum percentage of memory that the compressed pool can occupy */
> > +static unsigned int zswap_max_pool_percent = 20;
> > +module_param_named(max_pool_percent,
> > + zswap_max_pool_percent, uint, 0644);
> > +
>
> I think it's reasonable but... see comments in zbud_reclaim_page().
>
> > +/*
> > + * Maximum compression ratio, as as percentage, for an acceptable
> > + * compressed page. Any pages that do not compress by at least
> > + * this ratio will be rejected.
> > +*/
> > +static unsigned int zswap_max_compression_ratio = 80;
> > +module_param_named(max_compression_ratio,
> > + zswap_max_compression_ratio, uint, 0644);
> > +
>
> Prefer not export it, it's hard for use to know what value should set to.

True, this is kind of a remnant from zsmalloc. I can remove it.

>
> > +/*********************************
> > +* compression functions
> > +**********************************/
> > +/* per-cpu compression transforms */
> > +static struct crypto_comp * __percpu *zswap_comp_pcpu_tfms;
> > +
> > +enum comp_op {
> > + ZSWAP_COMPOP_COMPRESS,
> > + ZSWAP_COMPOP_DECOMPRESS
> > +};
> > +
> > +static int zswap_comp_op(enum comp_op op, const u8 *src, unsigned int slen,
> > + u8 *dst, unsigned int *dlen)
> > +{
> > + struct crypto_comp *tfm;
> > + int ret;
> > +
> > + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, get_cpu());
> > + switch (op) {
> > + case ZSWAP_COMPOP_COMPRESS:
> > + ret = crypto_comp_compress(tfm, src, slen, dst, dlen);
> > + break;
> > + case ZSWAP_COMPOP_DECOMPRESS:
> > + ret = crypto_comp_decompress(tfm, src, slen, dst, dlen);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + }
> > +
> > + put_cpu();
> > + return ret;
> > +}
> > +
> > +static int __init zswap_comp_init(void)
> > +{
> > + if (!crypto_has_comp(zswap_compressor, 0, 0)) {
> > + pr_info("%s compressor not available\n", zswap_compressor);
> > + /* fall back to default compressor */
> > + zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> > + if (!crypto_has_comp(zswap_compressor, 0, 0))
> > + /* can't even load the default compressor */
> > + return -ENODEV;
> > + }
> > + pr_info("using %s compressor\n", zswap_compressor);
> > +
> > + /* alloc percpu transforms */
> > + zswap_comp_pcpu_tfms = alloc_percpu(struct crypto_comp *);
> > + if (!zswap_comp_pcpu_tfms)
> > + return -ENOMEM;
> > + return 0;
> > +}
> > +
> > +static void zswap_comp_exit(void)
> > +{
> > + /* free percpu transforms */
> > + if (zswap_comp_pcpu_tfms)
> > + free_percpu(zswap_comp_pcpu_tfms);
> > +}
> > +
> > +/*********************************
> > +* data structures
> > +**********************************/
> > +/*
> > + * struct zswap_entry
> > + *
> > + * This structure contains the metadata for tracking a single compressed
> > + * page within zswap.
> > + *
> > + * rbnode - links the entry into red-black tree for the appropriate swap type
> > + * refcount - the number of outstanding reference to the entry. This is needed
> > + * to protect against premature freeing of the entry by code
> > + * concurent calls to load, invalidate, and writeback. The lock
> > + * for the zswap_tree structure that contains the entry must
> > + * be held while changing the refcount. Since the lock must
> > + * be held, there is no reason to also make refcount atomic.
> > + * type - the swap type for the entry. Used to map back to the zswap_tree
> > + * structure that contains the entry.
> > + * offset - the swap offset for the entry. Index into the red-black tree.
> > + * handle - zsmalloc allocation handle that stores the compressed page data
> > + * length - the length in bytes of the compressed page data. Needed during
> > + * decompression
>
> The sequence is different from the struct define?

Yes, Mel pointed this out too and I forgot to make the change.

>
> > + */
> > +struct zswap_entry {
> > + struct rb_node rbnode;
> > + pgoff_t offset;
> > + int refcount;
> > + unsigned int length;
> > + unsigned long handle;
> > +};
> > +
> > +struct zswap_header {
> > + swp_entry_t swpentry;
> > +};
> > +
> > +/*
> > + * The tree lock in the zswap_tree struct protects a few things:
> > + * - the rbtree
> > + * - the refcount field of each entry in the tree
> > + */
> > +struct zswap_tree {
> > + struct rb_root rbroot;
> > + spinlock_t lock;
> > + struct zbud_pool *pool;
> > + unsigned type;
>
> It seems that zswap_tree->type have no usage for zswap.

Good catch. Seems that it isn't used anymore.

>
> > +};
> > +
> > +static struct zswap_tree *zswap_trees[MAX_SWAPFILES];
> > +
> > +/*********************************
> > +* zswap entry functions
> > +**********************************/
> > +#define ZSWAP_KMEM_CACHE_NAME "zswap_entry_cache"
> > +static struct kmem_cache *zswap_entry_cache;
> > +
> > +static inline int zswap_entry_cache_create(void)
> > +{
> > + zswap_entry_cache =
> > + kmem_cache_create(ZSWAP_KMEM_CACHE_NAME,
> > + sizeof(struct zswap_entry), 0, 0, NULL);
> > + return (zswap_entry_cache == NULL);
> > +}
> > +
> > +static inline void zswap_entry_cache_destory(void)
> > +{
> > + kmem_cache_destroy(zswap_entry_cache);
> > +}
> > +
> > +static inline struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp)
> > +{
> > + struct zswap_entry *entry;
> > + entry = kmem_cache_alloc(zswap_entry_cache, gfp);
> > + if (!entry)
> > + return NULL;
> > + entry->refcount = 1;
> > + return entry;
> > +}
> > +
> > +static inline void zswap_entry_cache_free(struct zswap_entry *entry)
> > +{
> > + kmem_cache_free(zswap_entry_cache, entry);
> > +}
> > +
> > +static inline void zswap_entry_get(struct zswap_entry *entry)
> > +{
> > + entry->refcount++;
> > +}
> > +
> > +static inline int zswap_entry_put(struct zswap_entry *entry)
> > +{
> > + entry->refcount--;
> > + return entry->refcount;
> > +}
>
> Better if have lock comments here.

will do.

>
> > +
> > +/*********************************
> > +* rbtree functions
> > +**********************************/
> > +static struct zswap_entry *zswap_rb_search(struct rb_root *root, pgoff_t offset)
> > +{
> > + struct rb_node *node = root->rb_node;
> > + struct zswap_entry *entry;
> > +
> > + while (node) {
> > + entry = rb_entry(node, struct zswap_entry, rbnode);
> > + if (entry->offset > offset)
> > + node = node->rb_left;
> > + else if (entry->offset < offset)
> > + node = node->rb_right;
> > + else
> > + return entry;
> > + }
> > + return NULL;
> > +}
> > +
> > +/*
> > + * In the case that a entry with the same offset is found, it a pointer to
> > + * the existing entry is stored in dupentry and the function returns -EEXIST
> > +*/
> > +static int zswap_rb_insert(struct rb_root *root, struct zswap_entry *entry,
> > + struct zswap_entry **dupentry)
> > +{
> > + struct rb_node **link = &root->rb_node, *parent = NULL;
> > + struct zswap_entry *myentry;
> > +
> > + while (*link) {
> > + parent = *link;
> > + myentry = rb_entry(parent, struct zswap_entry, rbnode);
> > + if (myentry->offset > entry->offset)
> > + link = &(*link)->rb_left;
> > + else if (myentry->offset < entry->offset)
> > + link = &(*link)->rb_right;
> > + else {
> > + *dupentry = myentry;
> > + return -EEXIST;
> > + }
> > + }
> > + rb_link_node(&entry->rbnode, parent, link);
> > + rb_insert_color(&entry->rbnode, root);
> > + return 0;
> > +}
> > +
> > +/*********************************
> > +* per-cpu code
> > +**********************************/
> > +static DEFINE_PER_CPU(u8 *, zswap_dstmem);
> > +
> > +static int __zswap_cpu_notifier(unsigned long action, unsigned long cpu)
> > +{
> > + struct crypto_comp *tfm;
> > + u8 *dst;
> > +
> > + switch (action) {
> > + case CPU_UP_PREPARE:
> > + tfm = crypto_alloc_comp(zswap_compressor, 0, 0);
> > + if (IS_ERR(tfm)) {
> > + pr_err("can't allocate compressor transform\n");
> > + return NOTIFY_BAD;
> > + }
> > + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = tfm;
> > + dst = kmalloc(PAGE_SIZE * 2, GFP_KERNEL);
> > + if (!dst) {
> > + pr_err("can't allocate compressor buffer\n");
> > + crypto_free_comp(tfm);
> > + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> > + return NOTIFY_BAD;
> > + }
> > + per_cpu(zswap_dstmem, cpu) = dst;
> > + break;
> > + case CPU_DEAD:
> > + case CPU_UP_CANCELED:
> > + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu);
> > + if (tfm) {
> > + crypto_free_comp(tfm);
> > + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> > + }
> > + dst = per_cpu(zswap_dstmem, cpu);
> > + kfree(dst);
> > + per_cpu(zswap_dstmem, cpu) = NULL;
> > + break;
> > + default:
> > + break;
> > + }
> > + return NOTIFY_OK;
> > +}
> > +
> > +static int zswap_cpu_notifier(struct notifier_block *nb,
> > + unsigned long action, void *pcpu)
> > +{
> > + unsigned long cpu = (unsigned long)pcpu;
> > + return __zswap_cpu_notifier(action, cpu);
> > +}
> > +
> > +static struct notifier_block zswap_cpu_notifier_block = {
> > + .notifier_call = zswap_cpu_notifier
> > +};
> > +
> > +static int zswap_cpu_init(void)
> > +{
> > + unsigned long cpu;
> > +
> > + get_online_cpus();
> > + for_each_online_cpu(cpu)
> > + if (__zswap_cpu_notifier(CPU_UP_PREPARE, cpu) != NOTIFY_OK)
> > + goto cleanup;
> > + register_cpu_notifier(&zswap_cpu_notifier_block);
> > + put_online_cpus();
> > + return 0;
> > +
> > +cleanup:
> > + for_each_online_cpu(cpu)
> > + __zswap_cpu_notifier(CPU_UP_CANCELED, cpu);
> > + put_online_cpus();
> > + return -ENOMEM;
> > +}
> > +
> > +/*********************************
> > +* helpers
> > +**********************************/
> > +static inline bool zswap_is_full(void)
> > +{
> > + int pool_pages = atomic_read(&zswap_pool_pages);
> > + return (totalram_pages * zswap_max_pool_percent / 100 < pool_pages);
> > +}
> > +
> > +/*
> > + * Carries out the common pattern of freeing and entry's zsmalloc allocation,
> > + * freeing the entry itself, and decrementing the number of stored pages.
> > + */
> > +static void zswap_free_entry(struct zswap_tree *tree, struct zswap_entry *entry)
> > +{
> > + zbud_free(tree->pool, entry->handle);
> > + zswap_entry_cache_free(entry);
> > + atomic_dec(&zswap_stored_pages);
> > + atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
> > +}
> > +
> > +/*********************************
> > +* writeback code
> > +**********************************/
> > +/* return enum for zswap_get_swap_cache_page */
> > +enum zswap_get_swap_ret {
> > + ZSWAP_SWAPCACHE_NEW,
> > + ZSWAP_SWAPCACHE_EXIST,
> > + ZSWAP_SWAPCACHE_NOMEM
> > +};
> > +
> > +/*
> > + * zswap_get_swap_cache_page
> > + *
> > + * This is an adaption of read_swap_cache_async()
> > + *
> > + * This function tries to find a page with the given swap entry
> > + * in the swapper_space address space (the swap cache). If the page
> > + * is found, it is returned in retpage. Otherwise, a page is allocated,
> > + * added to the swap cache, and returned in retpage.
> > + *
> > + * If success, the swap cache page is returned in retpage
> > + * Returns 0 if page was already in the swap cache, page is not locked
> > + * Returns 1 if the new page needs to be populated, page is locked
> > + * Returns <0 on error
> > + */
> > +static int zswap_get_swap_cache_page(swp_entry_t entry,
> > + struct page **retpage)
> > +{
> > + struct page *found_page, *new_page = NULL;
> > + struct address_space *swapper_space = &swapper_spaces[swp_type(entry)];
> > + int err;
> > +
> > + *retpage = NULL;
> > + do {
> > + /*
> > + * First check the swap cache. Since this is normally
> > + * called after lookup_swap_cache() failed, re-calling
> > + * that would confuse statistics.
> > + */
> > + found_page = find_get_page(swapper_space, entry.val);
> > + if (found_page)
> > + break;
> > +
> > + /*
> > + * Get a new page to read into from swap.
> > + */
> > + if (!new_page) {
> > + new_page = alloc_page(GFP_KERNEL);
> > + if (!new_page)
> > + break; /* Out of memory */
> > + }
> > +
> > + /*
> > + * call radix_tree_preload() while we can wait.
> > + */
> > + err = radix_tree_preload(GFP_KERNEL);
> > + if (err)
> > + break;
> > +
> > + /*
> > + * Swap entry may have been freed since our caller observed it.
> > + */
> > + err = swapcache_prepare(entry);
> > + if (err == -EEXIST) { /* seems racy */
> > + radix_tree_preload_end();
> > + continue;
> > + }
> > + if (err) { /* swp entry is obsolete ? */
> > + radix_tree_preload_end();
> > + break;
> > + }
> > +
> > + /* May fail (-ENOMEM) if radix-tree node allocation failed. */
> > + __set_page_locked(new_page);
> > + SetPageSwapBacked(new_page);
> > + err = __add_to_swap_cache(new_page, entry);
> > + if (likely(!err)) {
> > + radix_tree_preload_end();
> > + lru_cache_add_anon(new_page);
> > + *retpage = new_page;
> > + return ZSWAP_SWAPCACHE_NEW;
> > + }
> > + radix_tree_preload_end();
> > + ClearPageSwapBacked(new_page);
> > + __clear_page_locked(new_page);
> > + /*
> > + * add_to_swap_cache() doesn't return -EEXIST, so we can safely
> > + * clear SWAP_HAS_CACHE flag.
> > + */
> > + swapcache_free(entry, NULL);
> > + } while (err != -ENOMEM);
> > +
> > + if (new_page)
> > + page_cache_release(new_page);
> > + if (!found_page)
> > + return ZSWAP_SWAPCACHE_NOMEM;
> > + *retpage = found_page;
> > + return ZSWAP_SWAPCACHE_EXIST;
> > +}
> > +
> > +/*
> > + * Attempts to free and entry by adding a page to the swap cache,
> > + * decompressing the entry data into the page, and issuing a
> > + * bio write to write the page back to the swap device.
> > + *
> > + * This can be thought of as a "resumed writeback" of the page
> > + * to the swap device. We are basically resuming the same swap
> > + * writeback path that was intercepted with the frontswap_store()
> > + * in the first place. After the page has been decompressed into
> > + * the swap cache, the compressed version stored by zswap can be
> > + * freed.
> > + */
> > +static int zswap_writeback_entry(struct zbud_pool *pool, unsigned long handle)
> > +{
> > + struct zswap_header *zhdr;
> > + swp_entry_t swpentry;
> > + struct zswap_tree *tree;
> > + pgoff_t offset;
> > + struct zswap_entry *entry;
> > + struct page *page;
> > + u8 *src, *dst;
> > + unsigned int dlen;
> > + int ret, refcount;
> > + struct writeback_control wbc = {
> > + .sync_mode = WB_SYNC_NONE,
> > + };
> > +
> > + /* extract swpentry from data */
> > + zhdr = zbud_map(pool, handle);
> > + swpentry = zhdr->swpentry; /* here */
> > + zbud_unmap(pool, handle);
> > + tree = zswap_trees[swp_type(swpentry)];
> > + offset = swp_offset(swpentry);
> > + BUG_ON(pool != tree->pool);
> > +
> > + /* find and ref zswap entry */
> > + spin_lock(&tree->lock);
> > + entry = zswap_rb_search(&tree->rbroot, offset);
> > + if (!entry) {
> > + /* entry was invalidated */
> > + spin_unlock(&tree->lock);
> > + return 0;
> > + }
> > + zswap_entry_get(entry);
> > + spin_unlock(&tree->lock);
> > + BUG_ON(offset != entry->offset);
> > +
> > + /* try to allocate swap cache page */
> > + switch (zswap_get_swap_cache_page(swpentry, &page)) {
> > + case ZSWAP_SWAPCACHE_NOMEM: /* no memory */
> > + ret = -ENOMEM;
> > + goto fail;
> > +
> > + case ZSWAP_SWAPCACHE_EXIST: /* page is unlocked */
> > + /* page is already in the swap cache, ignore for now */
> > + page_cache_release(page);
> > + ret = -EEXIST;
> > + goto fail;
> > +
> > + case ZSWAP_SWAPCACHE_NEW: /* page is locked */
> > + /* decompress */
> > + dlen = PAGE_SIZE;
> > + src = (u8 *)zbud_map(tree->pool, entry->handle) +
> > + sizeof(struct zswap_header);
> > + dst = kmap_atomic(page);
> > + ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src,
> > + entry->length, dst, &dlen);
> > + kunmap_atomic(dst);
> > + zbud_unmap(tree->pool, entry->handle);
> > + BUG_ON(ret);
> > + BUG_ON(dlen != PAGE_SIZE);
> > +
> > + /* page is up to date */
> > + SetPageUptodate(page);
> > + }
> > +
> > + /* start writeback */
> > + SetPageReclaim(page);
> > + __swap_writepage(page, &wbc, end_swap_bio_write);
> > + page_cache_release(page);
> > + zswap_written_back_pages++;
> > +
> > + spin_lock(&tree->lock);
> > +
> > + /* drop local reference */
> > + zswap_entry_put(entry);
> > + /* drop the initial reference from entry creation */
> > + refcount = zswap_entry_put(entry);
> > +
> > + /*
> > + * There are three possible values for refcount here:
> > + * (1) refcount is 1, load is in progress, unlink from rbtree,
> > + * load will free
> > + * (2) refcount is 0, (normal case) entry is valid,
> > + * remove from rbtree and free entry
> > + * (3) refcount is -1, invalidate happened during writeback,
> > + * free entry
> > + */
> > + if (refcount >= 0) {
> > + /* no invalidate yet, remove from rbtree */
> > + rb_erase(&entry->rbnode, &tree->rbroot);
> > + }
> > + spin_unlock(&tree->lock);
> > + if (refcount <= 0) {
> > + /* free the entry */
> > + zswap_free_entry(tree, entry);
> > + return 0;
> > + }
> > + return -EAGAIN;
> > +
> > +fail:
> > + spin_lock(&tree->lock);
> > + zswap_entry_put(entry);
> > + spin_unlock(&tree->lock);
> > + return ret;
> > +}
> > +
> > +/*********************************
> > +* frontswap hooks
> > +**********************************/
> > +/* attempts to compress and store an single page */
> > +static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> > + struct page *page)
> > +{
> > + struct zswap_tree *tree = zswap_trees[type];
> > + struct zswap_entry *entry, *dupentry;
> > + int ret;
> > + unsigned int dlen = PAGE_SIZE, len;
> > + unsigned long handle;
> > + char *buf;
> > + u8 *src, *dst;
> > + struct zswap_header *zhdr;
> > +
> > + if (!tree) {
> > + ret = -ENODEV;
> > + goto reject;
> > + }
> > +
> > + /* reclaim space if needed */
> > + if (zswap_is_full()) {
> > + zswap_pool_limit_hit++;
> > + if (zbud_reclaim_page(tree->pool, 8)) {
>
> My idea is to wake up a kernel thread here to do the reclaim.
> Once zswap is full(20% percent of total mem currently), the kernel
> thread should reclaim pages from it. Not only reclaim one page, it
> should depend on the current memory pressure.
> And then the API in zbud may like this:
> zbud_reclaim_page(pool, nr_pages_to_reclaim, nr_retry);

So kswapd for zswap. I'm not opposed to the idea if a case can be
made for the complexity. I must say, I don't see that case though.

The policy can evolve as deficiencies are demonstrated and solutions are
found. Can I get your ack on this pending the other changes?

Thanks,
Seth

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sjenning at linux

May 14, 2013, 9:35 AM

Post #5 of 28 (594 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On Mon, May 13, 2013 at 03:31:42PM -0700, Dan Magenheimer wrote:
> > From: Seth Jennings [mailto:sjenning [at] linux]
> > Subject: [PATCHv11 3/4] zswap: add to mm/
> >
> > zswap is a thin compression backend for frontswap. It receives pages from
> > frontswap and attempts to store them in a compressed memory pool, resulting in
> > an effective partial memory reclaim and dramatically reduced swap device I/O.
> >
> > Additionally, in most cases, pages can be retrieved from this compressed store
> > much more quickly than reading from tradition swap devices resulting in faster
> > performance for many workloads.
> >
> > It also has support for evicting swap pages that are currently compressed in
> > zswap to the swap device on an LRU(ish) basis. This functionality is very
> > important and make zswap a true cache in that, once the cache is full or can't
> > grow due to memory pressure, the oldest pages can be moved out of zswap to the
> > swap device so newer pages can be compressed and stored in zswap.
> >
> > This patch adds the zswap driver to mm/
> >
> > Signed-off-by: Seth Jennings <sjenning [at] linux>
>
> A couple of comments below...

Thanks for the review!

>
> > ---
> > mm/Kconfig | 15 +
> > mm/Makefile | 1 +
> > mm/zswap.c | 952 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 968 insertions(+)
> > create mode 100644 mm/zswap.c
> >
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 908f41b..4042e07 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -487,3 +487,18 @@ config ZBUD
> > While this design limits storage density, it has simple and
> > deterministic reclaim properties that make it preferable to a higher
> > density approach when reclaim will be used.
> > +
> > +config ZSWAP
> > + bool "In-kernel swap page compression"
> > + depends on FRONTSWAP && CRYPTO
> > + select CRYPTO_LZO
> > + select ZBUD
> > + default n
> > + help
> > + Zswap is a backend for the frontswap mechanism in the VMM.
> > + It receives pages from frontswap and attempts to store them
> > + in a compressed memory pool, resulting in an effective
> > + partial memory reclaim. In addition, pages and be retrieved
> > + from this compressed store much faster than most tradition
> > + swap devices resulting in reduced I/O and faster performance
> > + for many workloads.
> > diff --git a/mm/Makefile b/mm/Makefile
> > index 95f0197..f008033 100644
> > --- a/mm/Makefile
> > +++ b/mm/Makefile
> > @@ -32,6 +32,7 @@ obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
> > obj-$(CONFIG_BOUNCE) += bounce.o
> > obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o
> > obj-$(CONFIG_FRONTSWAP) += frontswap.o
> > +obj-$(CONFIG_ZSWAP) += zswap.o
> > obj-$(CONFIG_HAS_DMA) += dmapool.o
> > obj-$(CONFIG_HUGETLBFS) += hugetlb.o
> > obj-$(CONFIG_NUMA) += mempolicy.o
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > new file mode 100644
> > index 0000000..b1070ca
> > --- /dev/null
> > +++ b/mm/zswap.c
> > @@ -0,0 +1,952 @@
> > +/*
> > + * zswap.c - zswap driver file
> > + *
> > + * zswap is a backend for frontswap that takes pages that are in the
> > + * process of being swapped out and attempts to compress them and store
> > + * them in a RAM-based memory pool. This results in a significant I/O
> > + * reduction on the real swap device and, in the case of a slow swap
> > + * device, can also improve workload performance.
> > + *
> > + * Copyright (C) 2012 Seth Jennings <sjenning [at] linux>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version 2
> > + * of the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > +*/
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/module.h>
> > +#include <linux/cpu.h>
> > +#include <linux/highmem.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> > +#include <linux/atomic.h>
> > +#include <linux/frontswap.h>
> > +#include <linux/rbtree.h>
> > +#include <linux/swap.h>
> > +#include <linux/crypto.h>
> > +#include <linux/mempool.h>
> > +#include <linux/zbud.h>
> > +
> > +#include <linux/mm_types.h>
> > +#include <linux/page-flags.h>
> > +#include <linux/swapops.h>
> > +#include <linux/writeback.h>
> > +#include <linux/pagemap.h>
> > +
> > +/*********************************
> > +* statistics
> > +**********************************/
> > +/* Number of memory pages used by the compressed pool */
> > +static atomic_t zswap_pool_pages = ATOMIC_INIT(0);
> > +/* The number of compressed pages currently stored in zswap */
> > +static atomic_t zswap_stored_pages = ATOMIC_INIT(0);
> > +
> > +/*
> > + * The statistics below are not protected from concurrent access for
> > + * performance reasons so they may not be a 100% accurate. However,
> > + * they do provide useful information on roughly how many times a
> > + * certain event is occurring.
> > +*/
> > +static u64 zswap_pool_limit_hit;
> > +static u64 zswap_written_back_pages;
> > +static u64 zswap_reject_reclaim_fail;
> > +static u64 zswap_reject_compress_poor;
> > +static u64 zswap_reject_alloc_fail;
> > +static u64 zswap_reject_kmemcache_fail;
> > +static u64 zswap_duplicate_entry;
> > +
> > +/*********************************
> > +* tunables
> > +**********************************/
> > +/* Enable/disable zswap (disabled by default, fixed at boot for now) */
> > +static bool zswap_enabled;
> > +module_param_named(enabled, zswap_enabled, bool, 0);
> > +
> > +/* Compressor to be used by zswap (fixed at boot for now) */
> > +#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
> > +static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> > +module_param_named(compressor, zswap_compressor, charp, 0);
> > +
> > +/* The maximum percentage of memory that the compressed pool can occupy */
> > +static unsigned int zswap_max_pool_percent = 20;
> > +module_param_named(max_pool_percent,
> > + zswap_max_pool_percent, uint, 0644);
>
> This limit, along with the code that enforces it (by calling reclaim
> when the limit is reached), is IMHO questionable. Is there any
> other kernel memory allocation that is constrained by a percentage
> of total memory rather than dynamically according to current
> system conditions? As Mel pointed out (approx.), if this limit
> is reached by a zswap-storm and filled with pages of long-running,
> rarely-used processes, 20% of RAM (by default here) becomes forever
> clogged.

So there are two comments here 1) dynamic pool limit and 2) writeback
of pages in zswap that won't be faulted in or forced out by pressure.

Comment 1 feeds from the point of view that compressed pages should just be
another type of memory managed by the core MM. While ideal, very hard to
implement in practice. We are starting to realize that even the policy
governing to active vs inactive list is very hard to get right. Then shrinkers
add more complexity to the policy problem. Throwing another type in the mix
would just that much more complex and hard to get right (assuming there even
_is_ a "right" policy for everyone in such a complex system).

This max_pool_percent policy is simple, works well, and provides a
deterministic policy that users can understand. Users can be assured that a
dynamic policy heuristic won't go nuts and allow the compressed pool to grow
unbounded or be so aggressively reclaimed that it offers no value.

Comment 2 I agree is an issue. I already have patches for a "periodic
writeback" functionality that starts to shrink the zswap pool via
writeback if zswap goes idle for a period of time. This addresses
the issue with long-lived, never-accessed pages getting stuck in
zswap forever.

>
> Zswap reclaim/writeback needs to be cognizant of (and perhaps driven
> by) system memory pressure, not some user-settable percentage.
> There's some tough policy questions that need to be answered here,
> perhaps not before zswap gets merged, but certainly before it
> gets enabled by default by distros.

Agreed that it shouldn't block merging. I guess the distros will
have to make the call if the policy is good enough.

>
> > +/*
> > + * Maximum compression ratio, as as percentage, for an acceptable
> > + * compressed page. Any pages that do not compress by at least
> > + * this ratio will be rejected.
> > +*/
> > +static unsigned int zswap_max_compression_ratio = 80;
> > +module_param_named(max_compression_ratio,
> > + zswap_max_compression_ratio, uint, 0644);
>
> Per earlier discussion, this number is actually derived
> from a zsmalloc constraint and doesn't necessarily apply
> to zbud. And I don't think any mortal user or system
> administrator would have any idea what value to change
> this to or the potential impact of changing it. IMHO
> it should be removed, or at least moved to and enforced
> by the specific allocator code.

Yes, Bob pointed this out too. I'm removing it.

Can I get your ack pending this change?

Thanks,
Seth

>
> > +/*********************************
> > +* compression functions
> > +**********************************/
> > +/* per-cpu compression transforms */
> > +static struct crypto_comp * __percpu *zswap_comp_pcpu_tfms;
> > +
> > +enum comp_op {
> > + ZSWAP_COMPOP_COMPRESS,
> > + ZSWAP_COMPOP_DECOMPRESS
> > +};
> > +
> > +static int zswap_comp_op(enum comp_op op, const u8 *src, unsigned int slen,
> > + u8 *dst, unsigned int *dlen)
> > +{
> > + struct crypto_comp *tfm;
> > + int ret;
> > +
> > + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, get_cpu());
> > + switch (op) {
> > + case ZSWAP_COMPOP_COMPRESS:
> > + ret = crypto_comp_compress(tfm, src, slen, dst, dlen);
> > + break;
> > + case ZSWAP_COMPOP_DECOMPRESS:
> > + ret = crypto_comp_decompress(tfm, src, slen, dst, dlen);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + }
> > +
> > + put_cpu();
> > + return ret;
> > +}
> > +
> > +static int __init zswap_comp_init(void)
> > +{
> > + if (!crypto_has_comp(zswap_compressor, 0, 0)) {
> > + pr_info("%s compressor not available\n", zswap_compressor);
> > + /* fall back to default compressor */
> > + zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> > + if (!crypto_has_comp(zswap_compressor, 0, 0))
> > + /* can't even load the default compressor */
> > + return -ENODEV;
> > + }
> > + pr_info("using %s compressor\n", zswap_compressor);
> > +
> > + /* alloc percpu transforms */
> > + zswap_comp_pcpu_tfms = alloc_percpu(struct crypto_comp *);
> > + if (!zswap_comp_pcpu_tfms)
> > + return -ENOMEM;
> > + return 0;
> > +}
> > +
> > +static void zswap_comp_exit(void)
> > +{
> > + /* free percpu transforms */
> > + if (zswap_comp_pcpu_tfms)
> > + free_percpu(zswap_comp_pcpu_tfms);
> > +}
> > +
> > +/*********************************
> > +* data structures
> > +**********************************/
> > +/*
> > + * struct zswap_entry
> > + *
> > + * This structure contains the metadata for tracking a single compressed
> > + * page within zswap.
> > + *
> > + * rbnode - links the entry into red-black tree for the appropriate swap type
> > + * refcount - the number of outstanding reference to the entry. This is needed
> > + * to protect against premature freeing of the entry by code
> > + * concurent calls to load, invalidate, and writeback. The lock
> > + * for the zswap_tree structure that contains the entry must
> > + * be held while changing the refcount. Since the lock must
> > + * be held, there is no reason to also make refcount atomic.
> > + * type - the swap type for the entry. Used to map back to the zswap_tree
> > + * structure that contains the entry.
> > + * offset - the swap offset for the entry. Index into the red-black tree.
> > + * handle - zsmalloc allocation handle that stores the compressed page data
> > + * length - the length in bytes of the compressed page data. Needed during
> > + * decompression
> > + */
> > +struct zswap_entry {
> > + struct rb_node rbnode;
> > + pgoff_t offset;
> > + int refcount;
> > + unsigned int length;
> > + unsigned long handle;
> > +};
> > +
> > +struct zswap_header {
> > + swp_entry_t swpentry;
> > +};
> > +
> > +/*
> > + * The tree lock in the zswap_tree struct protects a few things:
> > + * - the rbtree
> > + * - the refcount field of each entry in the tree
> > + */
> > +struct zswap_tree {
> > + struct rb_root rbroot;
> > + spinlock_t lock;
> > + struct zbud_pool *pool;
> > + unsigned type;
> > +};
> > +
> > +static struct zswap_tree *zswap_trees[MAX_SWAPFILES];
> > +
> > +/*********************************
> > +* zswap entry functions
> > +**********************************/
> > +#define ZSWAP_KMEM_CACHE_NAME "zswap_entry_cache"
> > +static struct kmem_cache *zswap_entry_cache;
> > +
> > +static inline int zswap_entry_cache_create(void)
> > +{
> > + zswap_entry_cache =
> > + kmem_cache_create(ZSWAP_KMEM_CACHE_NAME,
> > + sizeof(struct zswap_entry), 0, 0, NULL);
> > + return (zswap_entry_cache == NULL);
> > +}
> > +
> > +static inline void zswap_entry_cache_destory(void)
> > +{
> > + kmem_cache_destroy(zswap_entry_cache);
> > +}
> > +
> > +static inline struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp)
> > +{
> > + struct zswap_entry *entry;
> > + entry = kmem_cache_alloc(zswap_entry_cache, gfp);
> > + if (!entry)
> > + return NULL;
> > + entry->refcount = 1;
> > + return entry;
> > +}
> > +
> > +static inline void zswap_entry_cache_free(struct zswap_entry *entry)
> > +{
> > + kmem_cache_free(zswap_entry_cache, entry);
> > +}
> > +
> > +static inline void zswap_entry_get(struct zswap_entry *entry)
> > +{
> > + entry->refcount++;
> > +}
> > +
> > +static inline int zswap_entry_put(struct zswap_entry *entry)
> > +{
> > + entry->refcount--;
> > + return entry->refcount;
> > +}
> > +
> > +/*********************************
> > +* rbtree functions
> > +**********************************/
> > +static struct zswap_entry *zswap_rb_search(struct rb_root *root, pgoff_t offset)
> > +{
> > + struct rb_node *node = root->rb_node;
> > + struct zswap_entry *entry;
> > +
> > + while (node) {
> > + entry = rb_entry(node, struct zswap_entry, rbnode);
> > + if (entry->offset > offset)
> > + node = node->rb_left;
> > + else if (entry->offset < offset)
> > + node = node->rb_right;
> > + else
> > + return entry;
> > + }
> > + return NULL;
> > +}
> > +
> > +/*
> > + * In the case that a entry with the same offset is found, it a pointer to
> > + * the existing entry is stored in dupentry and the function returns -EEXIST
> > +*/
> > +static int zswap_rb_insert(struct rb_root *root, struct zswap_entry *entry,
> > + struct zswap_entry **dupentry)
> > +{
> > + struct rb_node **link = &root->rb_node, *parent = NULL;
> > + struct zswap_entry *myentry;
> > +
> > + while (*link) {
> > + parent = *link;
> > + myentry = rb_entry(parent, struct zswap_entry, rbnode);
> > + if (myentry->offset > entry->offset)
> > + link = &(*link)->rb_left;
> > + else if (myentry->offset < entry->offset)
> > + link = &(*link)->rb_right;
> > + else {
> > + *dupentry = myentry;
> > + return -EEXIST;
> > + }
> > + }
> > + rb_link_node(&entry->rbnode, parent, link);
> > + rb_insert_color(&entry->rbnode, root);
> > + return 0;
> > +}
> > +
> > +/*********************************
> > +* per-cpu code
> > +**********************************/
> > +static DEFINE_PER_CPU(u8 *, zswap_dstmem);
> > +
> > +static int __zswap_cpu_notifier(unsigned long action, unsigned long cpu)
> > +{
> > + struct crypto_comp *tfm;
> > + u8 *dst;
> > +
> > + switch (action) {
> > + case CPU_UP_PREPARE:
> > + tfm = crypto_alloc_comp(zswap_compressor, 0, 0);
> > + if (IS_ERR(tfm)) {
> > + pr_err("can't allocate compressor transform\n");
> > + return NOTIFY_BAD;
> > + }
> > + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = tfm;
> > + dst = kmalloc(PAGE_SIZE * 2, GFP_KERNEL);
> > + if (!dst) {
> > + pr_err("can't allocate compressor buffer\n");
> > + crypto_free_comp(tfm);
> > + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> > + return NOTIFY_BAD;
> > + }
> > + per_cpu(zswap_dstmem, cpu) = dst;
> > + break;
> > + case CPU_DEAD:
> > + case CPU_UP_CANCELED:
> > + tfm = *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu);
> > + if (tfm) {
> > + crypto_free_comp(tfm);
> > + *per_cpu_ptr(zswap_comp_pcpu_tfms, cpu) = NULL;
> > + }
> > + dst = per_cpu(zswap_dstmem, cpu);
> > + kfree(dst);
> > + per_cpu(zswap_dstmem, cpu) = NULL;
> > + break;
> > + default:
> > + break;
> > + }
> > + return NOTIFY_OK;
> > +}
> > +
> > +static int zswap_cpu_notifier(struct notifier_block *nb,
> > + unsigned long action, void *pcpu)
> > +{
> > + unsigned long cpu = (unsigned long)pcpu;
> > + return __zswap_cpu_notifier(action, cpu);
> > +}
> > +
> > +static struct notifier_block zswap_cpu_notifier_block = {
> > + .notifier_call = zswap_cpu_notifier
> > +};
> > +
> > +static int zswap_cpu_init(void)
> > +{
> > + unsigned long cpu;
> > +
> > + get_online_cpus();
> > + for_each_online_cpu(cpu)
> > + if (__zswap_cpu_notifier(CPU_UP_PREPARE, cpu) != NOTIFY_OK)
> > + goto cleanup;
> > + register_cpu_notifier(&zswap_cpu_notifier_block);
> > + put_online_cpus();
> > + return 0;
> > +
> > +cleanup:
> > + for_each_online_cpu(cpu)
> > + __zswap_cpu_notifier(CPU_UP_CANCELED, cpu);
> > + put_online_cpus();
> > + return -ENOMEM;
> > +}
> > +
> > +/*********************************
> > +* helpers
> > +**********************************/
> > +static inline bool zswap_is_full(void)
> > +{
> > + int pool_pages = atomic_read(&zswap_pool_pages);
> > + return (totalram_pages * zswap_max_pool_percent / 100 < pool_pages);
> > +}
> > +
> > +/*
> > + * Carries out the common pattern of freeing and entry's zsmalloc allocation,
> > + * freeing the entry itself, and decrementing the number of stored pages.
> > + */
> > +static void zswap_free_entry(struct zswap_tree *tree, struct zswap_entry *entry)
> > +{
> > + zbud_free(tree->pool, entry->handle);
> > + zswap_entry_cache_free(entry);
> > + atomic_dec(&zswap_stored_pages);
> > + atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
> > +}
> > +
> > +/*********************************
> > +* writeback code
> > +**********************************/
> > +/* return enum for zswap_get_swap_cache_page */
> > +enum zswap_get_swap_ret {
> > + ZSWAP_SWAPCACHE_NEW,
> > + ZSWAP_SWAPCACHE_EXIST,
> > + ZSWAP_SWAPCACHE_NOMEM
> > +};
> > +
> > +/*
> > + * zswap_get_swap_cache_page
> > + *
> > + * This is an adaption of read_swap_cache_async()
> > + *
> > + * This function tries to find a page with the given swap entry
> > + * in the swapper_space address space (the swap cache). If the page
> > + * is found, it is returned in retpage. Otherwise, a page is allocated,
> > + * added to the swap cache, and returned in retpage.
> > + *
> > + * If success, the swap cache page is returned in retpage
> > + * Returns 0 if page was already in the swap cache, page is not locked
> > + * Returns 1 if the new page needs to be populated, page is locked
> > + * Returns <0 on error
> > + */
> > +static int zswap_get_swap_cache_page(swp_entry_t entry,
> > + struct page **retpage)
> > +{
> > + struct page *found_page, *new_page = NULL;
> > + struct address_space *swapper_space = &swapper_spaces[swp_type(entry)];
> > + int err;
> > +
> > + *retpage = NULL;
> > + do {
> > + /*
> > + * First check the swap cache. Since this is normally
> > + * called after lookup_swap_cache() failed, re-calling
> > + * that would confuse statistics.
> > + */
> > + found_page = find_get_page(swapper_space, entry.val);
> > + if (found_page)
> > + break;
> > +
> > + /*
> > + * Get a new page to read into from swap.
> > + */
> > + if (!new_page) {
> > + new_page = alloc_page(GFP_KERNEL);
> > + if (!new_page)
> > + break; /* Out of memory */
> > + }
> > +
> > + /*
> > + * call radix_tree_preload() while we can wait.
> > + */
> > + err = radix_tree_preload(GFP_KERNEL);
> > + if (err)
> > + break;
> > +
> > + /*
> > + * Swap entry may have been freed since our caller observed it.
> > + */
> > + err = swapcache_prepare(entry);
> > + if (err == -EEXIST) { /* seems racy */
> > + radix_tree_preload_end();
> > + continue;
> > + }
> > + if (err) { /* swp entry is obsolete ? */
> > + radix_tree_preload_end();
> > + break;
> > + }
> > +
> > + /* May fail (-ENOMEM) if radix-tree node allocation failed. */
> > + __set_page_locked(new_page);
> > + SetPageSwapBacked(new_page);
> > + err = __add_to_swap_cache(new_page, entry);
> > + if (likely(!err)) {
> > + radix_tree_preload_end();
> > + lru_cache_add_anon(new_page);
> > + *retpage = new_page;
> > + return ZSWAP_SWAPCACHE_NEW;
> > + }
> > + radix_tree_preload_end();
> > + ClearPageSwapBacked(new_page);
> > + __clear_page_locked(new_page);
> > + /*
> > + * add_to_swap_cache() doesn't return -EEXIST, so we can safely
> > + * clear SWAP_HAS_CACHE flag.
> > + */
> > + swapcache_free(entry, NULL);
> > + } while (err != -ENOMEM);
> > +
> > + if (new_page)
> > + page_cache_release(new_page);
> > + if (!found_page)
> > + return ZSWAP_SWAPCACHE_NOMEM;
> > + *retpage = found_page;
> > + return ZSWAP_SWAPCACHE_EXIST;
> > +}
> > +
> > +/*
> > + * Attempts to free and entry by adding a page to the swap cache,
> > + * decompressing the entry data into the page, and issuing a
> > + * bio write to write the page back to the swap device.
> > + *
> > + * This can be thought of as a "resumed writeback" of the page
> > + * to the swap device. We are basically resuming the same swap
> > + * writeback path that was intercepted with the frontswap_store()
> > + * in the first place. After the page has been decompressed into
> > + * the swap cache, the compressed version stored by zswap can be
> > + * freed.
> > + */
> > +static int zswap_writeback_entry(struct zbud_pool *pool, unsigned long handle)
> > +{
> > + struct zswap_header *zhdr;
> > + swp_entry_t swpentry;
> > + struct zswap_tree *tree;
> > + pgoff_t offset;
> > + struct zswap_entry *entry;
> > + struct page *page;
> > + u8 *src, *dst;
> > + unsigned int dlen;
> > + int ret, refcount;
> > + struct writeback_control wbc = {
> > + .sync_mode = WB_SYNC_NONE,
> > + };
> > +
> > + /* extract swpentry from data */
> > + zhdr = zbud_map(pool, handle);
> > + swpentry = zhdr->swpentry; /* here */
> > + zbud_unmap(pool, handle);
> > + tree = zswap_trees[swp_type(swpentry)];
> > + offset = swp_offset(swpentry);
> > + BUG_ON(pool != tree->pool);
> > +
> > + /* find and ref zswap entry */
> > + spin_lock(&tree->lock);
> > + entry = zswap_rb_search(&tree->rbroot, offset);
> > + if (!entry) {
> > + /* entry was invalidated */
> > + spin_unlock(&tree->lock);
> > + return 0;
> > + }
> > + zswap_entry_get(entry);
> > + spin_unlock(&tree->lock);
> > + BUG_ON(offset != entry->offset);
> > +
> > + /* try to allocate swap cache page */
> > + switch (zswap_get_swap_cache_page(swpentry, &page)) {
> > + case ZSWAP_SWAPCACHE_NOMEM: /* no memory */
> > + ret = -ENOMEM;
> > + goto fail;
> > +
> > + case ZSWAP_SWAPCACHE_EXIST: /* page is unlocked */
> > + /* page is already in the swap cache, ignore for now */
> > + page_cache_release(page);
> > + ret = -EEXIST;
> > + goto fail;
> > +
> > + case ZSWAP_SWAPCACHE_NEW: /* page is locked */
> > + /* decompress */
> > + dlen = PAGE_SIZE;
> > + src = (u8 *)zbud_map(tree->pool, entry->handle) +
> > + sizeof(struct zswap_header);
> > + dst = kmap_atomic(page);
> > + ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src,
> > + entry->length, dst, &dlen);
> > + kunmap_atomic(dst);
> > + zbud_unmap(tree->pool, entry->handle);
> > + BUG_ON(ret);
> > + BUG_ON(dlen != PAGE_SIZE);
> > +
> > + /* page is up to date */
> > + SetPageUptodate(page);
> > + }
> > +
> > + /* start writeback */
> > + SetPageReclaim(page);
> > + __swap_writepage(page, &wbc, end_swap_bio_write);
> > + page_cache_release(page);
> > + zswap_written_back_pages++;
> > +
> > + spin_lock(&tree->lock);
> > +
> > + /* drop local reference */
> > + zswap_entry_put(entry);
> > + /* drop the initial reference from entry creation */
> > + refcount = zswap_entry_put(entry);
> > +
> > + /*
> > + * There are three possible values for refcount here:
> > + * (1) refcount is 1, load is in progress, unlink from rbtree,
> > + * load will free
> > + * (2) refcount is 0, (normal case) entry is valid,
> > + * remove from rbtree and free entry
> > + * (3) refcount is -1, invalidate happened during writeback,
> > + * free entry
> > + */
> > + if (refcount >= 0) {
> > + /* no invalidate yet, remove from rbtree */
> > + rb_erase(&entry->rbnode, &tree->rbroot);
> > + }
> > + spin_unlock(&tree->lock);
> > + if (refcount <= 0) {
> > + /* free the entry */
> > + zswap_free_entry(tree, entry);
> > + return 0;
> > + }
> > + return -EAGAIN;
> > +
> > +fail:
> > + spin_lock(&tree->lock);
> > + zswap_entry_put(entry);
> > + spin_unlock(&tree->lock);
> > + return ret;
> > +}
> > +
> > +/*********************************
> > +* frontswap hooks
> > +**********************************/
> > +/* attempts to compress and store an single page */
> > +static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> > + struct page *page)
> > +{
> > + struct zswap_tree *tree = zswap_trees[type];
> > + struct zswap_entry *entry, *dupentry;
> > + int ret;
> > + unsigned int dlen = PAGE_SIZE, len;
> > + unsigned long handle;
> > + char *buf;
> > + u8 *src, *dst;
> > + struct zswap_header *zhdr;
> > +
> > + if (!tree) {
> > + ret = -ENODEV;
> > + goto reject;
> > + }
> > +
> > + /* reclaim space if needed */
> > + if (zswap_is_full()) {
> > + zswap_pool_limit_hit++;
> > + if (zbud_reclaim_page(tree->pool, 8)) {
> > + zswap_reject_reclaim_fail++;
> > + ret = -ENOMEM;
> > + goto reject;
> > + }
> > + }
>
> See comment above about enforcing "full".
>
> (No further comments below... Thanks, Dan)
>
> > + /* allocate entry */
> > + entry = zswap_entry_cache_alloc(GFP_KERNEL);
> > + if (!entry) {
> > + zswap_reject_kmemcache_fail++;
> > + ret = -ENOMEM;
> > + goto reject;
> > + }
> > +
> > + /* compress */
> > + dst = get_cpu_var(zswap_dstmem);
> > + src = kmap_atomic(page);
> > + ret = zswap_comp_op(ZSWAP_COMPOP_COMPRESS, src, PAGE_SIZE, dst, &dlen);
> > + kunmap_atomic(src);
> > + if (ret) {
> > + ret = -EINVAL;
> > + goto freepage;
> > + }
> > + len = dlen + sizeof(struct zswap_header);
> > + if ((len * 100 / PAGE_SIZE) > zswap_max_compression_ratio) {
> > + zswap_reject_compress_poor++;
> > + ret = -E2BIG;
> > + goto freepage;
> > + }
> > +
> > + /* store */
> > + ret = zbud_alloc(tree->pool, len, __GFP_NORETRY | __GFP_NOWARN,
> > + &handle);
> > + if (ret) {
> > + zswap_reject_alloc_fail++;
> > + goto freepage;
> > + }
> > + zhdr = zbud_map(tree->pool, handle);
> > + zhdr->swpentry = swp_entry(type, offset);
> > + buf = (u8 *)(zhdr + 1);
> > + memcpy(buf, dst, dlen);
> > + zbud_unmap(tree->pool, handle);
> > + put_cpu_var(zswap_dstmem);
> > +
> > + /* populate entry */
> > + entry->offset = offset;
> > + entry->handle = handle;
> > + entry->length = dlen;
> > +
> > + /* map */
> > + spin_lock(&tree->lock);
> > + do {
> > + ret = zswap_rb_insert(&tree->rbroot, entry, &dupentry);
> > + if (ret == -EEXIST) {
> > + zswap_duplicate_entry++;
> > + /* remove from rbtree */
> > + rb_erase(&dupentry->rbnode, &tree->rbroot);
> > + if (!zswap_entry_put(dupentry)) {
> > + /* free */
> > + zswap_free_entry(tree, dupentry);
> > + }
> > + }
> > + } while (ret == -EEXIST);
> > + spin_unlock(&tree->lock);
> > +
> > + /* update stats */
> > + atomic_inc(&zswap_stored_pages);
> > + atomic_set(&zswap_pool_pages, zbud_get_pool_size(tree->pool));
> > +
> > + return 0;
> > +
> > +freepage:
> > + put_cpu_var(zswap_dstmem);
> > + zswap_entry_cache_free(entry);
> > +reject:
> > + return ret;
> > +}
> > +
> > +/*
> > + * returns 0 if the page was successfully decompressed
> > + * return -1 on entry not found or error
> > +*/
> > +static int zswap_frontswap_load(unsigned type, pgoff_t offset,
> > + struct page *page)
> > +{
> > + struct zswap_tree *tree = zswap_trees[type];
> > + struct zswap_entry *entry;
> > + u8 *src, *dst;
> > + unsigned int dlen;
> > + int refcount, ret;
> > +
> > + /* find */
> > + spin_lock(&tree->lock);
> > + entry = zswap_rb_search(&tree->rbroot, offset);
> > + if (!entry) {
> > + /* entry was written back */
> > + spin_unlock(&tree->lock);
> > + return -1;
> > + }
> > + zswap_entry_get(entry);
> > + spin_unlock(&tree->lock);
> > +
> > + /* decompress */
> > + dlen = PAGE_SIZE;
> > + src = (u8 *)zbud_map(tree->pool, entry->handle) +
> > + sizeof(struct zswap_header);
> > + dst = kmap_atomic(page);
> > + ret = zswap_comp_op(ZSWAP_COMPOP_DECOMPRESS, src, entry->length,
> > + dst, &dlen);
> > + kunmap_atomic(dst);
> > + zbud_unmap(tree->pool, entry->handle);
> > + BUG_ON(ret);
> > +
> > + spin_lock(&tree->lock);
> > + refcount = zswap_entry_put(entry);
> > + if (likely(refcount)) {
> > + spin_unlock(&tree->lock);
> > + return 0;
> > + }
> > + spin_unlock(&tree->lock);
> > +
> > + /*
> > + * We don't have to unlink from the rbtree because
> > + * zswap_writeback_entry() or zswap_frontswap_invalidate page()
> > + * has already done this for us if we are the last reference.
> > + */
> > + /* free */
> > +
> > + zswap_free_entry(tree, entry);
> > +
> > + return 0;
> > +}
> > +
> > +/* invalidates a single page */
> > +static void zswap_frontswap_invalidate_page(unsigned type, pgoff_t offset)
> > +{
> > + struct zswap_tree *tree = zswap_trees[type];
> > + struct zswap_entry *entry;
> > + int refcount;
> > +
> > + /* find */
> > + spin_lock(&tree->lock);
> > + entry = zswap_rb_search(&tree->rbroot, offset);
> > + if (!entry) {
> > + /* entry was written back */
> > + spin_unlock(&tree->lock);
> > + return;
> > + }
> > +
> > + /* remove from rbtree */
> > + rb_erase(&entry->rbnode, &tree->rbroot);
> > +
> > + /* drop the initial reference from entry creation */
> > + refcount = zswap_entry_put(entry);
> > +
> > + spin_unlock(&tree->lock);
> > +
> > + if (refcount) {
> > + /* writeback in progress, writeback will free */
> > + return;
> > + }
> > +
> > + /* free */
> > + zswap_free_entry(tree, entry);
> > +}
> > +
> > +/* invalidates all pages for the given swap type */
> > +static void zswap_frontswap_invalidate_area(unsigned type)
> > +{
> > + struct zswap_tree *tree = zswap_trees[type];
> > + struct rb_node *node;
> > + struct zswap_entry *entry;
> > +
> > + if (!tree)
> > + return;
> > +
> > + /* walk the tree and free everything */
> > + spin_lock(&tree->lock);
> > + /*
> > + * TODO: Even though this code should not be executed because
> > + * the try_to_unuse() in swapoff should have emptied the tree,
> > + * it is very wasteful to rebalance the tree after every
> > + * removal when we are freeing the whole tree.
> > + *
> > + * If post-order traversal code is ever added to the rbtree
> > + * implementation, it should be used here.
> > + */
> > + while ((node = rb_first(&tree->rbroot))) {
> > + entry = rb_entry(node, struct zswap_entry, rbnode);
> > + rb_erase(&entry->rbnode, &tree->rbroot);
> > + zbud_free(tree->pool, entry->handle);
> > + zswap_entry_cache_free(entry);
> > + atomic_dec(&zswap_stored_pages);
> > + }
> > + tree->rbroot = RB_ROOT;
> > + spin_unlock(&tree->lock);
> > +}
> > +
> > +static struct zbud_ops zswap_zbud_ops = {
> > + .evict = zswap_writeback_entry
> > +};
> > +
> > +/* NOTE: this is called in atomic context from swapon and must not sleep */
> > +static void zswap_frontswap_init(unsigned type)
> > +{
> > + struct zswap_tree *tree;
> > +
> > + tree = kzalloc(sizeof(struct zswap_tree), GFP_ATOMIC);
> > + if (!tree)
> > + goto err;
> > + tree->pool = zbud_create_pool(GFP_NOWAIT, &zswap_zbud_ops);
> > + if (!tree->pool)
> > + goto freetree;
> > + tree->rbroot = RB_ROOT;
> > + spin_lock_init(&tree->lock);
> > + tree->type = type;
> > + zswap_trees[type] = tree;
> > + return;
> > +
> > +freetree:
> > + kfree(tree);
> > +err:
> > + pr_err("alloc failed, zswap disabled for swap type %d\n", type);
> > +}
> > +
> > +static struct frontswap_ops zswap_frontswap_ops = {
> > + .store = zswap_frontswap_store,
> > + .load = zswap_frontswap_load,
> > + .invalidate_page = zswap_frontswap_invalidate_page,
> > + .invalidate_area = zswap_frontswap_invalidate_area,
> > + .init = zswap_frontswap_init
> > +};
> > +
> > +/*********************************
> > +* debugfs functions
> > +**********************************/
> > +#ifdef CONFIG_DEBUG_FS
> > +#include <linux/debugfs.h>
> > +
> > +static struct dentry *zswap_debugfs_root;
> > +
> > +static int __init zswap_debugfs_init(void)
> > +{
> > + if (!debugfs_initialized())
> > + return -ENODEV;
> > +
> > + zswap_debugfs_root = debugfs_create_dir("zswap", NULL);
> > + if (!zswap_debugfs_root)
> > + return -ENOMEM;
> > +
> > + debugfs_create_u64("pool_limit_hit", S_IRUGO,
> > + zswap_debugfs_root, &zswap_pool_limit_hit);
> > + debugfs_create_u64("reject_reclaim_fail", S_IRUGO,
> > + zswap_debugfs_root, &zswap_reject_reclaim_fail);
> > + debugfs_create_u64("reject_alloc_fail", S_IRUGO,
> > + zswap_debugfs_root, &zswap_reject_alloc_fail);
> > + debugfs_create_u64("reject_kmemcache_fail", S_IRUGO,
> > + zswap_debugfs_root, &zswap_reject_kmemcache_fail);
> > + debugfs_create_u64("reject_compress_poor", S_IRUGO,
> > + zswap_debugfs_root, &zswap_reject_compress_poor);
> > + debugfs_create_u64("written_back_pages", S_IRUGO,
> > + zswap_debugfs_root, &zswap_written_back_pages);
> > + debugfs_create_u64("duplicate_entry", S_IRUGO,
> > + zswap_debugfs_root, &zswap_duplicate_entry);
> > + debugfs_create_atomic_t("pool_pages", S_IRUGO,
> > + zswap_debugfs_root, &zswap_pool_pages);
> > + debugfs_create_atomic_t("stored_pages", S_IRUGO,
> > + zswap_debugfs_root, &zswap_stored_pages);
> > +
> > + return 0;
> > +}
> > +
> > +static void __exit zswap_debugfs_exit(void)
> > +{
> > + debugfs_remove_recursive(zswap_debugfs_root);
> > +}
> > +#else
> > +static inline int __init zswap_debugfs_init(void)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void __exit zswap_debugfs_exit(void) { }
> > +#endif
> > +
> > +/*********************************
> > +* module init and exit
> > +**********************************/
> > +static int __init init_zswap(void)
> > +{
> > + if (!zswap_enabled)
> > + return 0;
> > +
> > + pr_info("loading zswap\n");
> > + if (zswap_entry_cache_create()) {
> > + pr_err("entry cache creation failed\n");
> > + goto error;
> > + }
> > + if (zswap_comp_init()) {
> > + pr_err("compressor initialization failed\n");
> > + goto compfail;
> > + }
> > + if (zswap_cpu_init()) {
> > + pr_err("per-cpu initialization failed\n");
> > + goto pcpufail;
> > + }
> > + frontswap_register_ops(&zswap_frontswap_ops);
> > + if (zswap_debugfs_init())
> > + pr_warn("debugfs initialization failed\n");
> > + return 0;
> > +pcpufail:
> > + zswap_comp_exit();
> > +compfail:
> > + zswap_entry_cache_destory();
> > +error:
> > + return -ENOMEM;
> > +}
> > +/* must be late so crypto has time to come up */
> > +late_initcall(init_zswap);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Seth Jennings <sjenning [at] linux>");
> > +MODULE_DESCRIPTION("Compressed cache for swap pages");
> > --
> > 1.7.9.5
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo [at] kvack For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont [at] kvack"> email [at] kvack </a>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 14, 2013, 9:37 AM

Post #6 of 28 (594 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Seth Jennings [mailto:sjenning [at] linux]
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On Tue, May 14, 2013 at 05:19:19PM +0800, Bob Liu wrote:
> > Hi Seth,
>
> Hi Bob, thanks for the review!
>
> >
> > > + /* reclaim space if needed */
> > > + if (zswap_is_full()) {
> > > + zswap_pool_limit_hit++;
> > > + if (zbud_reclaim_page(tree->pool, 8)) {
> >
> > My idea is to wake up a kernel thread here to do the reclaim.
> > Once zswap is full(20% percent of total mem currently), the kernel
> > thread should reclaim pages from it. Not only reclaim one page, it
> > should depend on the current memory pressure.
> > And then the API in zbud may like this:
> > zbud_reclaim_page(pool, nr_pages_to_reclaim, nr_retry);
>
> So kswapd for zswap. I'm not opposed to the idea if a case can be
> made for the complexity. I must say, I don't see that case though.
>
> The policy can evolve as deficiencies are demonstrated and solutions are
> found.

Hmmm... it is fairly easy to demonstrate the deficiency if
one tries. I actually first saw it occur on a real (though
early) EL6 system which started some graphics-related service
that caused a very brief swapstorm that was invisible during
normal boot but clogged up RAM with compressed pages which
later caused reduced weird benchmarking performance.

I think Mel's unpredictability concern applies equally here...
this may be a "long-term source of bugs and strange memory
management behavior."

> Can I get your ack on this pending the other changes?

I'd like to hear Mel's feedback about this, but perhaps
a compromise to allow for zswap merging would be to add
something like the following to zswap's Kconfig comment:

"Zswap reclaim policy is still primitive. Until it improves,
zswap should be considered experimental and is not recommended
for production use."

If Mel agrees with the unpredictability and also agrees
with the Kconfig compromise, I am willing to ack.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sjenning at linux

May 14, 2013, 10:28 AM

Post #7 of 28 (594 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On Tue, May 14, 2013 at 09:37:08AM -0700, Dan Magenheimer wrote:
> > From: Seth Jennings [mailto:sjenning [at] linux]
> > Subject: Re: [PATCHv11 3/4] zswap: add to mm/
> >
> > On Tue, May 14, 2013 at 05:19:19PM +0800, Bob Liu wrote:
> > > Hi Seth,
> >
> > Hi Bob, thanks for the review!
> >
> > >
> > > > + /* reclaim space if needed */
> > > > + if (zswap_is_full()) {
> > > > + zswap_pool_limit_hit++;
> > > > + if (zbud_reclaim_page(tree->pool, 8)) {
> > >
> > > My idea is to wake up a kernel thread here to do the reclaim.
> > > Once zswap is full(20% percent of total mem currently), the kernel
> > > thread should reclaim pages from it. Not only reclaim one page, it
> > > should depend on the current memory pressure.
> > > And then the API in zbud may like this:
> > > zbud_reclaim_page(pool, nr_pages_to_reclaim, nr_retry);
> >
> > So kswapd for zswap. I'm not opposed to the idea if a case can be
> > made for the complexity. I must say, I don't see that case though.
> >
> > The policy can evolve as deficiencies are demonstrated and solutions are
> > found.
>
> Hmmm... it is fairly easy to demonstrate the deficiency if
> one tries. I actually first saw it occur on a real (though
> early) EL6 system which started some graphics-related service
> that caused a very brief swapstorm that was invisible during
> normal boot but clogged up RAM with compressed pages which
> later caused reduced weird benchmarking performance.

Without any specifics, I'm not sure what I can do with this.

I'm hearing you say that the source of the benchmark degradation
are the idle pages in zswap. In that case, the periodic writeback
patches I have in the wings should address this.

I think we are on the same page without realizing it. Right now
zswap supports a kind of "direct reclaim" model at allocation time.
The periodic writeback patches will handle the proactive writeback
part to free up the zswap pool when it has idle pages in it.

>
> I think Mel's unpredictability concern applies equally here...
> this may be a "long-term source of bugs and strange memory
> management behavior."
>
> > Can I get your ack on this pending the other changes?
>
> I'd like to hear Mel's feedback about this, but perhaps
> a compromise to allow for zswap merging would be to add
> something like the following to zswap's Kconfig comment:
>
> "Zswap reclaim policy is still primitive. Until it improves,
> zswap should be considered experimental and is not recommended
> for production use."

Just for the record, an "experimental" tag in the Kconfig won't
work for me.

The reclaim policy for zswap is not primitive, it's simple. There
is a difference. Plus zswap is already runtime disabled by default.
If distros/customers enabled it, it is because they purposely
enabled it.

Seth

>
> If Mel agrees with the unpredictability and also agrees
> with the Kconfig compromise, I am willing to ack.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo [at] kvack For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont [at] kvack"> email [at] kvack </a>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 14, 2013, 1:18 PM

Post #8 of 28 (594 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Seth Jennings [mailto:sjenning [at] linux]
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> <snip>
>
> > > +/* The maximum percentage of memory that the compressed pool can occupy */
> > > +static unsigned int zswap_max_pool_percent = 20;
> > > +module_param_named(max_pool_percent,
> > > + zswap_max_pool_percent, uint, 0644);
>
> <snip>
>
> > This limit, along with the code that enforces it (by calling reclaim
> > when the limit is reached), is IMHO questionable. Is there any
> > other kernel memory allocation that is constrained by a percentage
> > of total memory rather than dynamically according to current
> > system conditions? As Mel pointed out (approx.), if this limit
> > is reached by a zswap-storm and filled with pages of long-running,
> > rarely-used processes, 20% of RAM (by default here) becomes forever
> > clogged.
>
> So there are two comments here 1) dynamic pool limit and 2) writeback
> of pages in zswap that won't be faulted in or forced out by pressure.
>
> Comment 1 feeds from the point of view that compressed pages should just be
> another type of memory managed by the core MM. While ideal, very hard to
> implement in practice. We are starting to realize that even the policy
> governing to active vs inactive list is very hard to get right. Then shrinkers
> add more complexity to the policy problem. Throwing another type in the mix
> would just that much more complex and hard to get right (assuming there even
> _is_ a "right" policy for everyone in such a complex system).
>
> This max_pool_percent policy is simple, works well, and provides a
> deterministic policy that users can understand. Users can be assured that a
> dynamic policy heuristic won't go nuts and allow the compressed pool to grow
> unbounded or be so aggressively reclaimed that it offers no value.

Hi Seth --

Hmmm... I'm not sure how to politely say "bullshit". :-)

The default 20% was randomly pulled out of the air long ago for zcache
experiments. If you can explain why 20% is better than 19% or 21%, or
better than 10% or 30% or even 50%, that would be a start. Then please try
to explain -- in terms an average sysadmin can understand -- under
what circumstances this number should be higher or lower, that would
be even better. In fact if you can explain it in even very broadbrush
terms like "higher for embedded" and "lower for server" that would be
useful. If the top Linux experts in compression can't answer these
questions (and the default is a random number, which it is), I don't
know how we can expect users to be "assured".

What you mean is "works well"... on the two benchmarks you've tried it
on. You say it's too hard to do dynamically... even though every other
significant RAM user in the kernel has to do it dynamically.
Workloads are dynamic and heavy users of RAM needs to deal with that.
You don't see a limit on the number of anonymous pages in the MM subsystem,
and you don't see a limit on the number of inodes in btrfs. Linus
would rightfully barf all over those limits and (if he was paying attention
to this discussion) he would barf on this limit too.

It's unfortunate that my proposed topic for LSFMM was pre-empted
by the zsmalloc vs zbud discussion and zswap vs zcache, because
I think the real challenge of zswap (or zcache) and the value to
distros and end users requires us to get this right BEFORE users
start filing bugs about performance weirdness. After which most
users and distros will simply default to 0% (i.e. turn zswap off)
because zswap unpredictably sometimes sucks.

<flame off> sorry...

> Comment 2 I agree is an issue. I already have patches for a "periodic
> writeback" functionality that starts to shrink the zswap pool via
> writeback if zswap goes idle for a period of time. This addresses
> the issue with long-lived, never-accessed pages getting stuck in
> zswap forever.

Pulling the call out of zswap_frontswap_store() (and ensuring there still
aren't any new races) would be a good start. But this is just a mechanism;
you haven't said anything about the policy or how you intend to
enforce the policy. Which just gets us back to Comment 1...

So Comment 1 and Comment 2 are really the same: How do we appropriately
manage the number of pages in the system that are used for storing
compressed pages?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 14, 2013, 1:54 PM

Post #9 of 28 (596 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Seth Jennings [mailto:sjenning [at] linux]
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On Tue, May 14, 2013 at 09:37:08AM -0700, Dan Magenheimer wrote:
> > > From: Seth Jennings [mailto:sjenning [at] linux]
> > > Subject: Re: [PATCHv11 3/4] zswap: add to mm/
> > >
> > > On Tue, May 14, 2013 at 05:19:19PM +0800, Bob Liu wrote:
> > > > Hi Seth,
> > >
> > > Hi Bob, thanks for the review!
> > >
> > > >
> > > > > + /* reclaim space if needed */
> > > > > + if (zswap_is_full()) {
> > > > > + zswap_pool_limit_hit++;
> > > > > + if (zbud_reclaim_page(tree->pool, 8)) {
> > > >
> > > > My idea is to wake up a kernel thread here to do the reclaim.
> > > > Once zswap is full(20% percent of total mem currently), the kernel
> > > > thread should reclaim pages from it. Not only reclaim one page, it
> > > > should depend on the current memory pressure.
> > > > And then the API in zbud may like this:
> > > > zbud_reclaim_page(pool, nr_pages_to_reclaim, nr_retry);
> > >
> > > So kswapd for zswap. I'm not opposed to the idea if a case can be
> > > made for the complexity. I must say, I don't see that case though.
> > >
> > > The policy can evolve as deficiencies are demonstrated and solutions are
> > > found.
> >
> > Hmmm... it is fairly easy to demonstrate the deficiency if
> > one tries. I actually first saw it occur on a real (though
> > early) EL6 system which started some graphics-related service
> > that caused a very brief swapstorm that was invisible during
> > normal boot but clogged up RAM with compressed pages which
> > later caused reduced weird benchmarking performance.
>
> Without any specifics, I'm not sure what I can do with this.

Well, I think its customary for the author of a patch to know
the limitations of the patch. I suggest you synthesize a
workload that attempts to measure worst case. That's exactly
what I did a year ago that led me to the realization that
zcache needed to solve some issues before it was ready to
promote out of staging.

> I'm hearing you say that the source of the benchmark degradation
> are the idle pages in zswap. In that case, the periodic writeback
> patches I have in the wings should address this.
>
> I think we are on the same page without realizing it. Right now
> zswap supports a kind of "direct reclaim" model at allocation time.
> The periodic writeback patches will handle the proactive writeback
> part to free up the zswap pool when it has idle pages in it.

I don't think we are on the same page though maybe you are heading
in the same direction now. I won't repeat the comments from the
previous email.

> > I think Mel's unpredictability concern applies equally here...
> > this may be a "long-term source of bugs and strange memory
> > management behavior."
> >
> > > Can I get your ack on this pending the other changes?
> >
> > I'd like to hear Mel's feedback about this, but perhaps
> > a compromise to allow for zswap merging would be to add
> > something like the following to zswap's Kconfig comment:
> >
> > "Zswap reclaim policy is still primitive. Until it improves,
> > zswap should be considered experimental and is not recommended
> > for production use."
>
> Just for the record, an "experimental" tag in the Kconfig won't
> work for me.
>
> The reclaim policy for zswap is not primitive, it's simple. There
> is a difference. Plus zswap is already runtime disabled by default.
> If distros/customers enabled it, it is because they purposely
> enabled it.

Hmmm... I think you are proposing to users/distros the following
use model: "If zswap works for you, turn it on. If it sucks,
turn it off. I can't tell you in advance whether it will work
or suck for your distro/workload, but it will probably work so
please try it."

That sounds awfully experimental to me.

The problem is not simple. Your solution is simple because
you are simply pretending that the harder parts of the problem
don't exist.

Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sjenning at linux

May 14, 2013, 3:55 PM

Post #10 of 28 (596 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On Tue, May 14, 2013 at 01:18:48PM -0700, Dan Magenheimer wrote:
> > From: Seth Jennings [mailto:sjenning [at] linux]
> > Subject: Re: [PATCHv11 3/4] zswap: add to mm/
> >
> > <snip>
> >
> > > > +/* The maximum percentage of memory that the compressed pool can occupy */
> > > > +static unsigned int zswap_max_pool_percent = 20;
> > > > +module_param_named(max_pool_percent,
> > > > + zswap_max_pool_percent, uint, 0644);
> >
> > <snip>
> >
> > > This limit, along with the code that enforces it (by calling reclaim
> > > when the limit is reached), is IMHO questionable. Is there any
> > > other kernel memory allocation that is constrained by a percentage
> > > of total memory rather than dynamically according to current
> > > system conditions? As Mel pointed out (approx.), if this limit
> > > is reached by a zswap-storm and filled with pages of long-running,
> > > rarely-used processes, 20% of RAM (by default here) becomes forever
> > > clogged.
> >
> > So there are two comments here 1) dynamic pool limit and 2) writeback
> > of pages in zswap that won't be faulted in or forced out by pressure.
> >
> > Comment 1 feeds from the point of view that compressed pages should just be
> > another type of memory managed by the core MM. While ideal, very hard to
> > implement in practice. We are starting to realize that even the policy
> > governing to active vs inactive list is very hard to get right. Then shrinkers
> > add more complexity to the policy problem. Throwing another type in the mix
> > would just that much more complex and hard to get right (assuming there even
> > _is_ a "right" policy for everyone in such a complex system).
> >
> > This max_pool_percent policy is simple, works well, and provides a
> > deterministic policy that users can understand. Users can be assured that a
> > dynamic policy heuristic won't go nuts and allow the compressed pool to grow
> > unbounded or be so aggressively reclaimed that it offers no value.
>
> Hi Seth --
>
> Hmmm... I'm not sure how to politely say "bullshit". :-)
>
> The default 20% was randomly pulled out of the air long ago for zcache
> experiments. If you can explain why 20% is better than 19% or 21%, or
> better than 10% or 30% or even 50%, that would be a start. Then please try
> to explain -- in terms an average sysadmin can understand -- under
> what circumstances this number should be higher or lower, that would
> be even better. In fact if you can explain it in even very broadbrush
> terms like "higher for embedded" and "lower for server" that would be
> useful. If the top Linux experts in compression can't answer these
> questions (and the default is a random number, which it is), I don't
> know how we can expect users to be "assured".

20% is a default maximum. There really isn't a particular reason for the
selection other than to supply reasonable default to a tunable. 20% is enough
to show the benefit while assuring the user zswap won't eat more than that
amount of memory under any circumstance. The point is to make it a tunable,
not to launch an incredibly in-depth study on what the default should be.

As guidance on how to tune it, switching to zbud actually made the math simpler
by bounding the best case to 2 and the expected density to very near 2. I have
two methods, one based on calculation and another based on experimentation.

Yes, I understand that there are many things to consider, but for the sake of
those that honestly care about the answer to the question, I'll answer it.

Method 1:

If you have a workload running on a machine with x GB of RAM and an anonymous
working set of y GB of pages where x < y, a good starting point for
max_pool_percent is ((y/x)-1)*100.

For example, if you have 10GB of RAM and 12GB anon working set, (12/10-1)*100 =
20. During operation there would be 8GB in uncompressed memory, and 4GB worth
of compressed memory occupying 2GB (i.e. 20%) of RAM. This will reduce swap I/O
to near zero assuming the pages compress <50% on average.

Bear in mind that this formula provides a lower bound on max_pool_percent if
you want to avoid swap I/0. Setting max_pool_percent to >20 would produce
the same situation.

Method 2:

Experimentally, one can just watch swap I/O rates while the workload is running
and increase max_pool_percent until no (or acceptable level of) swap I/O is
occurring.

As max_pool_percent increases, however, there is less and less room for
uncompressed memory, the only kind of memory on which the kernel can actually
operate. Compression/decompression activity might start dominating over useful
work. Going over 80 is probably not advised. If swap I/O is still observed
for high values of max_pool_percent, then the memory load should be reduced,
memory capacity should be increased, or performance degradation should be accepted.

>
> What you mean is "works well"... on the two benchmarks you've tried it
> on. You say it's too hard to do dynamically... even though every other
> significant RAM user in the kernel has to do it dynamically.
> Workloads are dynamic and heavy users of RAM needs to deal with that.
> You don't see a limit on the number of anonymous pages in the MM subsystem,
> and you don't see a limit on the number of inodes in btrfs. Linus
> would rightfully barf all over those limits and (if he was paying attention
> to this discussion) he would barf on this limit too.

Putting a user-tunable hard limit on the size of the compressed pool is in _no
way_ analogous to putting an fixed upper bound on system-wide anonymous memory
or number of inodes. In fact, they are so dissimilar, I don't know what else to
say about the attempted comparison.

zswap is not like other caches in the kernel. Most caches make use of
unused/less recently used memory in an effort to improve performance by
avoiding rereads from persistent media. In the case of zswap, its size is near
0 until memory pressure hits a threshold; a point at which traditional caches
start shrinking. zswap _grows_ under memory pressure while all other caches
shrink. This is why traditional cache sizing policies and techniques don't
work with zswap. In the absence of any precedent policy for this kind of
caching, zswap goes with a simple, but understandable one: user-tunable cap
on the maximum size and shrink through pressure and (soon) age driven writeback.

Seth

>
> It's unfortunate that my proposed topic for LSFMM was pre-empted
> by the zsmalloc vs zbud discussion and zswap vs zcache, because
> I think the real challenge of zswap (or zcache) and the value to
> distros and end users requires us to get this right BEFORE users
> start filing bugs about performance weirdness. After which most
> users and distros will simply default to 0% (i.e. turn zswap off)
> because zswap unpredictably sometimes sucks.
>
> <flame off> sorry...
>
> > Comment 2 I agree is an issue. I already have patches for a "periodic
> > writeback" functionality that starts to shrink the zswap pool via
> > writeback if zswap goes idle for a period of time. This addresses
> > the issue with long-lived, never-accessed pages getting stuck in
> > zswap forever.
>
> Pulling the call out of zswap_frontswap_store() (and ensuring there still
> aren't any new races) would be a good start. But this is just a mechanism;
> you haven't said anything about the policy or how you intend to
> enforce the policy. Which just gets us back to Comment 1...
>
> So Comment 1 and Comment 2 are really the same: How do we appropriately
> manage the number of pages in the system that are used for storing
> compressed pages?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo [at] kvack For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont [at] kvack"> email [at] kvack </a>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 15, 2013, 10:09 AM

Post #11 of 28 (589 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Seth Jennings [mailto:sjenning [at] linux]
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On Tue, May 14, 2013 at 01:18:48PM -0700, Dan Magenheimer wrote:
> > > From: Seth Jennings [mailto:sjenning [at] linux]
> > > Subject: Re: [PATCHv11 3/4] zswap: add to mm/
> > >
> > > <snip>
> > >
> > > > > +/* The maximum percentage of memory that the compressed pool can occupy */
> > > > > +static unsigned int zswap_max_pool_percent = 20;
> > > > > +module_param_named(max_pool_percent,
> > > > > + zswap_max_pool_percent, uint, 0644);
> > >
> > > <snip>
> > >
> > > > This limit, along with the code that enforces it (by calling reclaim
> > > > when the limit is reached), is IMHO questionable. Is there any
> > > > other kernel memory allocation that is constrained by a percentage
> > > > of total memory rather than dynamically according to current
> > > > system conditions? As Mel pointed out (approx.), if this limit
> > > > is reached by a zswap-storm and filled with pages of long-running,
> > > > rarely-used processes, 20% of RAM (by default here) becomes forever
> > > > clogged.
> > >
> > > So there are two comments here 1) dynamic pool limit and 2) writeback
> > > of pages in zswap that won't be faulted in or forced out by pressure.
> > >
> > > Comment 1 feeds from the point of view that compressed pages should just be
> > > another type of memory managed by the core MM. While ideal, very hard to
> > > implement in practice. We are starting to realize that even the policy
> > > governing to active vs inactive list is very hard to get right. Then shrinkers
> > > add more complexity to the policy problem. Throwing another type in the mix
> > > would just that much more complex and hard to get right (assuming there even
> > > _is_ a "right" policy for everyone in such a complex system).
> > >
> > > This max_pool_percent policy is simple, works well, and provides a
> > > deterministic policy that users can understand. Users can be assured that a
> > > dynamic policy heuristic won't go nuts and allow the compressed pool to grow
> > > unbounded or be so aggressively reclaimed that it offers no value.
> >
> > Hi Seth --
> >
> > Hmmm... I'm not sure how to politely say "bullshit". :-)
> >
> > The default 20% was randomly pulled out of the air long ago for zcache
> > experiments. If you can explain why 20% is better than 19% or 21%, or
> > better than 10% or 30% or even 50%, that would be a start. Then please try
> > to explain -- in terms an average sysadmin can understand -- under
> > what circumstances this number should be higher or lower, that would
> > be even better. In fact if you can explain it in even very broadbrush
> > terms like "higher for embedded" and "lower for server" that would be
> > useful. If the top Linux experts in compression can't answer these
> > questions (and the default is a random number, which it is), I don't
> > know how we can expect users to be "assured".
>
> 20% is a default maximum. There really isn't a particular reason for the
> selection other than to supply reasonable default to a tunable. 20% is enough
> to show the benefit while assuring the user zswap won't eat more than that
> amount of memory under any circumstance. The point is to make it a tunable,
> not to launch an incredibly in-depth study on what the default should be.

My point is that a tunable is worthless -- and essentially the same as
a fixed value -- unless you can clearly instruct target users how to
change it to match their needs.

> As guidance on how to tune it, switching to zbud actually made the math simpler
> by bounding the best case to 2 and the expected density to very near 2. I have
> two methods, one based on calculation and another based on experimentation.
>
> Yes, I understand that there are many things to consider, but for the sake of
> those that honestly care about the answer to the question, I'll answer it.
>
> Method 1:
>
> If you have a workload running on a machine with x GB of RAM and an anonymous
> working set of y GB of pages where x < y, a good starting point for
> max_pool_percent is ((y/x)-1)*100.
>
> For example, if you have 10GB of RAM and 12GB anon working set, (12/10-1)*100 =
> 20. During operation there would be 8GB in uncompressed memory, and 4GB worth
> of compressed memory occupying 2GB (i.e. 20%) of RAM. This will reduce swap I/O
> to near zero assuming the pages compress <50% on average.
>
> Bear in mind that this formula provides a lower bound on max_pool_percent if
> you want to avoid swap I/0. Setting max_pool_percent to >20 would produce
> the same situation.

OK, let's try to apply your method. You personally have undoubtedly
compiled the kernel hundreds, maybe thousands of times in the last year.
In the restricted environment where you and I have run benchmarks, this
is a fairly stable and reproducible workload == stable and reproducible
are somewhat rare in the real world.

Can you tell me what the "anon working set" is of compiling the kernel?
Have you, one of the top experts in Linux compression technology, ever
even once changed the max_pool_percent in your benchmark runs even as
an experiment?

This method also makes the assumption that the users that are
going to enable zswap are doing so because their system is currently
swapping its poor brains out (and for some reason can't increase the
RAM in their system). I sure hope that's not the only reason users
will enable it.

> Method 2:
>
> Experimentally, one can just watch swap I/O rates while the workload is running
> and increase max_pool_percent until no (or acceptable level of) swap I/O is
> occurring.
>
> As max_pool_percent increases, however, there is less and less room for
> uncompressed memory, the only kind of memory on which the kernel can actually
> operate. Compression/decompression activity might start dominating over useful
> work. Going over 80 is probably not advised. If swap I/O is still observed
> for high values of max_pool_percent, then the memory load should be reduced,
> memory capacity should be increased, or performance degradation should be accepted.

Method 2 assumes workloads are reproducible/idempotent.

So, I don't think either of these methods are answers to my question,
just handwaving.

> > What you mean is "works well"... on the two benchmarks you've tried it
> > on. You say it's too hard to do dynamically... even though every other
> > significant RAM user in the kernel has to do it dynamically.
> > Workloads are dynamic and heavy users of RAM needs to deal with that.
> > You don't see a limit on the number of anonymous pages in the MM subsystem,
> > and you don't see a limit on the number of inodes in btrfs. Linus
> > would rightfully barf all over those limits and (if he was paying attention
> > to this discussion) he would barf on this limit too.
>
> Putting a user-tunable hard limit on the size of the compressed pool is in _no
> way_ analogous to putting an fixed upper bound on system-wide anonymous memory
> or number of inodes. In fact, they are so dissimilar, I don't know what else to
> say about the attempted comparison.

I'm not sure why we disagree here, but I see them as very similar.

> zswap is not like other caches in the kernel. Most caches make use of
> unused/less recently used memory in an effort to improve performance by
> avoiding rereads from persistent media. In the case of zswap, its size is near
> 0 until memory pressure hits a threshold; a point at which traditional caches
> start shrinking. zswap _grows_ under memory pressure while all other caches
> shrink. This is why traditional cache sizing policies and techniques don't
> work with zswap. In the absence of any precedent policy for this kind of
> caching, zswap goes with a simple, but understandable one: user-tunable cap
> on the maximum size and shrink through pressure and (soon) age driven writeback.

Zswap is not like other caches in the kernel because it is not a cache
at all. It is simply a mechanism whereby the MM subsystem can increase
the total number of anonymous pages stored in RAM by identifying (via
frontswap) and compressing (via LZO) the lowest priority anonymous pages,
then decompressing them when a page fault occurs. The swap subsystem
is just a convenient place to put the hooks because it clearly identifies
the lowest priority anonymous pages -- and also provides a convenient
key (type+offset) to identify the compressed page (which obviously can't
be addressed using normal CPU direct addressing mechanisms)

The precedent policy is the MM subsystem itself, which is responsible
for managing the quantity of pages used for a wide variety of uses and
the priority of these classes against each other. It must do this
dynamically, not just because it must handle different kinds of
workloads, but also because each one of those workloads vary
dramatically across time. Compressed anonymous pages are still
a form of anonymous pages and, to the extent possible, need to be
counted and managed/prioritized as anonymous pages when the
MM subsystem is making policy decisions.

So any artificial limit in the MM subsystem (even as a percentage of
total RAM) is very suspicious and deserves scrutiny. As I said,
your max_pages solution is simple only because you are ignoring the
harder part of the problem and and now also pretending that users/distros
will ever have any clue at all as to how to solve that harder part of
the problem.

Sorry, but I don't think that's appropriate for a patch in the MM subsystem.

Thanks,
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


konrad.wilk at oracle

May 15, 2013, 11:55 AM

Post #12 of 28 (583 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> Sorry, but I don't think that's appropriate for a patch in the MM subsystem.

I am heading to the airport shortly so this email is a bit hastily typed.

Perhaps a compromise can be reached where this code is merged as a driver
not a core mm component. There is a high bar to be in the MM - it has to
work with many many different configurations.

And drivers don't have such a high bar. They just need to work on a specific
issue and that is it. If zswap ended up in say, drivers/mm that would make
it more palpable I think.

Thoughts?
>
> Thanks,
> Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 15, 2013, 12:35 PM

Post #13 of 28 (581 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Konrad Rzeszutek Wilk
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> > Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
>
> I am heading to the airport shortly so this email is a bit hastily typed.
>
> Perhaps a compromise can be reached where this code is merged as a driver
> not a core mm component. There is a high bar to be in the MM - it has to
> work with many many different configurations.
>
> And drivers don't have such a high bar. They just need to work on a specific
> issue and that is it. If zswap ended up in say, drivers/mm that would make
> it more palpable I think.
>
> Thoughts?

Hmmm...

To me, that sounds like a really good compromise. Then anyone
who wants to experiment with compressed swap pages can do so by
enabling the zswap driver. And the harder problem of deeply integrating
compression into the MM subsystem can proceed in parallel
by leveraging and building on the best of zswap and zcache
and zram.

Seth, if you want to re-post zswap as a driver... even a
previous zswap version with zsmalloc and without writeback...
I would be willing to ack it. If I correctly understand
Mel's concerns, I suspect he might feel the same.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sjenning at linux

May 15, 2013, 1:09 PM

Post #14 of 28 (583 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On Wed, May 15, 2013 at 02:55:06PM -0400, Konrad Rzeszutek Wilk wrote:
> > Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
>
> I am heading to the airport shortly so this email is a bit hastily typed.
>
> Perhaps a compromise can be reached where this code is merged as a driver
> not a core mm component. There is a high bar to be in the MM - it has to
> work with many many different configurations.
>
> And drivers don't have such a high bar. They just need to work on a specific
> issue and that is it. If zswap ended up in say, drivers/mm that would make
> it more palpable I think.
>
> Thoughts?

zswap, the writeback code particularly, depends on a number of non-exported
kernel symbols, namely:

swapcache_free
__swap_writepage
__add_to_swap_cache
swapcache_prepare
swapper_spaces

So it can't currently be built as a module and I'm not sure what the MM
folks would think about exporting them and making them part of the KABI.

Seth

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dave at sr71

May 15, 2013, 1:24 PM

Post #15 of 28 (584 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On 05/15/2013 01:09 PM, Seth Jennings wrote:
> On Wed, May 15, 2013 at 02:55:06PM -0400, Konrad Rzeszutek Wilk wrote:
>>> Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
>>
>> Perhaps a compromise can be reached where this code is merged as a driver
>> not a core mm component. There is a high bar to be in the MM - it has to
>> work with many many different configurations.
>>
>> And drivers don't have such a high bar. They just need to work on a specific
>> issue and that is it. If zswap ended up in say, drivers/mm that would make
>> it more palpable I think.

The issue is not whether it is a loadable module or a driver. Nobody
here is stupid enough to say, "hey, now it's a driver/module, all of the
complex VM interactions are finally fixed!"

If folks don't want this in their system, there's a way to turn it off,
today, with the sysfs tunables. We don't need _another_ way to turn it
off at runtime (unloading the module/driver).


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


riel at redhat

May 15, 2013, 1:45 PM

Post #16 of 28 (582 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On 05/15/2013 03:35 PM, Dan Magenheimer wrote:
>> From: Konrad Rzeszutek Wilk
>> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>>
>>> Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
>>
>> I am heading to the airport shortly so this email is a bit hastily typed.
>>
>> Perhaps a compromise can be reached where this code is merged as a driver
>> not a core mm component. There is a high bar to be in the MM - it has to
>> work with many many different configurations.
>>
>> And drivers don't have such a high bar. They just need to work on a specific
>> issue and that is it. If zswap ended up in say, drivers/mm that would make
>> it more palpable I think.
>>
>> Thoughts?
>
> Hmmm...
>
> To me, that sounds like a really good compromise.

Come on, we all know that is nonsense.

Sure, the zswap and zbud code may not be in their final state yet,
but they belong in the mm/ directory, together with the cleancache
code and all the other related bits of code.

Lets put them in their final destination, and hope the code attracts
attention by as many MM developers as can spare the time to help
improve it.

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


konrad.wilk at oracle

May 15, 2013, 1:45 PM

Post #17 of 28 (585 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

Seth Jennings <sjenning [at] linux> wrote:

>On Wed, May 15, 2013 at 02:55:06PM -0400, Konrad Rzeszutek Wilk wrote:
>> > Sorry, but I don't think that's appropriate for a patch in the MM
>subsystem.
>>
>> I am heading to the airport shortly so this email is a bit hastily
>typed.
>>
>> Perhaps a compromise can be reached where this code is merged as a
>driver
>> not a core mm component. There is a high bar to be in the MM - it has
>to
>> work with many many different configurations.
>>
>> And drivers don't have such a high bar. They just need to work on a
>specific
>> issue and that is it. If zswap ended up in say, drivers/mm that would
>make
>> it more palpable I think.
>>
>> Thoughts?
>
>zswap, the writeback code particularly, depends on a number of
>non-exported
>kernel symbols, namely:
>
>swapcache_free
>__swap_writepage
>__add_to_swap_cache
>swapcache_prepare
>swapper_spaces
>
>So it can't currently be built as a module and I'm not sure what the MM
>folks would think about exporting them and making them part of the
>KABI.
>
>Seth

Could those calls go through front swap? Meaning put the code that uses these calls in there?
--
Sent from my Android phone. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 15, 2013, 1:52 PM

Post #18 of 28 (583 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Seth Jennings [mailto:sjenning [at] linux]
> Sent: Wednesday, May 15, 2013 2:10 PM
> To: Konrad Rzeszutek Wilk
> Cc: Dan Magenheimer; Andrew Morton; Greg Kroah-Hartman; Nitin Gupta; Minchan Kim; Robert Jennings;
> Jenifer Hopper; Mel Gorman; Johannes Weiner; Rik van Riel; Larry Woodman; Benjamin Herrenschmidt; Dave
> Hansen; Joe Perches; Joonsoo Kim; Cody P Schafer; Hugh Dickens; Paul Mackerras; linux-mm [at] kvack;
> linux-kernel [at] vger; devel [at] driverdev
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On Wed, May 15, 2013 at 02:55:06PM -0400, Konrad Rzeszutek Wilk wrote:
> > > Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
> >
> > I am heading to the airport shortly so this email is a bit hastily typed.
> >
> > Perhaps a compromise can be reached where this code is merged as a driver
> > not a core mm component. There is a high bar to be in the MM - it has to
> > work with many many different configurations.
> >
> > And drivers don't have such a high bar. They just need to work on a specific
> > issue and that is it. If zswap ended up in say, drivers/mm that would make
> > it more palpable I think.
> >
> > Thoughts?
>
> zswap, the writeback code particularly, depends on a number of non-exported
> kernel symbols, namely:
>
> swapcache_free
> __swap_writepage
> __add_to_swap_cache
> swapcache_prepare
> swapper_spaces
>
> So it can't currently be built as a module and I'm not sure what the MM
> folks would think about exporting them and making them part of the KABI.

It can be built as a module if writeback is disabled (or ifdef'd by
a CONFIG_ZSWAP_WRITEBACK which depends on CONFIG_ZSWAP=y). The
folks at LSFMM who were planning to use zswap will be turning
off writeback anyway so an alternate is to pull writeback out
of zswap completely for now, since you don't really have a good
policy to manage it yet anyway.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 15, 2013, 1:55 PM

Post #19 of 28 (582 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Dave Hansen [mailto:dave [at] sr71]
> Sent: Wednesday, May 15, 2013 2:24 PM
> To: Seth Jennings
> Cc: Konrad Rzeszutek Wilk; Dan Magenheimer; Andrew Morton; Greg Kroah-Hartman; Nitin Gupta; Minchan
> Kim; Robert Jennings; Jenifer Hopper; Mel Gorman; Johannes Weiner; Rik van Riel; Larry Woodman;
> Benjamin Herrenschmidt; Joe Perches; Joonsoo Kim; Cody P Schafer; Hugh Dickens; Paul Mackerras; linux-
> mm [at] kvack; linux-kernel [at] vger; devel [at] driverdev
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On 05/15/2013 01:09 PM, Seth Jennings wrote:
> > On Wed, May 15, 2013 at 02:55:06PM -0400, Konrad Rzeszutek Wilk wrote:
> >>> Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
> >>
> >> Perhaps a compromise can be reached where this code is merged as a driver
> >> not a core mm component. There is a high bar to be in the MM - it has to
> >> work with many many different configurations.
> >>
> >> And drivers don't have such a high bar. They just need to work on a specific
> >> issue and that is it. If zswap ended up in say, drivers/mm that would make
> >> it more palpable I think.
>
> The issue is not whether it is a loadable module or a driver. Nobody
> here is stupid enough to say, "hey, now it's a driver/module, all of the
> complex VM interactions are finally fixed!"
>
> If folks don't want this in their system, there's a way to turn it off,
> today, with the sysfs tunables. We don't need _another_ way to turn it
> off at runtime (unloading the module/driver).

The issue is we KNOW the complex VM interactions are NOT fixed
and there has been very very little breadth testing (i.e.
across a wide range of workloads, and any attempts to show
how much harm can come from enabling it.)

That's (at least borderline) acceptable in a driver that can
be unloaded, but not in MM code IMHO.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 15, 2013, 2:36 PM

Post #20 of 28 (583 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Rik van Riel [mailto:riel [at] redhat]
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On 05/15/2013 03:35 PM, Dan Magenheimer wrote:
> >> From: Konrad Rzeszutek Wilk
> >> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
> >>
> >>> Sorry, but I don't think that's appropriate for a patch in the MM subsystem.
> >>
> >> I am heading to the airport shortly so this email is a bit hastily typed.
> >>
> >> Perhaps a compromise can be reached where this code is merged as a driver
> >> not a core mm component. There is a high bar to be in the MM - it has to
> >> work with many many different configurations.
> >>
> >> And drivers don't have such a high bar. They just need to work on a specific
> >> issue and that is it. If zswap ended up in say, drivers/mm that would make
> >> it more palpable I think.
> >>
> >> Thoughts?
> >
> > Hmmm...
> >
> > To me, that sounds like a really good compromise.
>
> Come on, we all know that is nonsense.
>
> Sure, the zswap and zbud code may not be in their final state yet,
> but they belong in the mm/ directory, together with the cleancache
> code and all the other related bits of code.
>
> Lets put them in their final destination, and hope the code attracts
> attention by as many MM developers as can spare the time to help
> improve it.

Hi Rik --

Seth has been hell-bent on getting SOME code into the kernel
for over a year, since he found out that enabling zcache, a staging
driver, resulted in a tainted kernel. First it was promoting
zcache+zsmalloc out of staging. Then it was zswap+zsmalloc without
writeback, then zswap+zsmalloc with writeback, and now zswap+zbud
with writeback but without a sane policy for writeback. All of
that time, I've been arguing and trying to integrate compression more
deeply and sensibly into MM, rather than just enabling compression as
a toy that happens to speed up a few benchmarks. (This,
in a nutshell, was the feedback I got at LSFMM12 from Andrea and
Mel... and I think also from you.) Seth has resisted every
step of the way, then integrated the functionality in question,
adapted my code (or Nitin's), and called it his own.

If you disagree with any of my arguments earlier in this thread,
please say so. Else, please reinforce that the MM subsystem
needs to dynamically adapt to a broad range of workloads,
which zswap does not (yet) do. Zswap is not simple, it is
simplistic*.

IMHO, it may be OK for a driver to be ham-handed in its memory
use, but that's not OK for something in mm/. So I think merging
zswap as a driver is a perfectly sensible compromise which lets
Seth get his code upstream, allows users (and leading-edge distros)
to experiment with compression, avoids these endless arguments,
and allows those who care to move forward on how to deeply
integrate compression into MM.

Dan

* simplistic, n., The tendency to oversimplify an issue or a problem
by ignoring complexities or complications.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


riel at redhat

May 15, 2013, 3:01 PM

Post #21 of 28 (582 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On 05/15/2013 05:36 PM, Dan Magenheimer wrote:

> If you disagree with any of my arguments earlier in this thread,
> please say so. Else, please reinforce that the MM subsystem
> needs to dynamically adapt to a broad range of workloads,
> which zswap does not (yet) do. Zswap is not simple, it is
> simplistic*.
>
> IMHO, it may be OK for a driver to be ham-handed in its memory
> use, but that's not OK for something in mm/.

It is functionality that a lot of people want.

IMHO it should be where it has most eyes on it, so its
deficiencies can be fixed. At this point all we know is
that zswap is somewhat simplistic, but we have no idea
yet what its failures modes are in practice.

The only way to find out, is to start using it.

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


riel at redhat

May 15, 2013, 3:14 PM

Post #22 of 28 (584 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On 05/14/2013 04:18 PM, Dan Magenheimer wrote:

> It's unfortunate that my proposed topic for LSFMM was pre-empted
> by the zsmalloc vs zbud discussion and zswap vs zcache, because
> I think the real challenge of zswap (or zcache) and the value to
> distros and end users requires us to get this right BEFORE users
> start filing bugs about performance weirdness. After which most
> users and distros will simply default to 0% (i.e. turn zswap off)
> because zswap unpredictably sometimes sucks.

I'm not sure we can get it right before people actually start
using it for real world setups, instead of just running benchmarks
on it.

The sooner we get the code out there, where users can play with
it (even if it is disabled by default and needs a sysfs or
sysctl config option to enable it), the sooner we will know how
well it works, and what needs to be changed.

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


dan.magenheimer at oracle

May 16, 2013, 9:45 AM

Post #23 of 28 (564 views)
Permalink
RE: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

> From: Rik van Riel [mailto:riel [at] redhat]
> Sent: Wednesday, May 15, 2013 4:15 PM
> To: Dan Magenheimer
> Cc: Seth Jennings; Andrew Morton; Greg Kroah-Hartman; Nitin Gupta; Minchan Kim; Konrad Wilk; Robert
> Jennings; Jenifer Hopper; Mel Gorman; Johannes Weiner; Larry Woodman; Benjamin Herrenschmidt; Dave
> Hansen; Joe Perches; Joonsoo Kim; Cody P Schafer; Hugh Dickens; Paul Mackerras; linux-mm [at] kvack;
> linux-kernel [at] vger; devel [at] driverdev
> Subject: Re: [PATCHv11 3/4] zswap: add to mm/
>
> On 05/14/2013 04:18 PM, Dan Magenheimer wrote:
>
> > It's unfortunate that my proposed topic for LSFMM was pre-empted
> > by the zsmalloc vs zbud discussion and zswap vs zcache, because
> > I think the real challenge of zswap (or zcache) and the value to
> > distros and end users requires us to get this right BEFORE users
> > start filing bugs about performance weirdness. After which most
> > users and distros will simply default to 0% (i.e. turn zswap off)
> > because zswap unpredictably sometimes sucks.
>
> I'm not sure we can get it right before people actually start
> using it for real world setups, instead of just running benchmarks
> on it.
>
> The sooner we get the code out there, where users can play with
> it (even if it is disabled by default and needs a sysfs or
> sysctl config option to enable it), the sooner we will know how
> well it works, and what needs to be changed.

/me sets stage of first Star Wars (1977)

/me envisions self as Obi-Wan Kenobi, old and tired of fighting,
in lightsaber battle with protege Darth Vader / Anakin Skywalker

/me sadly turns off lightsaber, holds useless handle at waist,
takes a deep breath, and promptly gets sliced into oblivion.

Time for A New Hope(tm).

(/me cc's Jon Corbet for a longshot last chance of making LWN's
Kernel Development Quotes of the Week.)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


riel at redhat

May 16, 2013, 10:06 AM

Post #24 of 28 (565 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On 05/16/2013 12:45 PM, Dan Magenheimer wrote:
>> From: Rik van Riel [mailto:riel [at] redhat]

>> I'm not sure we can get it right before people actually start
>> using it for real world setups, instead of just running benchmarks
>> on it.
>>
>> The sooner we get the code out there, where users can play with
>> it (even if it is disabled by default and needs a sysfs or
>> sysctl config option to enable it), the sooner we will know how
>> well it works, and what needs to be changed.
>
> /me sets stage of first Star Wars (1977)
>
> /me envisions self as Obi-Wan Kenobi, old and tired of fighting,
> in lightsaber battle with protege Darth Vader / Anakin Skywalker
>
> /me sadly turns off lightsaber, holds useless handle at waist,
> takes a deep breath, and promptly gets sliced into oblivion.
>
> Time for A New Hope(tm).

I would have hoped that in the battle against memory
shortages, we would all be on the same side.

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


riel at redhat

May 16, 2013, 10:16 AM

Post #25 of 28 (565 views)
Permalink
Re: [PATCHv11 3/4] zswap: add to mm/ [In reply to]

On 05/13/2013 08:40 AM, Seth Jennings wrote:
> zswap is a thin compression backend for frontswap. It receives pages from
> frontswap and attempts to store them in a compressed memory pool, resulting in
> an effective partial memory reclaim and dramatically reduced swap device I/O.
>
> Additionally, in most cases, pages can be retrieved from this compressed store
> much more quickly than reading from tradition swap devices resulting in faster
> performance for many workloads.
>
> It also has support for evicting swap pages that are currently compressed in
> zswap to the swap device on an LRU(ish) basis. This functionality is very
> important and make zswap a true cache in that, once the cache is full or can't
> grow due to memory pressure, the oldest pages can be moved out of zswap to the
> swap device so newer pages can be compressed and stored in zswap.
>
> This patch adds the zswap driver to mm/
>
> Signed-off-by: Seth Jennings <sjenning [at] linux>

Acked-by: Rik van Riel <riel [at] redhat>

As an aside, I agree with Dan that CONFIG_EXPERIMENTAL might be
appropriate here, or at least some kind of warning in the documentation
stating that this is fairly new code that still needs to be shaken
out in the real world.

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

First page Previous page 1 2 Next page Last page  View All Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.