2013-03-14 10:09:06

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 0/4] zcache: Support zero-filled pages more efficiently

Changelog:
v1 -> v2:
* avoid changing tmem.[ch] entirely, spotted by Dan.
* don't accumulate [eph|pers]pageframe and [eph|pers]zpages for
zero-filled pages, spotted by Dan
* cleanup TODO list
* add Dan Acked-by.

Motivation:

- Seth Jennings points out compress zero-filled pages with LZO(a lossless
data compression algorithm) will waste memory and result in fragmentation.
https://lkml.org/lkml/2012/8/14/347
- Dan Magenheimer add "Support zero-filled pages more efficiently" feature
in zcache TODO list https://lkml.org/lkml/2013/2/13/503

Design:

- For store page, capture zero-filled pages(evicted clean page cache pages and
swap pages), but don't compress them, set pampd which store zpage address to
0x2(since 0x0 and 0x1 has already been ocuppied) to mark special zero-filled
case and take advantage of tmem infrastructure to transform handle-tuple(pool
id, object id, and an index) to a pampd. Twice compress zero-filled pages will
contribute to one zcache_[eph|pers]_pageframes count accumulated.
- For load page, traverse tmem hierachical to transform handle-tuple to pampd
and identify zero-filled case by pampd equal to 0x2 when filesystem reads
file pages or a page needs to be swapped in, then refill the page to zero
and return.

Test:

dd if=/dev/zero of=zerofile bs=1MB count=500
vmtouch -t zerofile
vmtouch -e zerofile

formula:
- fragmentation level = (zcache_[eph|pers]_pageframes * PAGE_SIZE - zcache_[eph|pers]_zbytes)
* 100 / (zcache_[eph|pers]_pageframes * PAGE_SIZE)
- memory zcache occupy = zcache_[eph|pers]_zbytes

Result:

without zero-filled awareness:
- fragmentation level: 98%
- memory zcache occupy: 238MB
with zero-filled awareness:
- fragmentation level: 0%
- memory zcache occupy: 0MB

Wanpeng Li (4):
introduce zero-filled pages handler
zero-filled pages awareness
introduce zero-filled pages stat count
clean TODO list

drivers/staging/zcache/TODO | 3 +-
drivers/staging/zcache/zcache-main.c | 119 ++++++++++++++++++++++++++++++++--
2 files changed, 114 insertions(+), 8 deletions(-)

--
1.7.7.6


2013-03-14 10:09:12

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 4/4] clean TODO list

Cleanup TODO list since support zero-filled pages more efficiently has
already done by this patchset.

Acked-by: Dan Magenheimer <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
drivers/staging/zcache/TODO | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/zcache/TODO b/drivers/staging/zcache/TODO
index c1e26d4..9e755d3 100644
--- a/drivers/staging/zcache/TODO
+++ b/drivers/staging/zcache/TODO
@@ -65,5 +65,4 @@ ZCACHE FUTURE NEW FUNCTIONALITY

A. Support zsmalloc as an alternative high-density allocator
(See https://lkml.org/lkml/2013/1/23/511)
-B. Support zero-filled pages more efficiently
-C. Possibly support three zbuds per pageframe when space allows
+B. Possibly support three zbuds per pageframe when space allows
--
1.7.7.6

2013-03-14 10:09:19

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 3/4] introduce zero-filled page stat count

Introduce zero-filled page statistics to monitor the number of
zero-filled pages.

Acked-by: Dan Magenheimer <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
drivers/staging/zcache/zcache-main.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index db200b4..2091a4d 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -196,6 +196,7 @@ static ssize_t zcache_eph_nonactive_puts_ignored;
static ssize_t zcache_pers_nonactive_puts_ignored;
static ssize_t zcache_writtenback_pages;
static ssize_t zcache_outstanding_writeback_pages;
+static ssize_t zcache_pages_zero;

