Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...

[Minchan Kim <minchan@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

On Fri, Jul 27, 2018 at 02:13:57PM +0200, Tino Lehnig wrote:
> On 07/27/2018 02:05 PM, Minchan Kim wrote:
> > And bad page is always with writeback enable?
> > 
> > writeback enable means "echo "some dev" > /sys/block/zram0/backing_dev,
> > not just enable CONFIG_ZRAM_WRITEBACK.
> 
> Yes, the bug only appears when backing_dev is set.

Thanks for the clarifiation.

I made a mistake on previous patch.
Could you test this patches?

> 
> -- 
> Kind regards,
> 
> Tino Lehnig

[-- Attachment #2: 0001-zram-remove-BD_CAP_SYNCHRONOUS_IO-with-writeback-fea.patch --]
[-- Type: text/x-diff, Size: 1706 bytes --]

From 77a5fc378dfae733af5a0f0c7ef901668d8c9778 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan@kernel.org>
Date: Fri, 27 Jul 2018 15:15:33 +0900
Subject: [PATCH 1/2] zram: remove BD_CAP_SYNCHRONOUS_IO with writeback feature

If zram supports writeback feature, it's no more syncrhonous
device beause we need asynchronous IO opeation.

Do not pretend to be syncrhonous IO device. It makes system
very sluggish as waiting IO completion from upper layer.
Furthermore, it makes user-after-free problem because swap
think the opearion is done when the IO functions returns so
it could free page by will but in fact, IO is asynchrnous
so driver could access the freed page afterward.
(I will make description more clear at the formal patch).

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 drivers/block/zram/zram_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 7436b2d27fa3..0b6eda1bd77a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -298,7 +298,8 @@ static void reset_bdev(struct zram *zram)
 	zram->backing_dev = NULL;
 	zram->old_block_size = 0;
 	zram->bdev = NULL;
-
+	zram->disk->queue->backing_dev_info->capabilities |=
+				BDI_CAP_SYNCHRONOUS_IO;
 	kvfree(zram->bitmap);
 	zram->bitmap = NULL;
 }
@@ -400,6 +401,8 @@ static ssize_t backing_dev_store(struct device *dev,
 	zram->backing_dev = backing_dev;
 	zram->bitmap = bitmap;
 	zram->nr_pages = nr_pages;
+	zram->disk->queue->backing_dev_info->capabilities &=
+			~BDI_CAP_SYNCHRONOUS_IO;
 	up_write(&zram->init_lock);
 
 	pr_info("setup backing device %s\n", file_name);
-- 
2.18.0.345.g5c9ce644c3-goog


[-- Attachment #3: 0002-swap-free-allocated-page-if-swap_read-fails.patch --]
[-- Type: text/x-diff, Size: 949 bytes --]

From 7c263ac8b9557c60c631c239cc7b863ed762098f Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan@kernel.org>
Date: Fri, 27 Jul 2018 17:17:01 +0900
Subject: [PATCH 2/2] swap: free allocated page if swap_read fails

swap_readpage could fails by -ENOMEM. In this case, we should free
allocated page instantly and bail out. There is no reason to retry.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 7206a634270b..8a5e304ffd91 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2943,7 +2943,8 @@ int do_swap_page(struct vm_fault *vmf)
 				__SetPageSwapBacked(page);
 				set_page_private(page, entry.val);
 				lru_cache_add_anon(page);
-				swap_readpage(page, true);
+				if (swap_readpage(page, true))
+					goto out_page;
 			}
 		} else {
 			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
-- 
2.18.0.345.g5c9ce644c3-goog