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
>From 77a5fc378dfae733af5a0f0c7ef901668d8c9778 Mon Sep 17 00:00:00 2001 From: Minchan Kim <minc...@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 <minc...@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
>From 7c263ac8b9557c60c631c239cc7b863ed762098f Mon Sep 17 00:00:00 2001 From: Minchan Kim <minc...@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 <minc...@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