#ifdef CONFIG_DEBUG_FS
#include <linux/debugfs.h>
@@ -257,6 +258,7 @@ static int zcache_debugfs_init(void)
zdfs("outstanding_writeback_pages", S_IRUGO, root,
&zcache_outstanding_writeback_pages);
zdfs("writtenback_pages", S_IRUGO, root, &zcache_writtenback_pages);
+ zdfs("pages_zero", S_IRUGO, root, &zcache_pages_zero);
return 0;
}
#undef zdebugfs
@@ -326,6 +328,7 @@ void zcache_dump(void)
pr_info("zcache: outstanding_writeback_pages=%zd\n",
zcache_outstanding_writeback_pages);
pr_info("zcache: writtenback_pages=%zd\n", zcache_writtenback_pages);
+ pr_info("zcache: pages_zero=%zd\n", zcache_pages_zero);
}
#endif

@@ -562,6 +565,7 @@ static void *zcache_pampd_eph_create(char *data, size_t size, bool raw,
kunmap_atomic(user_mem);
clen = 0;
zero_filled = true;
+ zcache_pages_zero++;
goto got_pampd;
}
kunmap_atomic(user_mem);
@@ -645,6 +649,7 @@ static void *zcache_pampd_pers_create(char *data, size_t size, bool raw,
kunmap_atomic(user_mem);
clen = 0;
zero_filled = true;
+ zcache_pages_zero++;
goto got_pampd;
}
kunmap_atomic(user_mem);
@@ -866,6 +871,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
zpages = 0;
if (!raw)
*sizep = PAGE_SIZE;
+ zcache_pages_zero--;
goto zero_fill;
}

@@ -922,6 +928,7 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
zero_filled = true;
zsize = 0;
zpages = 0;
+ zcache_pages_zero--;
}

if (pampd_is_remote(pampd) && !zero_filled) {
--
1.7.7.6

2013-03-14 10:09:25

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 2/4] zero-filled pages awareness

Compression of zero-filled pages can unneccessarily cause internal
fragmentation, and thus waste memory. This special case can be
optimized.

This patch captures zero-filled pages, and marks their corresponding
zcache backing page entry as zero-filled. Whenever such zero-filled
page is retrieved, we fill the page frame with zero.

Acked-by: Dan Magenheimer <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
drivers/staging/zcache/zcache-main.c | 86 +++++++++++++++++++++++++++++++--
1 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index b71e033..db200b4 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -59,6 +59,11 @@ static inline void frontswap_tmem_exclusive_gets(bool b)
}
#endif

+/*
+ * mark pampd to special value in order that later
+ * retrieve will identify zero-filled pages
+ */
+
/* enable (or fix code) when Seth's patches are accepted upstream */
#define zcache_writeback_enabled 0

