All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Nitin Gupta <ngupta@vflare.org>,
	Jerome Marchand <jmarchan@redhat.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Minchan Kim <minchan@kernel.org>,
	Matthew Wilcox <matthew.r.wilcox@intel.com>,
	Karam Lee <karam.lee@lge.com>, Dave Chinner <david@fromorbit.com>
Subject: [PATCH] zram: rely on the bi_end_io for zram_rw_page fails
Date: Fri, 14 Nov 2014 09:49:07 +0900	[thread overview]
Message-ID: <1415926147-9023-1-git-send-email-minchan@kernel.org> (raw)

When I tested zram, I found processes got segfaulted.
The reason was zram_rw_page doesn't make the page dirty
again when swap write failed, and even it doesn't return
error by [1].

If error by zram internal happens, zram_rw_page should return
non-zero without calling page_endio.
It causes resubmit the IO with bio so that it ends up calling
bio->bi_end_io.

The reason is zram could be used for a block device for FS and
swap, which they uses different bio complete callback, which
works differently. So, we should rely on the bio I/O complete
handler rather than zram_bvec_rw itself in case of I/O fail.

This patch fixes the segfault issue as well one [1]'s
mentioned

[1] zram: make rw_page opeartion return 0

Cc: Matthew Wilcox <matthew.r.wilcox@intel.com>
Cc: Karam Lee <karam.lee@lge.com>
Cc: Dave Chinner <david@fromorbit.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 drivers/block/zram/zram_drv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 4b4f4dbc3cfd..0e0650feab2a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -978,12 +978,10 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
 out_unlock:
 	up_read(&zram->init_lock);
 out:
-	page_endio(page, rw, err);
+	if (unlikely(err))
+		return err;
 
-	/*
-	 * Return 0 prevents I/O fallback trial caused by rw_page fail
-	 * and upper layer can handle this IO error via page error.
-	 */
+	page_endio(page, rw, 0);
 	return 0;
 }
 
-- 
2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Nitin Gupta <ngupta@vflare.org>,
	Jerome Marchand <jmarchan@redhat.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Minchan Kim <minchan@kernel.org>,
	Matthew Wilcox <matthew.r.wilcox@intel.com>,
	Karam Lee <karam.lee@lge.com>, Dave Chinner <david@fromorbit.com>
Subject: [PATCH] zram: rely on the bi_end_io for zram_rw_page fails
Date: Fri, 14 Nov 2014 09:49:07 +0900	[thread overview]
Message-ID: <1415926147-9023-1-git-send-email-minchan@kernel.org> (raw)

When I tested zram, I found processes got segfaulted.
The reason was zram_rw_page doesn't make the page dirty
again when swap write failed, and even it doesn't return
error by [1].

If error by zram internal happens, zram_rw_page should return
non-zero without calling page_endio.
It causes resubmit the IO with bio so that it ends up calling
bio->bi_end_io.

The reason is zram could be used for a block device for FS and
swap, which they uses different bio complete callback, which
works differently. So, we should rely on the bio I/O complete
handler rather than zram_bvec_rw itself in case of I/O fail.

This patch fixes the segfault issue as well one [1]'s
mentioned

[1] zram: make rw_page opeartion return 0

Cc: Matthew Wilcox <matthew.r.wilcox@intel.com>
Cc: Karam Lee <karam.lee@lge.com>
Cc: Dave Chinner <david@fromorbit.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 drivers/block/zram/zram_drv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 4b4f4dbc3cfd..0e0650feab2a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -978,12 +978,10 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
 out_unlock:
 	up_read(&zram->init_lock);
 out:
-	page_endio(page, rw, err);
+	if (unlikely(err))
+		return err;
 
-	/*
-	 * Return 0 prevents I/O fallback trial caused by rw_page fail
-	 * and upper layer can handle this IO error via page error.
-	 */
+	page_endio(page, rw, 0);
 	return 0;
 }
 
-- 
2.0.0


             reply	other threads:[~2014-11-14  0:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-14  0:49 Minchan Kim [this message]
2014-11-14  0:49 ` [PATCH] zram: rely on the bi_end_io for zram_rw_page fails Minchan Kim
2014-11-15  9:19 ` Sergey Senozhatsky
2014-11-15  9:19   ` Sergey Senozhatsky
2014-11-18 23:23 ` Andrew Morton
2014-11-18 23:23   ` Andrew Morton
2014-11-18 23:52   ` Minchan Kim
2014-11-18 23:52     ` Minchan Kim
2014-11-19 21:15     ` Andrew Morton
2014-11-19 21:15       ` Andrew Morton
2014-11-19 23:32       ` Minchan Kim
2014-11-19 23:32         ` Minchan Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1415926147-9023-1-git-send-email-minchan@kernel.org \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=jmarchan@redhat.com \
    --cc=karam.lee@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=matthew.r.wilcox@intel.com \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.