Re: [PATCH v2] mm/zswap: Check all pool pages instead of one poolpages

From: Minchan Kim
Date: Mon Jan 20 2014 - 21:31:55 EST


Hello Cai,

On Mon, Jan 20, 2014 at 03:50:18PM +0800, Cai Liu wrote:
> zswap can support multiple swapfiles. So we need to check
> all zbud pool pages in zswap.
>
> Version 2:
> * add *total_zbud_pages* in zbud to record all the pages in pools
> * move the updating of pool pages statistics to
> alloc_zbud_page/free_zbud_page to hide the details
>
> Signed-off-by: Cai Liu <cai.liu@xxxxxxxxxxx>
> ---
> include/linux/zbud.h | 2 +-
> mm/zbud.c | 44 ++++++++++++++++++++++++++++++++------------
> mm/zswap.c | 4 ++--
> 3 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/zbud.h b/include/linux/zbud.h
> index 2571a5c..1dbc13e 100644
> --- a/include/linux/zbud.h
> +++ b/include/linux/zbud.h
> @@ -17,6 +17,6 @@ void zbud_free(struct zbud_pool *pool, unsigned long handle);
> int zbud_reclaim_page(struct zbud_pool *pool, unsigned int retries);
> void *zbud_map(struct zbud_pool *pool, unsigned long handle);
> void zbud_unmap(struct zbud_pool *pool, unsigned long handle);
> -u64 zbud_get_pool_size(struct zbud_pool *pool);
> +u64 zbud_get_pool_size(void);
>
> #endif /* _ZBUD_H_ */
> diff --git a/mm/zbud.c b/mm/zbud.c
> index 9451361..711aaf4 100644
> --- a/mm/zbud.c
> +++ b/mm/zbud.c
> @@ -52,6 +52,13 @@
> #include <linux/spinlock.h>
> #include <linux/zbud.h>
>
> +/*********************************
> +* statistics
> +**********************************/
> +
> +/* zbud pages in all pools */
> +static u64 total_zbud_pages;
> +
> /*****************
> * Structures
> *****************/
> @@ -142,10 +149,28 @@ static struct zbud_header *init_zbud_page(struct page *page)
> return zhdr;
> }
>
> +static struct page *alloc_zbud_page(struct zbud_pool *pool, gfp_t gfp)
> +{
> + struct page *page;
> +
> + page = alloc_page(gfp);
> +
> + if (page) {
> + pool->pages_nr++;
> + total_zbud_pages++;

Who protect race?

> + }
> +
> + return page;
> +}
> +
> +
> /* Resets the struct page fields and frees the page */
> -static void free_zbud_page(struct zbud_header *zhdr)
> +static void free_zbud_page(struct zbud_pool *pool, struct zbud_header *zhdr)
> {
> __free_page(virt_to_page(zhdr));
> +
> + pool->pages_nr--;
> + total_zbud_pages--;
> }
>
> /*
> @@ -279,11 +304,10 @@ int zbud_alloc(struct zbud_pool *pool, int size, gfp_t gfp,
>
> /* Couldn't find unbuddied zbud page, create new one */
> spin_unlock(&pool->lock);
> - page = alloc_page(gfp);
> + page = alloc_zbud_page(pool, gfp);
> if (!page)
> return -ENOMEM;
> spin_lock(&pool->lock);
> - pool->pages_nr++;
> zhdr = init_zbud_page(page);
> bud = FIRST;
>
> @@ -349,8 +373,7 @@ void zbud_free(struct zbud_pool *pool, unsigned long handle)
> if (zhdr->first_chunks == 0 && zhdr->last_chunks == 0) {
> /* zbud page is empty, free */
> list_del(&zhdr->lru);
> - free_zbud_page(zhdr);
> - pool->pages_nr--;
> + free_zbud_page(pool, zhdr);
> } else {
> /* Add to unbuddied list */
> freechunks = num_free_chunks(zhdr);
> @@ -447,8 +470,7 @@ next:
> * Both buddies are now free, free the zbud page and
> * return success.
> */
> - free_zbud_page(zhdr);
> - pool->pages_nr--;
> + free_zbud_page(pool, zhdr);
> spin_unlock(&pool->lock);
> return 0;
> } else if (zhdr->first_chunks == 0 ||
> @@ -496,14 +518,12 @@ void zbud_unmap(struct zbud_pool *pool, unsigned long handle)
>
> /**
> * zbud_get_pool_size() - gets the zbud pool size in pages
> - * @pool: pool whose size is being queried
> *
> - * Returns: size in pages of the given pool. The pool lock need not be
> - * taken to access pages_nr.
> + * Returns: size in pages of all the zbud pools.
> */
> -u64 zbud_get_pool_size(struct zbud_pool *pool)
> +u64 zbud_get_pool_size(void)
> {
> - return pool->pages_nr;
> + return total_zbud_pages;
> }
>
> static int __init init_zbud(void)
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 5a63f78..ef44d9d 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -291,7 +291,7 @@ static void zswap_free_entry(struct zswap_tree *tree,
> zbud_free(tree->pool, entry->handle);
> zswap_entry_cache_free(entry);
> atomic_dec(&zswap_stored_pages);
> - zswap_pool_pages = zbud_get_pool_size(tree->pool);
> + zswap_pool_pages = zbud_get_pool_size();
> }
>
> /* caller must hold the tree lock */
> @@ -716,7 +716,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
>
> /* update stats */
> atomic_inc(&zswap_stored_pages);
> - zswap_pool_pages = zbud_get_pool_size(tree->pool);
> + zswap_pool_pages = zbud_get_pool_size();
>
> return 0;
>
> --
> 1.7.10.4
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>

--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/