@@ -543,7 +548,23 @@ static void *zcache_pampd_eph_create(char *data, size_t size, bool raw,
{
void *pampd = NULL, *cdata = data;
unsigned clen = size;
+ bool zero_filled = false;
struct page *page = (struct page *)(data), *newpage;
+ char *user_mem;
+
+ user_mem = kmap_atomic(page);
+
+ /*
+ * Compressing zero-filled pages will waste memory and introduce
+ * serious fragmentation, skip it to avoid overhead
+ */
+ if (page_zero_filled(user_mem)) {
+ kunmap_atomic(user_mem);
+ clen = 0;
+ zero_filled = true;
+ goto got_pampd;
+ }
+ kunmap_atomic(user_mem);

if (!raw) {
zcache_compress(page, &cdata, &clen);
@@ -592,6 +613,8 @@ got_pampd:
zcache_eph_zpages_max = zcache_eph_zpages;
if (ramster_enabled && raw)
ramster_count_foreign_pages(true, 1);
+ if (zero_filled)
+ pampd = (void *)ZERO_FILLED;
out:
return pampd;
}
@@ -601,14 +624,31 @@ static void *zcache_pampd_pers_create(char *data, size_t size, bool raw,
{
void *pampd = NULL, *cdata = data;
unsigned clen = size;
+ bool zero_filled = false;
struct page *page = (struct page *)(data), *newpage;
unsigned long zbud_mean_zsize;
unsigned long curr_pers_zpages, total_zsize;
+ char *user_mem;

if (data == NULL) {
BUG_ON(!ramster_enabled);
goto create_pampd;
}
+
+ user_mem = kmap_atomic(page);
+
+ /*
+ * Compressing zero-filled pages will waste memory and introduce
+ * serious fragmentation, skip it to avoid overhead
+ */
+ if (page_zero_filled(page)) {
+ kunmap_atomic(user_mem);
+ clen = 0;
+ zero_filled = true;
+ goto got_pampd;
+ }
+ kunmap_atomic(user_mem);
+
curr_pers_zpages = zcache_pers_zpages;
/* FIXME CONFIG_RAMSTER... subtract atomic remote_pers_pages here? */
if (!raw)
@@ -674,6 +714,8 @@ got_pampd:
zcache_pers_zbytes_max = zcache_pers_zbytes;
if (ramster_enabled && raw)
ramster_count_foreign_pages(false, 1);
+ if (zero_filled)
+ pampd = (void *)ZERO_FILLED;
out:
return pampd;
}
@@ -735,7 +777,8 @@ out:
*/
void zcache_pampd_create_finish(void *pampd, bool eph)
{
- zbud_create_finish((struct zbudref *)pampd, eph);
+ if (pampd != (void *)ZERO_FILLED)
+ zbud_create_finish((struct zbudref *)pampd, eph);
}

/*
@@ -780,6 +823,14 @@ static int zcache_pampd_get_data(char *data, size_t *sizep, bool raw,
BUG_ON(preemptible());
BUG_ON(eph); /* fix later if shared pools get implemented */
BUG_ON(pampd_is_remote(pampd));
+
+ if (pampd == (void *)ZERO_FILLED) {
+ handle_zero_page(data);
+ if (!raw)
+ *sizep = PAGE_SIZE;
+ return 0;
+ }
+
if (raw)
ret = zbud_copy_from_zbud(data, (struct zbudref *)pampd,
sizep, eph);
@@ -801,12 +852,23 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
struct tmem_oid *oid, uint32_t index)
{
int ret;
- bool eph = !is_persistent(pool);
+ bool eph = !is_persistent(pool), zero_filled = false;
struct page *page = NULL;
unsigned int zsize, zpages;

BUG_ON(preemptible());
BUG_ON(pampd_is_remote(pampd));
+
+ if (pampd == (void *)ZERO_FILLED) {
+ handle_zero_page(data);
+ zero_filled = true;
+ zsize = 0;
+ zpages = 0;
+ if (!raw)
+ *sizep = PAGE_SIZE;
+ goto zero_fill;
+ }
+
if (raw)
ret = zbud_copy_from_zbud(data, (struct zbudref *)pampd,
sizep, eph);
@@ -818,6 +880,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
}
page = zbud_free_and_delist((struct zbudref *)pampd, eph,
&zsize, &zpages);
+zero_fill:
if (eph) {
if (page)
zcache_eph_pageframes =
@@ -837,7 +900,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
}
if (!is_local_client(pool->client))
ramster_count_foreign_pages(eph, -1);
- if (page)
+ if (page && !zero_filled)
zcache_free_page(page);
return ret;
}
@@ -851,16 +914,27 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
{
struct page *page = NULL;
unsigned int zsize, zpages;
+ bool zero_filled = false;

BUG_ON(preemptible());
- if (pampd_is_remote(pampd)) {
+
+ if (pampd == (void *)ZERO_FILLED) {
+ zero_filled = true;
+ zsize = 0;
+ zpages = 0;
+ }
+
+ if (pampd_is_remote(pampd) && !zero_filled) {
+
BUG_ON(!ramster_enabled);
pampd = ramster_pampd_free(pampd, pool, oid, index, acct);
if (pampd == NULL)
return;
}
if (is_ephemeral(pool)) {
- page = zbud_free_and_delist((struct zbudref *)pampd,
+ if (!zero_filled)
+ page = zbud_free_and_delist((struct zbudref *)pampd,
+
true, &zsize, &zpages);
if (page)
zcache_eph_pageframes =
@@ -883,7 +957,7 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
}
if (!is_local_client(pool->client))
ramster_count_foreign_pages(is_ephemeral(pool), -1);
- if (page)
+ if (page && !zero_filled)
zcache_free_page(page);
}

--
1.7.7.6

2013-03-14 10:09:56

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 1/4] introduce zero filled pages handler

Introduce zero-filled pages handler to capture and handle zero pages.

Acked-by: Dan Magenheimer <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
drivers/staging/zcache/zcache-main.c | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index 328898e..b71e033 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -460,6 +460,32 @@ static void zcache_obj_free(struct tmem_obj *obj, struct tmem_pool *pool)
kmem_cache_free(zcache_obj_cache, obj);
}

+static bool page_zero_filled(void *ptr)
+{
+ unsigned int pos;
+ unsigned long *page;
+
+ page = (unsigned long *)ptr;
+
+ for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) {
+ if (page[pos])
+ return false;
+ }
+
+ return true;
+}
+
+static void handle_zero_page(void *page)
+{
+ void *user_mem;
+
+ user_mem = kmap_atomic(page);
+ memset(user_mem, 0, PAGE_SIZE);
+ kunmap_atomic(user_mem);
+
+ flush_dcache_page(page);
+}
+
static struct tmem_hostops zcache_hostops = {
.obj_alloc = zcache_obj_alloc,
.obj_free = zcache_obj_free,
--
1.7.7.6

2013-03-16 13:03:11

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler

On Thu, Mar 14, 2013 at 06:08:14PM +0800, Wanpeng Li wrote:
> Introduce zero-filled pages handler to capture and handle zero pages.
>
> Acked-by: Dan Magenheimer <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> drivers/staging/zcache/zcache-main.c | 26 ++++++++++++++++++++++++++
> 1 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index 328898e..b71e033 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -460,6 +460,32 @@ static void zcache_obj_free(struct tmem_obj *obj, struct tmem_pool *pool)
> kmem_cache_free(zcache_obj_cache, obj);
> }
>
> +static bool page_zero_filled(void *ptr)

