On 01/07/2013 03:24 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. Additional, 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. This patch adds the zswap driver to mm/ Signed-off-by: Seth Jennings <sjenning@xxxxxxxxxxxxxxxxxx>
I like the approach of flushing pages into actual disk based swap when compressed swap is full. I would like it if that was advertised more prominently in the changelog :) The code looks mostly good, complaints are at the nitpick level. One worry is that the pool can grow to whatever maximum was decided, and there is no way to shrink it when memory is required for something else. Would it be an idea to add a shrinker for the zcache pool, that can also shrink the zcache pool when required? Of course, that does lead to the question of how to balance the pressure from that shrinker, with the new memory entering zcache from the swap side. I have no clear answers here, just something to think about...
+static void zswap_flush_entries(unsigned type, int nr) +{ + struct zswap_tree *tree = zswap_trees[type]; + struct zswap_entry *entry; + int i, ret; + +/* + * This limits is arbitrary for now until a better + * policy can be implemented. This is so we don't + * eat all of RAM decompressing pages for writeback. + */ +#define ZSWAP_MAX_OUTSTANDING_FLUSHES 64 + if (atomic_read(&zswap_outstanding_flushes) > + ZSWAP_MAX_OUTSTANDING_FLUSHES) + return;
Having this #define right in the middle of the function is rather ugly. Might be worth moving it to the top.
+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("saved_by_flush", S_IRUGO, + zswap_debugfs_root, &zswap_saved_by_flush); + debugfs_create_u64("pool_limit_hit", S_IRUGO, + zswap_debugfs_root, &zswap_pool_limit_hit); + debugfs_create_u64("reject_flush_attempted", S_IRUGO, + zswap_debugfs_root, &zswap_flush_attempted); + debugfs_create_u64("reject_tmppage_fail", S_IRUGO, + zswap_debugfs_root, &zswap_reject_tmppage_fail); + debugfs_create_u64("reject_flush_fail", S_IRUGO, + zswap_debugfs_root, &zswap_reject_flush_fail); + debugfs_create_u64("reject_zsmalloc_fail", S_IRUGO, + zswap_debugfs_root, &zswap_reject_zsmalloc_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("flushed_pages", S_IRUGO, + zswap_debugfs_root, &zswap_flushed_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); + debugfs_create_atomic_t("outstanding_flushes", S_IRUGO, + zswap_debugfs_root, &zswap_outstanding_flushes); +
Some of these statistics would be very useful to system administrators, who will not be mounting debugfs on production systems. Would it make sense to export some of these statistics through sysfs? -- All rights reversed -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>