* [PATCH] zswap: Same-filled pages handling [not found] <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p1> @ 2017-10-18 10:48 ` Srividya Desireddy 2017-10-18 12:34 ` Matthew Wilcox ` (5 more replies) 2017-10-18 14:43 ` [PATCH] " Srividya Desireddy 1 sibling, 6 replies; 22+ messages in thread From: Srividya Desireddy @ 2017-10-18 10:48 UTC (permalink / raw) To: sjenning, ddstreet, linux-mm, linux-kernel, penberg Cc: Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy From: Srividya Desireddy <srividya.dr@samsung.com> Date: Wed, 18 Oct 2017 15:39:02 +0530 Subject: [PATCH] zswap: Same-filled pages handling Zswap is a cache which compresses the pages that are being swapped out and stores them into a dynamically allocated RAM-based memory pool. Experiments have shown that around 10-20% of pages stored in zswap are same-filled pages (i.e. contents of the page are all same), but these pages are handled as normal pages by compressing and allocating memory in the pool. This patch adds a check in zswap_frontswap_store() to identify same-filled page before compression of the page. If the page is a same-filled page, set zswap_entry.length to zero, save the same-filled value and skip the compression of the page and alloction of memory in zpool. In zswap_frontswap_load(), check if value of zswap_entry.length is zero corresponding to the page to be loaded. If zswap_entry.length is zero, fill the page with same-filled value. This saves the decompression time during load. On a ARM Quad Core 32-bit device with 1.5GB RAM by launching and relaunching different applications, out of ~64000 pages stored in zswap, ~11000 pages were same-value filled pages (including zero-filled pages) and ~9000 pages were zero-filled pages. An average of 17% of pages(including zero-filled pages) in zswap are same-value filled pages and 14% pages are zero-filled pages. An average of 3% of pages are same-filled non-zero pages. The below table shows the execution time profiling with the patch. Baseline With patch % Improvement ----------------------------------------------------------------- *Zswap Store Time 26.5ms 18ms 32% (of same value pages) *Zswap Load Time (of same value pages) 25.5ms 13ms 49% ----------------------------------------------------------------- On Ubuntu PC with 2GB RAM, while executing kernel build and other test scripts and running multimedia applications, out of 360000 pages stored in zswap 78000(~22%) of pages were found to be same-value filled pages (including zero-filled pages) and 64000(~17%) are zero-filled pages. So an average of %5 of pages are same-filled non-zero pages. The below table shows the execution time profiling with the patch. Baseline With patch % Improvement ----------------------------------------------------------------- *Zswap Store Time 91ms 74ms 19% (of same value pages) *Zswap Load Time 50ms 7.5ms 85% (of same value pages) ----------------------------------------------------------------- *The execution times may vary with test device used. Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> --- mm/zswap.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 5 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index d39581a..4dd8b89 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -49,6 +49,8 @@ static u64 zswap_pool_total_size; /* The number of compressed pages currently stored in zswap */ static atomic_t zswap_stored_pages = ATOMIC_INIT(0); +/* The number of same-value filled pages currently stored in zswap */ +static atomic_t zswap_same_filled_pages = ATOMIC_INIT(0); /* * The statistics below are not protected from concurrent access for @@ -116,6 +118,11 @@ static int zswap_compressor_param_set(const char *, static unsigned int zswap_max_pool_percent = 20; module_param_named(max_pool_percent, zswap_max_pool_percent, uint, 0644); +/* Enable/disable handling same-value filled pages (enabled by default) */ +static bool zswap_same_filled_pages_enabled = true; +module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled, + bool, 0644); + /********************************* * data structures **********************************/ @@ -145,9 +152,10 @@ struct zswap_pool { * be held while changing the refcount. Since the lock must * be held, there is no reason to also make refcount atomic. * length - the length in bytes of the compressed page data. Needed during - * decompression + * decompression. For a same value filled page length is 0. * pool - the zswap_pool the entry's data is in * handle - zpool allocation handle that stores the compressed page data + * value - value of the same-value filled pages which have same content */ struct zswap_entry { struct rb_node rbnode; @@ -155,7 +163,10 @@ struct zswap_entry { int refcount; unsigned int length; struct zswap_pool *pool; - unsigned long handle; + union { + unsigned long handle; + unsigned long value; + }; }; struct zswap_header { @@ -320,8 +331,12 @@ static void zswap_rb_erase(struct rb_root *root, struct zswap_entry *entry) */ static void zswap_free_entry(struct zswap_entry *entry) { - zpool_free(entry->pool->zpool, entry->handle); - zswap_pool_put(entry->pool); + if (!entry->length) + atomic_dec(&zswap_same_filled_pages); + else { + zpool_free(entry->pool->zpool, entry->handle); + zswap_pool_put(entry->pool); + } zswap_entry_cache_free(entry); atomic_dec(&zswap_stored_pages); zswap_update_total_size(); @@ -953,6 +968,34 @@ static int zswap_shrink(void) return ret; } +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) +{ + unsigned int pos; + unsigned long *page; + + page = (unsigned long *)ptr; + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { + if (page[pos] != page[0]) + return 0; + } + *value = page[0]; + return 1; +} + +static void zswap_fill_page(void *ptr, unsigned long value) +{ + unsigned int pos; + unsigned long *page; + + page = (unsigned long *)ptr; + if (value == 0) + memset(page, 0, PAGE_SIZE); + else { + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) + page[pos] = value; + } +} + /********************************* * frontswap hooks **********************************/ @@ -965,7 +1008,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, struct crypto_comp *tfm; int ret; unsigned int dlen = PAGE_SIZE, len; - unsigned long handle; + unsigned long handle, value; char *buf; u8 *src, *dst; struct zswap_header *zhdr; @@ -993,6 +1036,19 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, goto reject; } + if (zswap_same_filled_pages_enabled) { + src = kmap_atomic(page); + if (zswap_is_page_same_filled(src, &value)) { + kunmap_atomic(src); + entry->offset = offset; + entry->length = 0; + entry->value = value; + atomic_inc(&zswap_same_filled_pages); + goto insert_entry; + } + kunmap_atomic(src); + } + /* if entry is successfully added, it keeps the reference */ entry->pool = zswap_pool_current_get(); if (!entry->pool) { @@ -1037,6 +1093,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, entry->handle = handle; entry->length = dlen; +insert_entry: /* map */ spin_lock(&tree->lock); do { @@ -1089,6 +1146,13 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, } spin_unlock(&tree->lock); + if (!entry->length) { + dst = kmap_atomic(page); + zswap_fill_page(dst, entry->value); + kunmap_atomic(dst); + goto freeentry; + } + /* decompress */ dlen = PAGE_SIZE; src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle, @@ -1101,6 +1165,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, zpool_unmap_handle(entry->pool->zpool, entry->handle); BUG_ON(ret); +freeentry: spin_lock(&tree->lock); zswap_entry_put(tree, entry); spin_unlock(&tree->lock); @@ -1209,6 +1274,8 @@ static int __init zswap_debugfs_init(void) zswap_debugfs_root, &zswap_pool_total_size); debugfs_create_atomic_t("stored_pages", S_IRUGO, zswap_debugfs_root, &zswap_stored_pages); + debugfs_create_atomic_t("same_filled_pages", 0444, + zswap_debugfs_root, &zswap_same_filled_pages); return 0; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy @ 2017-10-18 12:34 ` Matthew Wilcox 2017-10-18 13:33 ` Timofey Titovets 2017-10-18 20:43 ` Andi Kleen ` (4 subsequent siblings) 5 siblings, 1 reply; 22+ messages in thread From: Matthew Wilcox @ 2017-10-18 12:34 UTC (permalink / raw) To: Srividya Desireddy Cc: sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 10:48:32AM +0000, Srividya Desireddy wrote: > +static void zswap_fill_page(void *ptr, unsigned long value) > +{ > + unsigned int pos; > + unsigned long *page; > + > + page = (unsigned long *)ptr; > + if (value == 0) > + memset(page, 0, PAGE_SIZE); > + else { > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) > + page[pos] = value; > + } > +} I think you meant: static void zswap_fill_page(void *ptr, unsigned long value) { memset_l(ptr, value, PAGE_SIZE / sizeof(unsigned long)); } (and you should see significantly better numbers at least on x86; I don't know if anyone's done an arm64 version of memset_l yet). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 12:34 ` Matthew Wilcox @ 2017-10-18 13:33 ` Timofey Titovets 2017-10-18 14:11 ` Matthew Wilcox 0 siblings, 1 reply; 22+ messages in thread From: Timofey Titovets @ 2017-10-18 13:33 UTC (permalink / raw) To: Matthew Wilcox Cc: Srividya Desireddy, sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy 2017-10-18 15:34 GMT+03:00 Matthew Wilcox <willy@infradead.org>: > On Wed, Oct 18, 2017 at 10:48:32AM +0000, Srividya Desireddy wrote: >> +static void zswap_fill_page(void *ptr, unsigned long value) >> +{ >> + unsigned int pos; >> + unsigned long *page; >> + >> + page = (unsigned long *)ptr; >> + if (value == 0) >> + memset(page, 0, PAGE_SIZE); >> + else { >> + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) >> + page[pos] = value; >> + } >> +} > > I think you meant: > > static void zswap_fill_page(void *ptr, unsigned long value) > { > memset_l(ptr, value, PAGE_SIZE / sizeof(unsigned long)); > } > > (and you should see significantly better numbers at least on x86; > I don't know if anyone's done an arm64 version of memset_l yet). > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> IIRC kernel have special zero page, and if i understand correctly. You can map all zero pages to that zero page and not touch zswap completely. (Your situation look like some KSM case (i.e. KSM can handle pages with same content), but i'm not sure if that applicable there) Thanks. -- Have a nice day, Timofey. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 13:33 ` Timofey Titovets @ 2017-10-18 14:11 ` Matthew Wilcox 0 siblings, 0 replies; 22+ messages in thread From: Matthew Wilcox @ 2017-10-18 14:11 UTC (permalink / raw) To: Timofey Titovets Cc: Srividya Desireddy, sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 04:33:43PM +0300, Timofey Titovets wrote: > 2017-10-18 15:34 GMT+03:00 Matthew Wilcox <willy@infradead.org>: > > On Wed, Oct 18, 2017 at 10:48:32AM +0000, Srividya Desireddy wrote: > >> +static void zswap_fill_page(void *ptr, unsigned long value) > >> +{ > >> + unsigned int pos; > >> + unsigned long *page; > >> + > >> + page = (unsigned long *)ptr; > >> + if (value == 0) > >> + memset(page, 0, PAGE_SIZE); > >> + else { > >> + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) > >> + page[pos] = value; > >> + } > >> +} > > > > I think you meant: > > > > static void zswap_fill_page(void *ptr, unsigned long value) > > { > > memset_l(ptr, value, PAGE_SIZE / sizeof(unsigned long)); > > } > > IIRC kernel have special zero page, and if i understand correctly. > You can map all zero pages to that zero page and not touch zswap completely. > (Your situation look like some KSM case (i.e. KSM can handle pages > with same content), but i'm not sure if that applicable there) You're confused by the word "same". What Srividya meant was that the page is filled with a pattern, eg 0xfffefffefffefffe..., not that it is the same as any other page. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy 2017-10-18 12:34 ` Matthew Wilcox @ 2017-10-18 20:43 ` Andi Kleen 2017-10-19 1:10 ` Matthew Wilcox 2017-10-18 21:31 ` Timofey Titovets ` (3 subsequent siblings) 5 siblings, 1 reply; 22+ messages in thread From: Andi Kleen @ 2017-10-18 20:43 UTC (permalink / raw) To: Srividya Desireddy Cc: sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy Srividya Desireddy <srividya.dr@samsung.com> writes: > > On a ARM Quad Core 32-bit device with 1.5GB RAM by launching and > relaunching different applications, out of ~64000 pages stored in > zswap, ~11000 pages were same-value filled pages (including zero-filled > pages) and ~9000 pages were zero-filled pages. What are the values for the non zero cases? > +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) > +{ > + unsigned int pos; > + unsigned long *page; > + > + page = (unsigned long *)ptr; > + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { > + if (page[pos] != page[0]) > + return 0; > + } So on 32bit it checks for 32bit repeating values and on 64bit for 64bit repeating values. Does that make sense? Did you test the patch on a 64bit system? Overall I would expect this extra pass to be fairly expensive. It may be better to add some special check to the compressor, and let it abort if it sees a string of same values, and only do the check then. -Andi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 20:43 ` Andi Kleen @ 2017-10-19 1:10 ` Matthew Wilcox 2017-10-19 4:30 ` Andi Kleen 0 siblings, 1 reply; 22+ messages in thread From: Matthew Wilcox @ 2017-10-19 1:10 UTC (permalink / raw) To: Andi Kleen Cc: Srividya Desireddy, sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 01:43:10PM -0700, Andi Kleen wrote: > > +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) > > +{ > > + unsigned int pos; > > + unsigned long *page; > > + > > + page = (unsigned long *)ptr; > > + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { > > + if (page[pos] != page[0]) > > + return 0; > > + } > > So on 32bit it checks for 32bit repeating values and on 64bit > for 64bit repeating values. Does that make sense? Yes. Every 64-bit repeating pattern is also a 32-bit repeating pattern. Supporting a 64-bit pattern on a 32-bit kernel is painful, but it makes no sense to *not* support a 64-bit pattern on a 64-bit kernel. This is the same approach used in zram, fwiw. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-19 1:10 ` Matthew Wilcox @ 2017-10-19 4:30 ` Andi Kleen 2017-10-19 13:24 ` Matthew Wilcox 0 siblings, 1 reply; 22+ messages in thread From: Andi Kleen @ 2017-10-19 4:30 UTC (permalink / raw) To: Matthew Wilcox Cc: Srividya Desireddy, sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy > Yes. Every 64-bit repeating pattern is also a 32-bit repeating pattern. > Supporting a 64-bit pattern on a 32-bit kernel is painful, but it makes > no sense to *not* support a 64-bit pattern on a 64-bit kernel. But a 32bit repeating pattern is not necessarily a 64bit pattern. >This is the same approach used in zram, fwiw. Sounds bogus. -Andi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-19 4:30 ` Andi Kleen @ 2017-10-19 13:24 ` Matthew Wilcox 0 siblings, 0 replies; 22+ messages in thread From: Matthew Wilcox @ 2017-10-19 13:24 UTC (permalink / raw) To: Andi Kleen Cc: Srividya Desireddy, sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 09:30:32PM -0700, Andi Kleen wrote: > > Yes. Every 64-bit repeating pattern is also a 32-bit repeating pattern. > > Supporting a 64-bit pattern on a 32-bit kernel is painful, but it makes > > no sense to *not* support a 64-bit pattern on a 64-bit kernel. > > But a 32bit repeating pattern is not necessarily a 64bit pattern. Oops, I said it backwards. What I mean is that if you have the repeating pattern: 0x12345678 12345678 12345678 12345678 12345678 12345678 that's the same as the repeating pattern: 0x1234567812345678 1234567812345678 1234567812345678 so the 64-bit kernel is able to find all patterns that the 32-bit kernel is, and more. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy 2017-10-18 12:34 ` Matthew Wilcox 2017-10-18 20:43 ` Andi Kleen @ 2017-10-18 21:31 ` Timofey Titovets 2017-10-19 1:08 ` Matthew Wilcox ` (2 more replies) 2017-11-17 21:27 ` Dan Streetman ` (2 subsequent siblings) 5 siblings, 3 replies; 22+ messages in thread From: Timofey Titovets @ 2017-10-18 21:31 UTC (permalink / raw) To: Srividya Desireddy Cc: sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy > +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) > +{ > + unsigned int pos; > + unsigned long *page; > + > + page = (unsigned long *)ptr; > + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { > + if (page[pos] != page[0]) > + return 0; > + } > + *value = page[0]; > + return 1; > +} > + In theory you can speedup that check by memcmp(), And do something like first: memcmp(ptr, ptr + PAGE_SIZE/sizeof(*page)/2, PAGE_SIZE/2); After compare 1/4 with 2/4 Then 1/8 with 2/8. And after do you check with pattern, only on first 512 bytes. Just because memcmp() on fresh CPU are crazy fast. That can easy make you check less expensive. > +static void zswap_fill_page(void *ptr, unsigned long value) > +{ > + unsigned int pos; > + unsigned long *page; > + > + page = (unsigned long *)ptr; > + if (value == 0) > + memset(page, 0, PAGE_SIZE); > + else { > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) > + page[pos] = value; > + } > +} Same here, but with memcpy(). P.S. I'm just too busy to make fast performance test in user space, but my recent experience with that CPU commands, show what that make a sense: KSM patch: https://patchwork.kernel.org/patch/9980803/ User space tests: https://github.com/Nefelim4ag/memcmpe PAGE_SIZE: 65536, loop count: 1966080 memcmp: -28 time: 3216 ms, th: 40064.644611 MiB/s memcmpe: -28, offset: 62232 time: 3588 ms, th: 35902.462390 MiB/s memcmpe: -28, offset: 62232 time: 71 ms, th: 1792233.164286 MiB/s IIRC, with code like our, you must see ~2.5GiB/s Thanks. -- Have a nice day, Timofey. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 21:31 ` Timofey Titovets @ 2017-10-19 1:08 ` Matthew Wilcox [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p3> 2017-11-17 22:07 ` Dan Streetman 2 siblings, 0 replies; 22+ messages in thread From: Matthew Wilcox @ 2017-10-19 1:08 UTC (permalink / raw) To: Timofey Titovets Cc: Srividya Desireddy, sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Thu, Oct 19, 2017 at 12:31:18AM +0300, Timofey Titovets wrote: > > +static void zswap_fill_page(void *ptr, unsigned long value) > > +{ > > + unsigned int pos; > > + unsigned long *page; > > + > > + page = (unsigned long *)ptr; > > + if (value == 0) > > + memset(page, 0, PAGE_SIZE); > > + else { > > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) > > + page[pos] = value; > > + } > > +} > > Same here, but with memcpy(). No. Use memset_l which is optimised for this specific job. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p3>]
* Re: [PATCH] zswap: Same-filled pages handling [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p3> @ 2017-11-02 15:08 ` Srividya Desireddy 2017-11-17 22:10 ` Dan Streetman 0 siblings, 1 reply; 22+ messages in thread From: Srividya Desireddy @ 2017-11-02 15:08 UTC (permalink / raw) To: sjenning, Matthew Wilcox, Timofey Titovets Cc: linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 19, 2017 at 6:38 AM, Matthew Wilcox wrote: > On Thu, Oct 19, 2017 at 12:31:18AM +0300, Timofey Titovets wrote: >> > +static void zswap_fill_page(void *ptr, unsigned long value) >> > +{ >> > + unsigned int pos; >> > + unsigned long *page; >> > + >> > + page = (unsigned long *)ptr; >> > + if (value == 0) >> > + memset(page, 0, PAGE_SIZE); >> > + else { >> > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) >> > + page[pos] = value; >> > + } >> > +} >> >> Same here, but with memcpy(). > >No. Use memset_l which is optimised for this specific job. I have tested this patch using memset_l() function in zswap_fill_page() on x86 64-bit system with 2GB RAM. The performance remains same. But, memset_l() funcion might be optimised in future. @Seth Jennings/Dan Streetman: Should I use memset_l() function in this patch. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-11-02 15:08 ` Srividya Desireddy @ 2017-11-17 22:10 ` Dan Streetman 0 siblings, 0 replies; 22+ messages in thread From: Dan Streetman @ 2017-11-17 22:10 UTC (permalink / raw) To: srividya.dr Cc: sjenning, Matthew Wilcox, Timofey Titovets, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Thu, Nov 2, 2017 at 11:08 AM, Srividya Desireddy <srividya.dr@samsung.com> wrote: > > On Wed, Oct 19, 2017 at 6:38 AM, Matthew Wilcox wrote: >> On Thu, Oct 19, 2017 at 12:31:18AM +0300, Timofey Titovets wrote: >>> > +static void zswap_fill_page(void *ptr, unsigned long value) >>> > +{ >>> > + unsigned int pos; >>> > + unsigned long *page; >>> > + >>> > + page = (unsigned long *)ptr; >>> > + if (value == 0) >>> > + memset(page, 0, PAGE_SIZE); >>> > + else { >>> > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) >>> > + page[pos] = value; >>> > + } >>> > +} >>> >>> Same here, but with memcpy(). >> >>No. Use memset_l which is optimised for this specific job. > > I have tested this patch using memset_l() function in zswap_fill_page() on > x86 64-bit system with 2GB RAM. The performance remains same. > But, memset_l() funcion might be optimised in future. > @Seth Jennings/Dan Streetman: Should I use memset_l() function in this patch. my testing showed also showed minimal if any difference when using memset_l(), but it's simpler code and should never be slower than looping. I'll ack it if you want to send an additional patch making this change (on top of the one I already acked). > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 21:31 ` Timofey Titovets 2017-10-19 1:08 ` Matthew Wilcox [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p3> @ 2017-11-17 22:07 ` Dan Streetman 2 siblings, 0 replies; 22+ messages in thread From: Dan Streetman @ 2017-11-17 22:07 UTC (permalink / raw) To: Timofey Titovets Cc: Srividya Desireddy, sjenning, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 5:31 PM, Timofey Titovets <nefelim4ag@gmail.com> wrote: >> +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) >> +{ >> + unsigned int pos; >> + unsigned long *page; >> + >> + page = (unsigned long *)ptr; >> + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { >> + if (page[pos] != page[0]) >> + return 0; >> + } >> + *value = page[0]; >> + return 1; >> +} >> + > > In theory you can speedup that check by memcmp(), > And do something like first: > memcmp(ptr, ptr + PAGE_SIZE/sizeof(*page)/2, PAGE_SIZE/2); > After compare 1/4 with 2/4 > Then 1/8 with 2/8. > And after do you check with pattern, only on first 512 bytes. > > Just because memcmp() on fresh CPU are crazy fast. > That can easy make you check less expensive. I did check this, and it is actually significantly worse; keep in mind that doing it ^ way may is a smaller loop, but is actually doing more memory comparisons. > >> +static void zswap_fill_page(void *ptr, unsigned long value) >> +{ >> + unsigned int pos; >> + unsigned long *page; >> + >> + page = (unsigned long *)ptr; >> + if (value == 0) >> + memset(page, 0, PAGE_SIZE); >> + else { >> + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) >> + page[pos] = value; >> + } >> +} > > Same here, but with memcpy(). > > P.S. > I'm just too busy to make fast performance test in user space, > but my recent experience with that CPU commands, show what that make a sense: > KSM patch: https://patchwork.kernel.org/patch/9980803/ > User space tests: https://github.com/Nefelim4ag/memcmpe > PAGE_SIZE: 65536, loop count: 1966080 > memcmp: -28 time: 3216 ms, th: 40064.644611 MiB/s > memcmpe: -28, offset: 62232 time: 3588 ms, th: 35902.462390 MiB/s > memcmpe: -28, offset: 62232 time: 71 ms, th: 1792233.164286 MiB/s > > IIRC, with code like our, you must see ~2.5GiB/s > > Thanks. > -- > Have a nice day, > Timofey. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy ` (2 preceding siblings ...) 2017-10-18 21:31 ` Timofey Titovets @ 2017-11-17 21:27 ` Dan Streetman 2017-11-20 23:46 ` Andrew Morton [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p4> 5 siblings, 0 replies; 22+ messages in thread From: Dan Streetman @ 2017-11-17 21:27 UTC (permalink / raw) To: srividya.dr, Andrew Morton Cc: sjenning, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 6:48 AM, Srividya Desireddy <srividya.dr@samsung.com> wrote: > > From: Srividya Desireddy <srividya.dr@samsung.com> > Date: Wed, 18 Oct 2017 15:39:02 +0530 > Subject: [PATCH] zswap: Same-filled pages handling > > Zswap is a cache which compresses the pages that are being swapped out > and stores them into a dynamically allocated RAM-based memory pool. > Experiments have shown that around 10-20% of pages stored in zswap > are same-filled pages (i.e. contents of the page are all same), but > these pages are handled as normal pages by compressing and allocating > memory in the pool. > > This patch adds a check in zswap_frontswap_store() to identify same-filled > page before compression of the page. If the page is a same-filled page, set > zswap_entry.length to zero, save the same-filled value and skip the > compression of the page and alloction of memory in zpool. > In zswap_frontswap_load(), check if value of zswap_entry.length is zero > corresponding to the page to be loaded. If zswap_entry.length is zero, > fill the page with same-filled value. This saves the decompression time > during load. > > On a ARM Quad Core 32-bit device with 1.5GB RAM by launching and > relaunching different applications, out of ~64000 pages stored in > zswap, ~11000 pages were same-value filled pages (including zero-filled > pages) and ~9000 pages were zero-filled pages. > > An average of 17% of pages(including zero-filled pages) in zswap are > same-value filled pages and 14% pages are zero-filled pages. > An average of 3% of pages are same-filled non-zero pages. > > The below table shows the execution time profiling with the patch. > > Baseline With patch % Improvement > ----------------------------------------------------------------- > *Zswap Store Time 26.5ms 18ms 32% > (of same value pages) > *Zswap Load Time > (of same value pages) 25.5ms 13ms 49% > ----------------------------------------------------------------- > > On Ubuntu PC with 2GB RAM, while executing kernel build and other test > scripts and running multimedia applications, out of 360000 pages > stored in zswap 78000(~22%) of pages were found to be same-value filled > pages (including zero-filled pages) and 64000(~17%) are zero-filled > pages. So an average of %5 of pages are same-filled non-zero pages. > > The below table shows the execution time profiling with the patch. > > Baseline With patch % Improvement > ----------------------------------------------------------------- > *Zswap Store Time 91ms 74ms 19% > (of same value pages) > *Zswap Load Time 50ms 7.5ms 85% > (of same value pages) > ----------------------------------------------------------------- > > *The execution times may vary with test device used. First, I'm really sorry for such a long delay in looking at this. I did test this patch out this week, and I added some instrumentation to check the performance impact, and tested with a small program to try to check the best and worst cases. When doing a lot of swap where all (or almost all) pages are same-value, I found this patch does save both time and space, significantly. The exact improvement in time and space depends on which compressor is being used, but roughly agrees with the numbers you listed. In the worst case situation, where all (or almost all) pages have the same-value *except* the final long (meaning, zswap will check each long on the entire page but then still have to pass the page to the compressor), the same-value check is around 10-15% of the total time spent in zswap_frontswap_store(). That's a not-insignificant amount of time, but it's not huge. Considering that most systems will probably be swapping pages that aren't similar to the worst case (although I don't have any data to know that), I'd say the improvement is worth the possible worst-case performance impact. > > Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> Acked-by: Dan Streetman <ddstreet@ieee.org> > --- > mm/zswap.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 72 insertions(+), 5 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index d39581a..4dd8b89 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -49,6 +49,8 @@ > static u64 zswap_pool_total_size; > /* The number of compressed pages currently stored in zswap */ > static atomic_t zswap_stored_pages = ATOMIC_INIT(0); > +/* The number of same-value filled pages currently stored in zswap */ > +static atomic_t zswap_same_filled_pages = ATOMIC_INIT(0); > > /* > * The statistics below are not protected from concurrent access for > @@ -116,6 +118,11 @@ static int zswap_compressor_param_set(const char *, > static unsigned int zswap_max_pool_percent = 20; > module_param_named(max_pool_percent, zswap_max_pool_percent, uint, 0644); > > +/* Enable/disable handling same-value filled pages (enabled by default) */ > +static bool zswap_same_filled_pages_enabled = true; > +module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled, > + bool, 0644); > + > /********************************* > * data structures > **********************************/ > @@ -145,9 +152,10 @@ struct zswap_pool { > * be held while changing the refcount. Since the lock must > * be held, there is no reason to also make refcount atomic. > * length - the length in bytes of the compressed page data. Needed during > - * decompression > + * decompression. For a same value filled page length is 0. > * pool - the zswap_pool the entry's data is in > * handle - zpool allocation handle that stores the compressed page data > + * value - value of the same-value filled pages which have same content > */ > struct zswap_entry { > struct rb_node rbnode; > @@ -155,7 +163,10 @@ struct zswap_entry { > int refcount; > unsigned int length; > struct zswap_pool *pool; > - unsigned long handle; > + union { > + unsigned long handle; > + unsigned long value; > + }; > }; > > struct zswap_header { > @@ -320,8 +331,12 @@ static void zswap_rb_erase(struct rb_root *root, struct zswap_entry *entry) > */ > static void zswap_free_entry(struct zswap_entry *entry) > { > - zpool_free(entry->pool->zpool, entry->handle); > - zswap_pool_put(entry->pool); > + if (!entry->length) > + atomic_dec(&zswap_same_filled_pages); > + else { > + zpool_free(entry->pool->zpool, entry->handle); > + zswap_pool_put(entry->pool); > + } > zswap_entry_cache_free(entry); > atomic_dec(&zswap_stored_pages); > zswap_update_total_size(); > @@ -953,6 +968,34 @@ static int zswap_shrink(void) > return ret; > } > > +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) > +{ > + unsigned int pos; > + unsigned long *page; > + > + page = (unsigned long *)ptr; > + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { > + if (page[pos] != page[0]) > + return 0; > + } > + *value = page[0]; > + return 1; > +} > + > +static void zswap_fill_page(void *ptr, unsigned long value) > +{ > + unsigned int pos; > + unsigned long *page; > + > + page = (unsigned long *)ptr; > + if (value == 0) > + memset(page, 0, PAGE_SIZE); > + else { > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) > + page[pos] = value; > + } > +} > + > /********************************* > * frontswap hooks > **********************************/ > @@ -965,7 +1008,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, > struct crypto_comp *tfm; > int ret; > unsigned int dlen = PAGE_SIZE, len; > - unsigned long handle; > + unsigned long handle, value; > char *buf; > u8 *src, *dst; > struct zswap_header *zhdr; > @@ -993,6 +1036,19 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, > goto reject; > } > > + if (zswap_same_filled_pages_enabled) { > + src = kmap_atomic(page); > + if (zswap_is_page_same_filled(src, &value)) { > + kunmap_atomic(src); > + entry->offset = offset; > + entry->length = 0; > + entry->value = value; > + atomic_inc(&zswap_same_filled_pages); > + goto insert_entry; > + } > + kunmap_atomic(src); > + } > + > /* if entry is successfully added, it keeps the reference */ > entry->pool = zswap_pool_current_get(); > if (!entry->pool) { > @@ -1037,6 +1093,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, > entry->handle = handle; > entry->length = dlen; > > +insert_entry: > /* map */ > spin_lock(&tree->lock); > do { > @@ -1089,6 +1146,13 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > } > spin_unlock(&tree->lock); > > + if (!entry->length) { > + dst = kmap_atomic(page); > + zswap_fill_page(dst, entry->value); > + kunmap_atomic(dst); > + goto freeentry; > + } > + > /* decompress */ > dlen = PAGE_SIZE; > src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle, > @@ -1101,6 +1165,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, > zpool_unmap_handle(entry->pool->zpool, entry->handle); > BUG_ON(ret); > > +freeentry: > spin_lock(&tree->lock); > zswap_entry_put(tree, entry); > spin_unlock(&tree->lock); > @@ -1209,6 +1274,8 @@ static int __init zswap_debugfs_init(void) > zswap_debugfs_root, &zswap_pool_total_size); > debugfs_create_atomic_t("stored_pages", S_IRUGO, > zswap_debugfs_root, &zswap_stored_pages); > + debugfs_create_atomic_t("same_filled_pages", 0444, > + zswap_debugfs_root, &zswap_same_filled_pages); > > return 0; > } > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy ` (3 preceding siblings ...) 2017-11-17 21:27 ` Dan Streetman @ 2017-11-20 23:46 ` Andrew Morton 2017-11-28 11:35 ` Dan Streetman [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p6> [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p4> 5 siblings, 2 replies; 22+ messages in thread From: Andrew Morton @ 2017-11-20 23:46 UTC (permalink / raw) To: srividya.dr Cc: sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, 18 Oct 2017 10:48:32 +0000 Srividya Desireddy <srividya.dr@samsung.com> wrote: > +/* Enable/disable handling same-value filled pages (enabled by default) */ > +static bool zswap_same_filled_pages_enabled = true; > +module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled, > + bool, 0644); Do we actually need this? Being able to disable the new feature shows a certain lack of confidence ;) I guess we can remove it later as that confidence grows. Please send a patch to document this parameter in Documentation/vm/zswap.txt. And if you have time, please check that the rest of that file is up-to-date? Thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling 2017-11-20 23:46 ` Andrew Morton @ 2017-11-28 11:35 ` Dan Streetman [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p6> 1 sibling, 0 replies; 22+ messages in thread From: Dan Streetman @ 2017-11-28 11:35 UTC (permalink / raw) To: Andrew Morton Cc: Srividya Desireddy, sjenning, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Mon, Nov 20, 2017 at 6:46 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > > On Wed, 18 Oct 2017 10:48:32 +0000 Srividya Desireddy <srividya.dr@samsung.com> wrote: > > > +/* Enable/disable handling same-value filled pages (enabled by default) */ > > +static bool zswap_same_filled_pages_enabled = true; > > +module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled, > > + bool, 0644); > > Do we actually need this? Being able to disable the new feature shows > a certain lack of confidence ;) I guess we can remove it later as that > confidence grows. No, it's not absolutely needed to have the param to enable/disable the feature, but my concern is around how many pages actually benefit from this, since it adds some overhead to check every page - the usefulness of the feature depends entirely on the system workload. So having a way to disable it is helpful, for use cases that know it won't benefit them. > > Please send a patch to document this parameter in > Documentation/vm/zswap.txt. And if you have time, please check that > the rest of that file is up-to-date? Srividya, can you send a patch to doc this feature please. I'll check the rest of the file is correct. > > Thanks. > ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p6>]
* [PATCH] zswap: Update with same-value filled page feature [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p6> @ 2017-11-29 15:34 ` Srividya Desireddy 2017-11-29 21:29 ` Dan Streetman 2017-12-06 11:48 ` [PATCH v2] " Srividya Desireddy 1 sibling, 1 reply; 22+ messages in thread From: Srividya Desireddy @ 2017-11-29 15:34 UTC (permalink / raw) To: Dan Streetman, sjenning, linux-mm, linux-kernel Cc: Dinakar Reddy Pathireddy, RAJIB BASU, Srikanth Mandalapu, SHARAN ALLUR, JUHUN KIM, Srividya Desireddy, srividya.desireddy From: Srividya Desireddy <srividya.dr@samsung.com> Date: Wed, 29 Nov 2017 20:23:15 +0530 Subject: [PATCH] zswap: Update with same-value filled page feature Updated zswap document with details on same-value filled pages identification feature. The usage of zswap.same_filled_pages_enabled module parameter is explained. Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> --- Documentation/vm/zswap.txt | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/Documentation/vm/zswap.txt b/Documentation/vm/zswap.txt index 89fff7d..cc015b5 100644 --- a/Documentation/vm/zswap.txt +++ b/Documentation/vm/zswap.txt @@ -98,5 +98,25 @@ request is made for a page in an old zpool, it is uncompressed using its original compressor. Once all pages are removed from an old zpool, the zpool and its compressor are freed. +Some of the pages in zswap are same-value filled pages (i.e. contents of the +page have same value or repetitive pattern). These pages include zero-filled +pages and they are handled differently. During store operation, a page is +checked if it is a same-value filled page before compressing it. If true, the +compressed length of the page is set to zero and the pattern or same-filled +value is stored. + +Same-value filled pages identification feature is enabled by default and can be +disabled at boot time by setting the "same_filled_pages_enabled" attribute to 0, +e.g. zswap.same_filled_pages_enabled=0. It can also be enabled and disabled at +runtime using the sysfs "same_filled_pages_enabled" attribute, e.g. + +echo 1 > /sys/module/zswap/parameters/same_filled_pages_enabled + +When zswap same-filled page identification is disabled at runtime, it will stop +checking for the same-value filled pages during store operation. However, the +existing pages which are marked as same-value filled pages will be loaded or +invalidated. + A debugfs interface is provided for various statistic about pool size, number -of pages stored, and various counters for the reasons pages are rejected. +of pages stored, same-value filled pages and various counters for the reasons +pages are rejected. -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Update with same-value filled page feature 2017-11-29 15:34 ` [PATCH] zswap: Update with same-value filled page feature Srividya Desireddy @ 2017-11-29 21:29 ` Dan Streetman 0 siblings, 0 replies; 22+ messages in thread From: Dan Streetman @ 2017-11-29 21:29 UTC (permalink / raw) To: Srividya Desireddy Cc: sjenning, linux-mm, linux-kernel, Dinakar Reddy Pathireddy, RAJIB BASU, Srikanth Mandalapu, SHARAN ALLUR, JUHUN KIM, srividya.desireddy On Wed, Nov 29, 2017 at 10:34 AM, Srividya Desireddy <srividya.dr@samsung.com> wrote: > From: Srividya Desireddy <srividya.dr@samsung.com> > Date: Wed, 29 Nov 2017 20:23:15 +0530 > Subject: [PATCH] zswap: Update with same-value filled page feature > > Updated zswap document with details on same-value filled > pages identification feature. > The usage of zswap.same_filled_pages_enabled module parameter > is explained. > > Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> > --- > Documentation/vm/zswap.txt | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/Documentation/vm/zswap.txt b/Documentation/vm/zswap.txt > index 89fff7d..cc015b5 100644 > --- a/Documentation/vm/zswap.txt > +++ b/Documentation/vm/zswap.txt > @@ -98,5 +98,25 @@ request is made for a page in an old zpool, it is uncompressed using its > original compressor. Once all pages are removed from an old zpool, the zpool > and its compressor are freed. > > +Some of the pages in zswap are same-value filled pages (i.e. contents of the > +page have same value or repetitive pattern). These pages include zero-filled > +pages and they are handled differently. During store operation, a page is > +checked if it is a same-value filled page before compressing it. If true, the > +compressed length of the page is set to zero and the pattern or same-filled > +value is stored. > + > +Same-value filled pages identification feature is enabled by default and can be > +disabled at boot time by setting the "same_filled_pages_enabled" attribute to 0, > +e.g. zswap.same_filled_pages_enabled=0. It can also be enabled and disabled at > +runtime using the sysfs "same_filled_pages_enabled" attribute, e.g. > + > +echo 1 > /sys/module/zswap/parameters/same_filled_pages_enabled > + > +When zswap same-filled page identification is disabled at runtime, it will stop > +checking for the same-value filled pages during store operation. However, the > +existing pages which are marked as same-value filled pages will be loaded or > +invalidated. On first read I thought you were saying existing pages were immediately loaded or invalidated, which of course is not the case. Can you update the sentence to clarify existing pages are not modified by disabling the param, like: "However, the existing pages which are marked as same-value filled pages remain stored unchanged until they are either loaded or invalidated." except for that the doc update looks good. > + > A debugfs interface is provided for various statistic about pool size, number > -of pages stored, and various counters for the reasons pages are rejected. > +of pages stored, same-value filled pages and various counters for the reasons > +pages are rejected. > -- > 2.7.4 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] zswap: Update with same-value filled page feature [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p6> 2017-11-29 15:34 ` [PATCH] zswap: Update with same-value filled page feature Srividya Desireddy @ 2017-12-06 11:48 ` Srividya Desireddy 2017-12-06 15:20 ` Dan Streetman 1 sibling, 1 reply; 22+ messages in thread From: Srividya Desireddy @ 2017-12-06 11:48 UTC (permalink / raw) To: Dan Streetman, sjenning, linux-mm, linux-kernel, Srividya Desireddy Cc: Dinakar Reddy Pathireddy, RAJIB BASU, Srikanth Mandalapu, SHARAN ALLUR, JUHUN KIM, srividya.desireddy From: Srividya Desireddy <srividya.dr@samsung.com> Date: Wed, 6 Dec 2017 16:29:50 +0530 Subject: [PATCH v2] zswap: Update with same-value filled page feature Changes since v1: Updated to clarify about zswap.same_filled_pages_enabled parameter. Updated zswap document with details on same-value filled pages identification feature. The usage of zswap.same_filled_pages_enabled module parameter is explained. Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> --- Documentation/vm/zswap.txt | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/Documentation/vm/zswap.txt b/Documentation/vm/zswap.txt index 89fff7d..0b3a114 100644 --- a/Documentation/vm/zswap.txt +++ b/Documentation/vm/zswap.txt @@ -98,5 +98,25 @@ request is made for a page in an old zpool, it is uncompressed using its original compressor. Once all pages are removed from an old zpool, the zpool and its compressor are freed. +Some of the pages in zswap are same-value filled pages (i.e. contents of the +page have same value or repetitive pattern). These pages include zero-filled +pages and they are handled differently. During store operation, a page is +checked if it is a same-value filled page before compressing it. If true, the +compressed length of the page is set to zero and the pattern or same-filled +value is stored. + +Same-value filled pages identification feature is enabled by default and can be +disabled at boot time by setting the "same_filled_pages_enabled" attribute to 0, +e.g. zswap.same_filled_pages_enabled=0. It can also be enabled and disabled at +runtime using the sysfs "same_filled_pages_enabled" attribute, e.g. + +echo 1 > /sys/module/zswap/parameters/same_filled_pages_enabled + +When zswap same-filled page identification is disabled at runtime, it will stop +checking for the same-value filled pages during store operation. However, the +existing pages which are marked as same-value filled pages remain stored +unchanged in zswap until they are either loaded or invalidated. + A debugfs interface is provided for various statistic about pool size, number -of pages stored, and various counters for the reasons pages are rejected. +of pages stored, same-value filled pages and various counters for the reasons +pages are rejected. -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2] zswap: Update with same-value filled page feature 2017-12-06 11:48 ` [PATCH v2] " Srividya Desireddy @ 2017-12-06 15:20 ` Dan Streetman 0 siblings, 0 replies; 22+ messages in thread From: Dan Streetman @ 2017-12-06 15:20 UTC (permalink / raw) To: Srividya Desireddy Cc: sjenning, linux-mm, linux-kernel, Dinakar Reddy Pathireddy, RAJIB BASU, Srikanth Mandalapu, SHARAN ALLUR, JUHUN KIM, srividya.desireddy, Andrew Morton On Wed, Dec 6, 2017 at 6:48 AM, Srividya Desireddy <srividya.dr@samsung.com> wrote: > From: Srividya Desireddy <srividya.dr@samsung.com> > Date: Wed, 6 Dec 2017 16:29:50 +0530 > Subject: [PATCH v2] zswap: Update with same-value filled page feature > > Changes since v1: > Updated to clarify about zswap.same_filled_pages_enabled parameter. > > Updated zswap document with details on same-value filled > pages identification feature. > The usage of zswap.same_filled_pages_enabled module parameter > is explained. > > Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> Acked-by: Dan Streetman <ddstreet@ieee.org> > --- > Documentation/vm/zswap.txt | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/Documentation/vm/zswap.txt b/Documentation/vm/zswap.txt > index 89fff7d..0b3a114 100644 > --- a/Documentation/vm/zswap.txt > +++ b/Documentation/vm/zswap.txt > @@ -98,5 +98,25 @@ request is made for a page in an old zpool, it is uncompressed using its > original compressor. Once all pages are removed from an old zpool, the zpool > and its compressor are freed. > > +Some of the pages in zswap are same-value filled pages (i.e. contents of the > +page have same value or repetitive pattern). These pages include zero-filled > +pages and they are handled differently. During store operation, a page is > +checked if it is a same-value filled page before compressing it. If true, the > +compressed length of the page is set to zero and the pattern or same-filled > +value is stored. > + > +Same-value filled pages identification feature is enabled by default and can be > +disabled at boot time by setting the "same_filled_pages_enabled" attribute to 0, > +e.g. zswap.same_filled_pages_enabled=0. It can also be enabled and disabled at > +runtime using the sysfs "same_filled_pages_enabled" attribute, e.g. > + > +echo 1 > /sys/module/zswap/parameters/same_filled_pages_enabled > + > +When zswap same-filled page identification is disabled at runtime, it will stop > +checking for the same-value filled pages during store operation. However, the > +existing pages which are marked as same-value filled pages remain stored > +unchanged in zswap until they are either loaded or invalidated. > + > A debugfs interface is provided for various statistic about pool size, number > -of pages stored, and various counters for the reasons pages are rejected. > +of pages stored, same-value filled pages and various counters for the reasons > +pages are rejected. > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p4>]
* [PATCH v2] zswap: Same-filled pages handling [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p4> @ 2017-11-21 14:18 ` Srividya Desireddy 0 siblings, 0 replies; 22+ messages in thread From: Srividya Desireddy @ 2017-11-21 14:18 UTC (permalink / raw) To: Andrew Morton, sjenning, ddstreet, linux-mm, willy, linux-kernel, nefelim4ag, penberg Cc: Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy, Srividya Desireddy, Sangamanatha . From: Srividya Desireddy <srividya.dr@samsung.com> Date: Sat, 18 Nov 2017 18:29:16 +0530 Subject: [PATCH v2] zswap: Same-filled pages handling Changes since v1 : Added memset_l instead of for loop. Zswap is a cache which compresses the pages that are being swapped out and stores them into a dynamically allocated RAM-based memory pool. Experiments have shown that around 10-20% of pages stored in zswap are same-filled pages (i.e. contents of the page are all same), but these pages are handled as normal pages by compressing and allocating memory in the pool. This patch adds a check in zswap_frontswap_store() to identify same-filled page before compression of the page. If the page is a same-filled page, set zswap_entry.length to zero, save the same-filled value and skip the compression of the page and alloction of memory in zpool. In zswap_frontswap_load(), check if value of zswap_entry.length is zero corresponding to the page to be loaded. If zswap_entry.length is zero, fill the page with same-filled value. This saves the decompression time during load. On a ARM Quad Core 32-bit device with 1.5GB RAM by launching and relaunching different applications, out of ~64000 pages stored in zswap, ~11000 pages were same-value filled pages (including zero-filled pages) and ~9000 pages were zero-filled pages. An average of 17% of pages(including zero-filled pages) in zswap are same-value filled pages and 14% pages are zero-filled pages. An average of 3% of pages are same-filled non-zero pages. The below table shows the execution time profiling with the patch. Baseline With patch % Improvement ----------------------------------------------------------------- *Zswap Store Time 26.5ms 18ms 32% (of same value pages) *Zswap Load Time (of same value pages) 25.5ms 13ms 49% ----------------------------------------------------------------- On Ubuntu PC with 2GB RAM, while executing kernel build and other test scripts and running multimedia applications, out of 360000 pages stored in zswap 78000(~22%) of pages were found to be same-value filled pages (including zero-filled pages) and 64000(~17%) are zero-filled pages. So an average of %5 of pages are same-filled non-zero pages. The below table shows the execution time profiling with the patch. Baseline With patch % Improvement ----------------------------------------------------------------- *Zswap Store Time 91ms 74ms 19% (of same value pages) *Zswap Load Time 50ms 7.5ms 85% (of same value pages) ----------------------------------------------------------------- *The execution times may vary with test device used. Signed-off-by: Srividya Desireddy <srividya.dr@samsung.com> --- mm/zswap.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index d39581a..1133b4ce 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -49,6 +49,8 @@ static u64 zswap_pool_total_size; /* The number of compressed pages currently stored in zswap */ static atomic_t zswap_stored_pages = ATOMIC_INIT(0); +/* The number of same-value filled pages currently stored in zswap */ +static atomic_t zswap_same_filled_pages = ATOMIC_INIT(0); /* * The statistics below are not protected from concurrent access for @@ -116,6 +118,11 @@ module_param_cb(zpool, &zswap_zpool_param_ops, &zswap_zpool_type, 0644); static unsigned int zswap_max_pool_percent = 20; module_param_named(max_pool_percent, zswap_max_pool_percent, uint, 0644); +/* Enable/disable handling same-value filled pages (enabled by default) */ +static bool zswap_same_filled_pages_enabled = true; +module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled, + bool, 0644); + /********************************* * data structures **********************************/ @@ -145,9 +152,10 @@ struct zswap_pool { * be held while changing the refcount. Since the lock must * be held, there is no reason to also make refcount atomic. * length - the length in bytes of the compressed page data. Needed during - * decompression + * decompression. For a same value filled page length is 0. * pool - the zswap_pool the entry's data is in * handle - zpool allocation handle that stores the compressed page data + * value - value of the same-value filled pages which have same content */ struct zswap_entry { struct rb_node rbnode; @@ -155,7 +163,10 @@ struct zswap_entry { int refcount; unsigned int length; struct zswap_pool *pool; - unsigned long handle; + union { + unsigned long handle; + unsigned long value; + }; }; struct zswap_header { @@ -320,8 +331,12 @@ static void zswap_rb_erase(struct rb_root *root, struct zswap_entry *entry) */ static void zswap_free_entry(struct zswap_entry *entry) { - zpool_free(entry->pool->zpool, entry->handle); - zswap_pool_put(entry->pool); + if (!entry->length) + atomic_dec(&zswap_same_filled_pages); + else { + zpool_free(entry->pool->zpool, entry->handle); + zswap_pool_put(entry->pool); + } zswap_entry_cache_free(entry); atomic_dec(&zswap_stored_pages); zswap_update_total_size(); @@ -953,6 +968,28 @@ static int zswap_shrink(void) return ret; } +static int zswap_is_page_same_filled(void *ptr, unsigned long *value) +{ + unsigned int pos; + unsigned long *page; + + page = (unsigned long *)ptr; + for (pos = 1; pos < PAGE_SIZE / sizeof(*page); pos++) { + if (page[pos] != page[0]) + return 0; + } + *value = page[0]; + return 1; +} + +static void zswap_fill_page(void *ptr, unsigned long value) +{ + unsigned long *page; + + page = (unsigned long *)ptr; + memset_l(page, value, PAGE_SIZE / sizeof(unsigned long)); +} + /********************************* * frontswap hooks **********************************/ @@ -965,7 +1002,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, struct crypto_comp *tfm; int ret; unsigned int dlen = PAGE_SIZE, len; - unsigned long handle; + unsigned long handle, value; char *buf; u8 *src, *dst; struct zswap_header *zhdr; @@ -993,6 +1030,19 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, goto reject; } + if (zswap_same_filled_pages_enabled) { + src = kmap_atomic(page); + if (zswap_is_page_same_filled(src, &value)) { + kunmap_atomic(src); + entry->offset = offset; + entry->length = 0; + entry->value = value; + atomic_inc(&zswap_same_filled_pages); + goto insert_entry; + } + kunmap_atomic(src); + } + /* if entry is successfully added, it keeps the reference */ entry->pool = zswap_pool_current_get(); if (!entry->pool) { @@ -1037,6 +1087,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset, entry->handle = handle; entry->length = dlen; +insert_entry: /* map */ spin_lock(&tree->lock); do { @@ -1089,6 +1140,13 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, } spin_unlock(&tree->lock); + if (!entry->length) { + dst = kmap_atomic(page); + zswap_fill_page(dst, entry->value); + kunmap_atomic(dst); + goto freeentry; + } + /* decompress */ dlen = PAGE_SIZE; src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle, @@ -1101,6 +1159,7 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset, zpool_unmap_handle(entry->pool->zpool, entry->handle); BUG_ON(ret); +freeentry: spin_lock(&tree->lock); zswap_entry_put(tree, entry); spin_unlock(&tree->lock); @@ -1209,6 +1268,8 @@ static int __init zswap_debugfs_init(void) zswap_debugfs_root, &zswap_pool_total_size); debugfs_create_atomic_t("stored_pages", S_IRUGO, zswap_debugfs_root, &zswap_stored_pages); + debugfs_create_atomic_t("same_filled_pages", 0444, + zswap_debugfs_root, &zswap_same_filled_pages); return 0; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] zswap: Same-filled pages handling [not found] <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p1> 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy @ 2017-10-18 14:43 ` Srividya Desireddy 1 sibling, 0 replies; 22+ messages in thread From: Srividya Desireddy @ 2017-10-18 14:43 UTC (permalink / raw) To: Matthew Wilcox, Timofey Titovets Cc: sjenning, ddstreet, linux-mm, linux-kernel, penberg, Dinakar Reddy Pathireddy, SHARAN ALLUR, RAJIB BASU, JUHUN KIM, srividya.desireddy On Wed, Oct 18, 2017 at 7:41 PM, Matthew Wilcox wrote: > On Wed, Oct 18, 2017 at 04:33:43PM +0300, Timofey Titovets wrote: >> 2017-10-18 15:34 GMT+03:00 Matthew Wilcox <willy@infradead.org>: >> > On Wed, Oct 18, 2017 at 10:48:32AM +0000, Srividya Desireddy wrote: >> >> +static void zswap_fill_page(void *ptr, unsigned long value) >> >> +{ >> >> + unsigned int pos; >> >> + unsigned long *page; >> >> + >> >> + page = (unsigned long *)ptr; >> >> + if (value == 0) >> >> + memset(page, 0, PAGE_SIZE); >> >> + else { >> >> + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) >> >> + page[pos] = value; >> >> + } >> >> +} >> > >> > I think you meant: >> > >> > static void zswap_fill_page(void *ptr, unsigned long value) >> > { >> > memset_l(ptr, value, PAGE_SIZE / sizeof(unsigned long)); >> > } >> >> IIRC kernel have special zero page, and if i understand correctly. >> You can map all zero pages to that zero page and not touch zswap completely. >> (Your situation look like some KSM case (i.e. KSM can handle pages >> with same content), but i'm not sure if that applicable there) > >You're confused by the word "same". What Srividya meant was that the >page is filled with a pattern, eg 0xfffefffefffefffe..., not that it is >the same as any other page. In kernel there is a special zero page or empty_zero_page which is in general allocated in paging_init() function, to map all zero pages. But, same-value-filled pages including zero pages exist in memory because applications may be initializing the allocated pages with a value and not using them; or the actual content written to the memory pages during execution itself is same-value, in case of multimedia data for example. I had earlier posted a patch with similar implementaion of KSM concept for Zswap: https://lkml.org/lkml/2016/8/17/171 https://lkml.org/lkml/2017/2/17/612 - Srividya ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-12-06 15:21 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p1> 2017-10-18 10:48 ` [PATCH] zswap: Same-filled pages handling Srividya Desireddy 2017-10-18 12:34 ` Matthew Wilcox 2017-10-18 13:33 ` Timofey Titovets 2017-10-18 14:11 ` Matthew Wilcox 2017-10-18 20:43 ` Andi Kleen 2017-10-19 1:10 ` Matthew Wilcox 2017-10-19 4:30 ` Andi Kleen 2017-10-19 13:24 ` Matthew Wilcox 2017-10-18 21:31 ` Timofey Titovets 2017-10-19 1:08 ` Matthew Wilcox [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p3> 2017-11-02 15:08 ` Srividya Desireddy 2017-11-17 22:10 ` Dan Streetman 2017-11-17 22:07 ` Dan Streetman 2017-11-17 21:27 ` Dan Streetman 2017-11-20 23:46 ` Andrew Morton 2017-11-28 11:35 ` Dan Streetman [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p6> 2017-11-29 15:34 ` [PATCH] zswap: Update with same-value filled page feature Srividya Desireddy 2017-11-29 21:29 ` Dan Streetman 2017-12-06 11:48 ` [PATCH v2] " Srividya Desireddy 2017-12-06 15:20 ` Dan Streetman [not found] ` <CGME20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p4> 2017-11-21 14:18 ` [PATCH v2] zswap: Same-filled pages handling Srividya Desireddy 2017-10-18 14:43 ` [PATCH] " Srividya Desireddy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).