Shouldn't this be 'struct page *p' ?
> +{
> + unsigned int pos;
> + unsigned long *page;
> +
> + page = (unsigned long *)ptr;

That way you can avoid this casting.
> +
> + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) {
> + if (page[pos])
> + return false;

Perhaps allocate a static page filled with zeros and just do memcmp?
> + }
> +
> + return true;
> +}
> +
> +static void handle_zero_page(void *page)
> +{
> + void *user_mem;
> +
> + user_mem = kmap_atomic(page);
> + memset(user_mem, 0, PAGE_SIZE);
> + kunmap_atomic(user_mem);
> +
> + flush_dcache_page(page);

This is new. Could you kindly explain why it is needed? Thanks.
> +}
> +
> static struct tmem_hostops zcache_hostops = {
> .obj_alloc = zcache_obj_alloc,
> .obj_free = zcache_obj_free,
> --
> 1.7.7.6
>

2013-03-16 13:06:47

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] introduce zero-filled page stat count

On Thu, Mar 14, 2013 at 06:08:16PM +0800, Wanpeng Li wrote:
> Introduce zero-filled page statistics to monitor the number of
> zero-filled pages.

Hm, you must be using an older version of the driver. Please
rebase it against Greg KH's staging tree. This is where most if not
all of the DebugFS counters got moved to a different file.

>
> Acked-by: Dan Magenheimer <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> drivers/staging/zcache/zcache-main.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index db200b4..2091a4d 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -196,6 +196,7 @@ static ssize_t zcache_eph_nonactive_puts_ignored;
> static ssize_t zcache_pers_nonactive_puts_ignored;
> static ssize_t zcache_writtenback_pages;
> static ssize_t zcache_outstanding_writeback_pages;
> +static ssize_t zcache_pages_zero;
>
> #ifdef CONFIG_DEBUG_FS
> #include <linux/debugfs.h>
> @@ -257,6 +258,7 @@ static int zcache_debugfs_init(void)
> zdfs("outstanding_writeback_pages", S_IRUGO, root,
> &zcache_outstanding_writeback_pages);
> zdfs("writtenback_pages", S_IRUGO, root, &zcache_writtenback_pages);
> + zdfs("pages_zero", S_IRUGO, root, &zcache_pages_zero);
> return 0;
> }
> #undef zdebugfs
> @@ -326,6 +328,7 @@ void zcache_dump(void)
> pr_info("zcache: outstanding_writeback_pages=%zd\n",
> zcache_outstanding_writeback_pages);
> pr_info("zcache: writtenback_pages=%zd\n", zcache_writtenback_pages);
> + pr_info("zcache: pages_zero=%zd\n", zcache_pages_zero);
> }
> #endif
>
> @@ -562,6 +565,7 @@ static void *zcache_pampd_eph_create(char *data, size_t size, bool raw,
> kunmap_atomic(user_mem);
> clen = 0;
> zero_filled = true;
> + zcache_pages_zero++;
> goto got_pampd;
> }
> kunmap_atomic(user_mem);
> @@ -645,6 +649,7 @@ static void *zcache_pampd_pers_create(char *data, size_t size, bool raw,
> kunmap_atomic(user_mem);
> clen = 0;
> zero_filled = true;
> + zcache_pages_zero++;
> goto got_pampd;
> }
> kunmap_atomic(user_mem);
> @@ -866,6 +871,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
> zpages = 0;
> if (!raw)
> *sizep = PAGE_SIZE;
> + zcache_pages_zero--;
> goto zero_fill;
> }
>
> @@ -922,6 +928,7 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
> zero_filled = true;
> zsize = 0;
> zpages = 0;
> + zcache_pages_zero--;
> }
>
> if (pampd_is_remote(pampd) && !zero_filled) {
> --
> 1.7.7.6
>

2013-03-16 13:07:29

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] zero-filled pages awareness

