[rh7] mm/tswap: fix lockup in tswap_evict_page()

Submitted by Andrey Ryabinin on July 3, 2018, 4:35 p.m.

Details

Message ID 20180703163538.24677-1-aryabinin@virtuozzo.com
State New
Series "mm/tswap: fix lockup in tswap_evict_page()"
Headers show

Commit Message

Andrey Ryabinin July 3, 2018, 4:35 p.m.
Surprisingly after the upstream commit 7c00bafee87c
("mm/swap: free swap slots in batch"), backported
by redhat, SWAP_HAS_CACHE in swap_info->swap_map[offset]
doesn't mean that swap entry has a page in swap cache anymore.

Now we may set SWAP_HAS_CACHE even if we know that page is not cached.
Commit 7c00bafee87c changed swapcache_free() so it will set SWAP_HAS_CACHE:
-       p->swap_map[offset] = usage;
+       p->swap_map[offset] = usage ? : SWAP_HAS_CACHE;

And it will be cleared only by swapcache_free_entries(), but swapcache_free_entries()
call may be delayed indefinitely, until the 'swp_slots' per-cpu cache
is full.

It's not that obvious, how to fix this mess. For now, I think, the safest
course of action would be a workaround in the tswap, similar to what we
do in __read_swap_cache_async() - bail out on zero swap_count. Apperently
it works for __read_swap_cache_async(), so it should work for tswap.

https://jira.sw.ru/browse/PSBM-86344
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 mm/tswap.c | 5 +++++
 1 file changed, 5 insertions(+)

Patch hide | download patch | download mbox

diff --git a/mm/tswap.c b/mm/tswap.c
index b7a990e8cd8d..7b02c1142db2 100644
--- a/mm/tswap.c
+++ b/mm/tswap.c
@@ -183,6 +183,11 @@  static int tswap_evict_page(struct page *page)
 		goto out;
 	}
 
+	if (!__swp_swapcount(entry) && swap_slot_cache_enabled) {
+		err = -ENOENT;
+		goto out;
+	}
+
 	err = swapcache_prepare(entry);
 	if (err == -EEXIST) {
 		cond_resched();

Comments

Cyrill Gorcunov July 4, 2018, 7:56 a.m.
On Tue, Jul 03, 2018 at 07:35:38PM +0300, Andrey Ryabinin wrote:
> Surprisingly after the upstream commit 7c00bafee87c
> ("mm/swap: free swap slots in batch"), backported
> by redhat, SWAP_HAS_CACHE in swap_info->swap_map[offset]
> doesn't mean that swap entry has a page in swap cache anymore.
> 
> Now we may set SWAP_HAS_CACHE even if we know that page is not cached.
> Commit 7c00bafee87c changed swapcache_free() so it will set SWAP_HAS_CACHE:
> -       p->swap_map[offset] = usage;
> +       p->swap_map[offset] = usage ? : SWAP_HAS_CACHE;
> 
> And it will be cleared only by swapcache_free_entries(), but swapcache_free_entries()
> call may be delayed indefinitely, until the 'swp_slots' per-cpu cache
> is full.
> 
> It's not that obvious, how to fix this mess. For now, I think, the safest
> course of action would be a workaround in the tswap, similar to what we
> do in __read_swap_cache_async() - bail out on zero swap_count. Apperently
> it works for __read_swap_cache_async(), so it should work for tswap.
> 
> https://jira.sw.ru/browse/PSBM-86344
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Kirill Tkhai July 4, 2018, 7:59 a.m.
On 03.07.2018 19:35, Andrey Ryabinin wrote:
> Surprisingly after the upstream commit 7c00bafee87c
> ("mm/swap: free swap slots in batch"), backported
> by redhat, SWAP_HAS_CACHE in swap_info->swap_map[offset]
> doesn't mean that swap entry has a page in swap cache anymore.
> 
> Now we may set SWAP_HAS_CACHE even if we know that page is not cached.
> Commit 7c00bafee87c changed swapcache_free() so it will set SWAP_HAS_CACHE:
> -       p->swap_map[offset] = usage;
> +       p->swap_map[offset] = usage ? : SWAP_HAS_CACHE;
> 
> And it will be cleared only by swapcache_free_entries(), but swapcache_free_entries()
> call may be delayed indefinitely, until the 'swp_slots' per-cpu cache
> is full.
> 
> It's not that obvious, how to fix this mess. For now, I think, the safest
> course of action would be a workaround in the tswap, similar to what we
> do in __read_swap_cache_async() - bail out on zero swap_count. Apperently
> it works for __read_swap_cache_async(), so it should work for tswap.
> 
> https://jira.sw.ru/browse/PSBM-86344
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>

> ---
>  mm/tswap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/mm/tswap.c b/mm/tswap.c
> index b7a990e8cd8d..7b02c1142db2 100644
> --- a/mm/tswap.c
> +++ b/mm/tswap.c
> @@ -183,6 +183,11 @@ static int tswap_evict_page(struct page *page)
>  		goto out;
>  	}
>  
> +	if (!__swp_swapcount(entry) && swap_slot_cache_enabled) {
> +		err = -ENOENT;
> +		goto out;
> +	}
> +
>  	err = swapcache_prepare(entry);
>  	if (err == -EEXIST) {
>  		cond_resched();
>