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

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

[Wanpeng Li]

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

[PATCH v2 1/4] introduce zero filled pages handler

[Wanpeng Li]

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

Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>
Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 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

[PATCH v2 2/4] zero-filled pages awareness

[Wanpeng Li]

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 <dan.magenheimer@oracle.com>
Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 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

[PATCH v2 3/4] introduce zero-filled page stat count

[Wanpeng Li]

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

Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>
Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 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

[PATCH v2 4/4] clean TODO list

[Wanpeng Li]

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

Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>
Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 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

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

[Konrad Rzeszutek Wilk]

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 <dan.magenheimer@oracle.com>
> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> ---
>  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
> 

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

[Konrad Rzeszutek Wilk]

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 <dan.magenheimer@oracle.com>
> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> ---
>  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
> 

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

[Konrad Rzeszutek Wilk]

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 <dan.magenheimer@oracle.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> ---
>  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
> 

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

[Dan Magenheimer]

> From: Konrad Rzeszutek Wilk [mailto:konrad@darnok.org]
> 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?

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

[Ric Mason]

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 <dan.magenheimer@oracle.com>
>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> ---
>>   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 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>

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

[Konrad Rzeszutek Wilk]

On Sun, Mar 17, 2013 at 8:58 AM, Ric Mason <ric.masonn@gmail.com> 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?

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

[Konrad Rzeszutek Wilk]

On Sat, Mar 16, 2013 at 2:24 PM, Dan Magenheimer
<dan.magenheimer@oracle.com> wrote:
>> From: Konrad Rzeszutek Wilk [mailto:konrad@darnok.org]
>> 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?

>

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

[Ric Mason]

On 03/20/2013 12:41 AM, Konrad Rzeszutek Wilk wrote:
> On Sun, Mar 17, 2013 at 8:58 AM, Ric Mason <ric.masonn@gmail.com> 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.

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

[Dan Magenheimer]

> From: Konrad Rzeszutek Wilk [mailto:konrad@darnok.org]
> Sent: Tuesday, March 19, 2013 10:44 AM
> To: Dan Magenheimer
> Cc: Wanpeng Li; Greg Kroah-Hartman; Andrew Morton; Seth Jennings; Minchan Kim; linux-mm@kvack.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 1/4] introduce zero filled pages handler
> 
> On Sat, Mar 16, 2013 at 2:24 PM, Dan Magenheimer
> <dan.magenheimer@oracle.com> wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad@darnok.org]
> >> 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))