On Thu, Mar 14, 2013 at 06:08:15PM +0800, Wanpeng Li wrote:
> Compression of zero-filled pages can unneccessarily cause internal
> fragmentation, and thus waste memory. This special case can be
> optimized.
>
> This patch captures zero-filled pages, and marks their corresponding
> zcache backing page entry as zero-filled. Whenever such zero-filled
> page is retrieved, we fill the page frame with zero.
>
> Acked-by: Dan Magenheimer <[email protected]>

Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> drivers/staging/zcache/zcache-main.c | 86 +++++++++++++++++++++++++++++++--
> 1 files changed, 80 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index b71e033..db200b4 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -59,6 +59,11 @@ static inline void frontswap_tmem_exclusive_gets(bool b)
> }
> #endif
>
> +/*
> + * mark pampd to special value in order that later
> + * retrieve will identify zero-filled pages
> + */
> +
> /* enable (or fix code) when Seth's patches are accepted upstream */
> #define zcache_writeback_enabled 0
>
> @@ -543,7 +548,23 @@ static void *zcache_pampd_eph_create(char *data, size_t size, bool raw,
> {
> void *pampd = NULL, *cdata = data;
> unsigned clen = size;
> + bool zero_filled = false;
> struct page *page = (struct page *)(data), *newpage;
> + char *user_mem;
> +
> + user_mem = kmap_atomic(page);
> +
> + /*
> + * Compressing zero-filled pages will waste memory and introduce
> + * serious fragmentation, skip it to avoid overhead
> + */
> + if (page_zero_filled(user_mem)) {
> + kunmap_atomic(user_mem);
> + clen = 0;
> + zero_filled = true;
> + goto got_pampd;
> + }
> + kunmap_atomic(user_mem);
>
> if (!raw) {
> zcache_compress(page, &cdata, &clen);
> @@ -592,6 +613,8 @@ got_pampd:
> zcache_eph_zpages_max = zcache_eph_zpages;
> if (ramster_enabled && raw)
> ramster_count_foreign_pages(true, 1);
> + if (zero_filled)
> + pampd = (void *)ZERO_FILLED;
> out:
> return pampd;
> }
> @@ -601,14 +624,31 @@ static void *zcache_pampd_pers_create(char *data, size_t size, bool raw,
> {
> void *pampd = NULL, *cdata = data;
> unsigned clen = size;
> + bool zero_filled = false;
> struct page *page = (struct page *)(data), *newpage;
> unsigned long zbud_mean_zsize;
> unsigned long curr_pers_zpages, total_zsize;
> + char *user_mem;
>
> if (data == NULL) {
> BUG_ON(!ramster_enabled);
> goto create_pampd;
> }
> +
> + user_mem = kmap_atomic(page);
> +
> + /*
> + * Compressing zero-filled pages will waste memory and introduce
> + * serious fragmentation, skip it to avoid overhead
> + */
> + if (page_zero_filled(page)) {
> + kunmap_atomic(user_mem);
> + clen = 0;
> + zero_filled = true;
> + goto got_pampd;
> + }
> + kunmap_atomic(user_mem);
> +
> curr_pers_zpages = zcache_pers_zpages;
> /* FIXME CONFIG_RAMSTER... subtract atomic remote_pers_pages here? */
> if (!raw)
> @@ -674,6 +714,8 @@ got_pampd:
> zcache_pers_zbytes_max = zcache_pers_zbytes;
> if (ramster_enabled && raw)
> ramster_count_foreign_pages(false, 1);
> + if (zero_filled)
> + pampd = (void *)ZERO_FILLED;
> out:
> return pampd;
> }
> @@ -735,7 +777,8 @@ out:
> */
> void zcache_pampd_create_finish(void *pampd, bool eph)
> {
> - zbud_create_finish((struct zbudref *)pampd, eph);
> + if (pampd != (void *)ZERO_FILLED)
> + zbud_create_finish((struct zbudref *)pampd, eph);
> }
>
> /*
> @@ -780,6 +823,14 @@ static int zcache_pampd_get_data(char *data, size_t *sizep, bool raw,
> BUG_ON(preemptible());
> BUG_ON(eph); /* fix later if shared pools get implemented */
> BUG_ON(pampd_is_remote(pampd));
> +
> + if (pampd == (void *)ZERO_FILLED) {
> + handle_zero_page(data);
> + if (!raw)
> + *sizep = PAGE_SIZE;
> + return 0;
> + }
> +
> if (raw)
> ret = zbud_copy_from_zbud(data, (struct zbudref *)pampd,
> sizep, eph);
> @@ -801,12 +852,23 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
> struct tmem_oid *oid, uint32_t index)
> {
> int ret;
> - bool eph = !is_persistent(pool);
> + bool eph = !is_persistent(pool), zero_filled = false;
> struct page *page = NULL;
> unsigned int zsize, zpages;
>
> BUG_ON(preemptible());
> BUG_ON(pampd_is_remote(pampd));
> +
> + if (pampd == (void *)ZERO_FILLED) {
> + handle_zero_page(data);
> + zero_filled = true;
> + zsize = 0;
> + zpages = 0;
> + if (!raw)
> + *sizep = PAGE_SIZE;
> + goto zero_fill;
> + }
> +
> if (raw)
> ret = zbud_copy_from_zbud(data, (struct zbudref *)pampd,
> sizep, eph);
> @@ -818,6 +880,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
> }
> page = zbud_free_and_delist((struct zbudref *)pampd, eph,
> &zsize, &zpages);
> +zero_fill:
> if (eph) {
> if (page)
> zcache_eph_pageframes =
> @@ -837,7 +900,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
> }
> if (!is_local_client(pool->client))
> ramster_count_foreign_pages(eph, -1);
> - if (page)
> + if (page && !zero_filled)
> zcache_free_page(page);
> return ret;
> }
> @@ -851,16 +914,27 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
> {
> struct page *page = NULL;
> unsigned int zsize, zpages;
> + bool zero_filled = false;
>
> BUG_ON(preemptible());
> - if (pampd_is_remote(pampd)) {
> +
> + if (pampd == (void *)ZERO_FILLED) {
> + zero_filled = true;
> + zsize = 0;
> + zpages = 0;
> + }
> +
> + if (pampd_is_remote(pampd) && !zero_filled) {
> +
> BUG_ON(!ramster_enabled);
> pampd = ramster_pampd_free(pampd, pool, oid, index, acct);
> if (pampd == NULL)
> return;
> }
> if (is_ephemeral(pool)) {
> - page = zbud_free_and_delist((struct zbudref *)pampd,
> + if (!zero_filled)
> + page = zbud_free_and_delist((struct zbudref *)pampd,
> +
> true, &zsize, &zpages);
> if (page)
> zcache_eph_pageframes =
> @@ -883,7 +957,7 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
> }
> if (!is_local_client(pool->client))
> ramster_count_foreign_pages(is_ephemeral(pool), -1);
> - if (page)
> + if (page && !zero_filled)
> zcache_free_page(page);
> }
>
> --
> 1.7.7.6
>

2013-03-16 18:24:59

by Dan Magenheimer

[permalink] [raw]
Subject: RE: [PATCH v2 1/4] introduce zero filled pages handler

> From: Konrad Rzeszutek Wilk [mailto:[email protected]]
> Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler
>
> > +
> > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) {
> > + if (page[pos])
> > + return false;
>
> Perhaps allocate a static page filled with zeros and just do memcmp?

That seems like a bad idea. Why compare two different
memory locations when comparing one memory location
to a register will do?

2013-03-17 12:58:59

by Ric Mason

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] introduce zero-filled page stat count

Hi Konrad,
On 03/16/2013 09:06 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 14, 2013 at 06:08:16PM +0800, Wanpeng Li wrote:
>> Introduce zero-filled page statistics to monitor the number of
>> zero-filled pages.
> Hm, you must be using an older version of the driver. Please
> rebase it against Greg KH's staging tree. This is where most if not
> all of the DebugFS counters got moved to a different file.

It seems that zcache debugfs in Greg's staging-next is buggy, Could you
test it?

>
>> Acked-by: Dan Magenheimer <[email protected]>
>> Signed-off-by: Wanpeng Li <[email protected]>
>> ---
>> drivers/staging/zcache/zcache-main.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
>> index db200b4..2091a4d 100644
>> --- a/drivers/staging/zcache/zcache-main.c
>> +++ b/drivers/staging/zcache/zcache-main.c
>> @@ -196,6 +196,7 @@ static ssize_t zcache_eph_nonactive_puts_ignored;
>> static ssize_t zcache_pers_nonactive_puts_ignored;
>> static ssize_t zcache_writtenback_pages;
>> static ssize_t zcache_outstanding_writeback_pages;
>> +static ssize_t zcache_pages_zero;
>>
>> #ifdef CONFIG_DEBUG_FS
>> #include <linux/debugfs.h>
>> @@ -257,6 +258,7 @@ static int zcache_debugfs_init(void)
>> zdfs("outstanding_writeback_pages", S_IRUGO, root,
>> &zcache_outstanding_writeback_pages);
>> zdfs("writtenback_pages", S_IRUGO, root, &zcache_writtenback_pages);
>> + zdfs("pages_zero", S_IRUGO, root, &zcache_pages_zero);
>> return 0;
>> }
>> #undef zdebugfs
>> @@ -326,6 +328,7 @@ void zcache_dump(void)
>> pr_info("zcache: outstanding_writeback_pages=%zd\n",
>> zcache_outstanding_writeback_pages);
>> pr_info("zcache: writtenback_pages=%zd\n", zcache_writtenback_pages);
>> + pr_info("zcache: pages_zero=%zd\n", zcache_pages_zero);
>> }
>> #endif
>>
>> @@ -562,6 +565,7 @@ static void *zcache_pampd_eph_create(char *data, size_t size, bool raw,
>> kunmap_atomic(user_mem);
>> clen = 0;
>> zero_filled = true;
>> + zcache_pages_zero++;
>> goto got_pampd;
>> }
>> kunmap_atomic(user_mem);
>> @@ -645,6 +649,7 @@ static void *zcache_pampd_pers_create(char *data, size_t size, bool raw,
>> kunmap_atomic(user_mem);
>> clen = 0;
>> zero_filled = true;
>> + zcache_pages_zero++;
>> goto got_pampd;
>> }
>> kunmap_atomic(user_mem);
>> @@ -866,6 +871,7 @@ static int zcache_pampd_get_data_and_free(char *data, size_t *sizep, bool raw,
>> zpages = 0;
>> if (!raw)
>> *sizep = PAGE_SIZE;
>> + zcache_pages_zero--;
>> goto zero_fill;
>> }
>>
>> @@ -922,6 +928,7 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
>> zero_filled = true;
>> zsize = 0;
>> zpages = 0;
>> + zcache_pages_zero--;
>> }
>>
>> if (pampd_is_remote(pampd) && !zero_filled) {
>> --
>> 1.7.7.6
>>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

2013-03-19 16:41:53

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] introduce zero-filled page stat count

