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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Jan 2014 11:58:46 +0800
From:	Bob Liu <lliubbo@...il.com>
To:	Cai Liu <liucai.lfn@...il.com>
Cc:	Minchan Kim <minchan@...nel.org>,
	Seth Jennings <sjenning@...ux.vnet.ibm.com>,
	Bob Liu <bob.liu@...cle.com>, Cai Liu <cai.liu@...sung.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux-MM <linux-mm@...ck.org>,
	Linux-Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] mm/zswap: Check all pool pages instead of one pool pages

On Tue, Jan 21, 2014 at 11:07 AM, Cai Liu <liucai.lfn@...il.com> wrote:
> Thanks for your review.
>
> 2014/1/21 Minchan Kim <minchan@...nel.org>:
>> 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@...sung.com>
>>> ---
>>>  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?
>
> Yes, here the pool->pages_nr and also the total_zbud_pages are not protected.
> I will re-do it.
>
> I will change *total_zbud_pages* to atomic type.

And how about just add total_zbud_pages++ and leave pool->pages_nr in
its original place which already protected by pool->lock?

> For *pool->pages_nr*, one way is to use pool->lock to protect. But I
> think it is too heavy.
> So does it ok to change pages_nr to atomic type too?
>

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