2013-09-06 05:17:23

by Weijie Yang

[permalink] [raw]
Subject: [PATCH v2 4/4] mm/zswap: use GFP_NOIO instead of GFP_KERNEL

To avoid zswap store and reclaim functions called recursively,
use GFP_NOIO instead of GFP_KERNEL

Signed-off-by: Weijie Yang <[email protected]>
---
mm/zswap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index cc40e6a..3d05ed8 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -427,7 +427,7 @@ static int zswap_get_swap_cache_page(swp_entry_t entry,
* Get a new page to read into from swap.
*/
if (!new_page) {
- new_page = alloc_page(GFP_KERNEL);
+ new_page = alloc_page(GFP_NOIO);
if (!new_page)
break; /* Out of memory */
}
@@ -435,7 +435,7 @@ static int zswap_get_swap_cache_page(swp_entry_t entry,
/*
* call radix_tree_preload() while we can wait.
*/
- err = radix_tree_preload(GFP_KERNEL);
+ err = radix_tree_preload(GFP_NOIO);
if (err)
break;

@@ -636,7 +636,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
}

/* allocate entry */
- entry = zswap_entry_cache_alloc(GFP_KERNEL);
+ entry = zswap_entry_cache_alloc(GFP_NOIO);
if (!entry) {
zswap_reject_kmemcache_fail++;
ret = -ENOMEM;
--
1.7.10.4


2013-09-06 06:42:15

by Bob Liu

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] mm/zswap: use GFP_NOIO instead of GFP_KERNEL


On 09/06/2013 01:16 PM, Weijie Yang wrote:
> To avoid zswap store and reclaim functions called recursively,
> use GFP_NOIO instead of GFP_KERNEL
>

The reason of using GFP_KERNEL in write back path is we want to try our
best to move those pages from zswap to real swap device.

I think it would be better to keep GFP_KERNEL flag but find some other
ways to skip zswap/zswap_frontswap_store() if zswap write back is in
progress.

What I can think of currently is adding a mutex to zswap, take that
mutex when zswap write back happens and check the mutex in
zswap_frontswap_store().


> Signed-off-by: Weijie Yang <[email protected]>
> ---
> mm/zswap.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index cc40e6a..3d05ed8 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -427,7 +427,7 @@ static int zswap_get_swap_cache_page(swp_entry_t entry,
> * Get a new page to read into from swap.
> */
> if (!new_page) {
> - new_page = alloc_page(GFP_KERNEL);
> + new_page = alloc_page(GFP_NOIO);
> if (!new_page)
> break; /* Out of memory */
> }
> @@ -435,7 +435,7 @@ static int zswap_get_swap_cache_page(swp_entry_t entry,
> /*
> * call radix_tree_preload() while we can wait.
> */
> - err = radix_tree_preload(GFP_KERNEL);
> + err = radix_tree_preload(GFP_NOIO);
> if (err)
> break;
>
> @@ -636,7 +636,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> }
>
> /* allocate entry */
> - entry = zswap_entry_cache_alloc(GFP_KERNEL);
> + entry = zswap_entry_cache_alloc(GFP_NOIO);
> if (!entry) {
> zswap_reject_kmemcache_fail++;
> ret = -ENOMEM;
>

--
Regards,
-Bob

2013-09-09 16:47:59

by Seth Jennings

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] mm/zswap: use GFP_NOIO instead of GFP_KERNEL

On Fri, Sep 06, 2013 at 01:16:45PM +0800, Weijie Yang wrote:
> To avoid zswap store and reclaim functions called recursively,
> use GFP_NOIO instead of GFP_KERNEL
>
> Signed-off-by: Weijie Yang <[email protected]>

I agree with Bob to some degree that GFP_NOIO is a broadsword here.
Ideally, we'd like to continue allowing writeback of dirty file pages
and the like. However, I don't agree that a mutex is the way to do
this.

My first thought was to use the PF_MEMALLOC task flag, but it is already
set for kswapd and any task doing direct reclaim. A new task flag would
work but I'm not sure how acceptable that would be.

In the meantime, this does do away with the possibility of very deep
recursion between the store and reclaim paths.

Acked-by: Seth Jennings <[email protected]>

2013-09-16 09:16:54

by Weijie Yang

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] mm/zswap: use GFP_NOIO instead of GFP_KERNEL

On Tue, Sep 10, 2013 at 12:47 AM, Seth Jennings
<[email protected]> wrote:
> On Fri, Sep 06, 2013 at 01:16:45PM +0800, Weijie Yang wrote:
>> To avoid zswap store and reclaim functions called recursively,
>> use GFP_NOIO instead of GFP_KERNEL
>>
>> Signed-off-by: Weijie Yang <[email protected]>
>
> I agree with Bob to some degree that GFP_NOIO is a broadsword here.
> Ideally, we'd like to continue allowing writeback of dirty file pages
> and the like. However, I don't agree that a mutex is the way to do
> this.
>
> My first thought was to use the PF_MEMALLOC task flag, but it is already
> set for kswapd and any task doing direct reclaim. A new task flag would
> work but I'm not sure how acceptable that would be.

as GFP_NOIO is controversial and not the most appropriate method,
I will keep GFP_KERNEL flag until we find a better way to resolve
this problem.

> In the meantime, this does do away with the possibility of very deep
> recursion between the store and reclaim paths.
>
> Acked-by: Seth Jennings <[email protected]>
>