On Sun, Mar 17, 2013 at 8:58 AM, Ric Mason <[email protected]> wrote:
> Hi Konrad,
>
> On 03/16/2013 09:06 PM, Konrad Rzeszutek Wilk wrote:
>>
>> On Thu, Mar 14, 2013 at 06:08:16PM +0800, Wanpeng Li wrote:
>>>
>>> Introduce zero-filled page statistics to monitor the number of
>>> zero-filled pages.
>>
>> Hm, you must be using an older version of the driver. Please
>> rebase it against Greg KH's staging tree. This is where most if not
>> all of the DebugFS counters got moved to a different file.
>
>
> It seems that zcache debugfs in Greg's staging-next is buggy, Could you test
> it?
>
Could you email me what the issue you are seeing?

2013-03-19 16:44:24

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler

On Sat, Mar 16, 2013 at 2:24 PM, Dan Magenheimer
<[email protected]> wrote:
>> From: Konrad Rzeszutek Wilk [mailto:[email protected]]
>> Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler
>>
>> > +
>> > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) {
>> > + if (page[pos])
>> > + return false;
>>
>> Perhaps allocate a static page filled with zeros and just do memcmp?
>
> That seems like a bad idea. Why compare two different
> memory locations when comparing one memory location
> to a register will do?
>

Good point. I was hoping there was an fast memcmp that would
do fancy SSE registers. But it is memory against memory instead of
registers.

Perhaps a cunning trick would be to check (as a shortcircuit)
check against 'empty_zero_page' and if that check fails, then try
to do the check for each byte in the code?

>

2013-03-19 23:38:44

by Ric Mason

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] introduce zero-filled page stat count

On 03/20/2013 12:41 AM, Konrad Rzeszutek Wilk wrote:
> On Sun, Mar 17, 2013 at 8:58 AM, Ric Mason <[email protected]> wrote:
>> Hi Konrad,
>>
>> On 03/16/2013 09:06 PM, Konrad Rzeszutek Wilk wrote:
>>> On Thu, Mar 14, 2013 at 06:08:16PM +0800, Wanpeng Li wrote:
>>>> Introduce zero-filled page statistics to monitor the number of
>>>> zero-filled pages.
>>> Hm, you must be using an older version of the driver. Please
>>> rebase it against Greg KH's staging tree. This is where most if not
>>> all of the DebugFS counters got moved to a different file.
>>
>> It seems that zcache debugfs in Greg's staging-next is buggy, Could you test
>> it?
>>
> Could you email me what the issue you are seeing?
They have already fixed in Wanpeng's patchset v4.

2013-03-25 19:08:22

by Dan Magenheimer

[permalink] [raw]
Subject: RE: [PATCH v2 1/4] introduce zero filled pages handler

> From: Konrad Rzeszutek Wilk [mailto:[email protected]]
> Sent: Tuesday, March 19, 2013 10:44 AM
> To: Dan Magenheimer
> Cc: Wanpeng Li; Greg Kroah-Hartman; Andrew Morton; Seth Jennings; Minchan Kim; [email protected];
> [email protected]
> Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler
>
> On Sat, Mar 16, 2013 at 2:24 PM, Dan Magenheimer
> <[email protected]> wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:[email protected]]
> >> Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler
> >>
> >> > +
> >> > + for (pos = 0; pos < PAGE_SIZE / sizeof(*page); pos++) {
> >> > + if (page[pos])
> >> > + return false;
> >>
> >> Perhaps allocate a static page filled with zeros and just do memcmp?
> >
> > That seems like a bad idea. Why compare two different
> > memory locations when comparing one memory location
> > to a register will do?
>
> Good point. I was hoping there was an fast memcmp that would
> do fancy SSE registers. But it is memory against memory instead of
> registers.
>
> Perhaps a cunning trick would be to check (as a shortcircuit)
> check against 'empty_zero_page' and if that check fails, then try
> to do the check for each byte in the code?

Curious about this, I added some code to check for this case.
In my test run, the conditional "if (page == ZERO_PAGE(0))"
was never true, for >200000 pages passed through frontswap that
were zero-filled. My test run is certainly not conclusive,
but perhaps some other code in the swap subsystem disqualifies
ZERO_PAGE as a candidate for swapping? Or maybe it is accessed
frequently enough that it never falls out of the active-anonymous
page queue?

Dan

P.S. In arch/x86/include/asm/pgtable.h:

#define ZERO_PAGE(vaddr) (virt_to_page(empty_zero_page))