linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] remove page_endio()
       [not found] <CGME20230403132223eucas1p28adb1d36d39add989d46e9f175c07986@eucas1p2.samsung.com>
@ 2023-04-03 13:22 ` Pankaj Raghav
       [not found]   ` <CGME20230403132223eucas1p2a27e8239b8bda4fc16b675a9473fd61f@eucas1p2.samsung.com>
                     ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-03 13:22 UTC (permalink / raw)
  To: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch
  Cc: devel, mcgrof, linux-block, linux-mm, linux-kernel, gost.dev,
	linux-fsdevel, Pankaj Raghav

It was decided to remove the page_endio() as per the previous RFC
discussion[1] of this series and move that functionality into the caller
itself. One of the side benefit of doing that is the callers have been
modified to directly work on folios as page_endio() already worked on
folios.

mpage changes were tested with a simple boot testing. orangefs was
tested by Mike Marshall (No code changes since he tested).
A basic testing was performed for the zram changes with fio and writeback to
a backing device.

Changes since v1:
- Always chain the IO to the parent as it can never be NULL (Minchan)
- Added reviewed and tested by tags

Changes since RFC 2[2]:
- Call bio_put in zram bio end io handler (Still not Acked by hch[3])
- Call folio_set_error in mpage read endio error path (Willy)
- Directly call folio->mapping in mpage write endio error path (Willy)

[1] https://lore.kernel.org/linux-mm/ZBHcl8Pz2ULb4RGD@infradead.org/
[2] https://lore.kernel.org/linux-mm/20230322135013.197076-1-p.raghav@samsung.com/
[3] https://lore.kernel.org/linux-mm/8adb0770-6124-e11f-2551-6582db27ed32@samsung.com/

Pankaj Raghav (5):
  zram: always chain bio to the parent in read_from_bdev_async
  orangefs: use folios in orangefs_readahead
  mpage: split bi_end_io callback for reads and writes
  mpage: use folios in bio end_io handler
  filemap: remove page_endio()

 drivers/block/zram/zram_drv.c | 16 +++------------
 fs/mpage.c                    | 38 +++++++++++++++++++++++++++--------
 fs/orangefs/inode.c           |  9 +++++----
 include/linux/pagemap.h       |  2 --
 mm/filemap.c                  | 30 ---------------------------
 5 files changed, 38 insertions(+), 57 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async
       [not found]   ` <CGME20230403132223eucas1p2a27e8239b8bda4fc16b675a9473fd61f@eucas1p2.samsung.com>
@ 2023-04-03 13:22     ` Pankaj Raghav
  2023-04-03 21:19       ` Minchan Kim
  2023-04-04 15:06       ` Christoph Hellwig
  0 siblings, 2 replies; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-03 13:22 UTC (permalink / raw)
  To: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch
  Cc: devel, mcgrof, linux-block, linux-mm, linux-kernel, gost.dev,
	linux-fsdevel, Pankaj Raghav

zram_bvec_read() is called with the bio set to NULL only in
writeback_store() function. When a writeback is triggered,
zram_bvec_read() is called only if ZRAM_WB flag is not set. That will
result only calling zram_read_from_zspool() in __zram_bvec_read().

rw_page callback used to call read_from_bdev_async with a NULL parent
bio but that has been removed since commit 3222d8c2a7f8
("block: remove ->rw_page").

We can now safely always call bio_chain() as read_from_bdev_async() will
be called with a parent bio set. A WARN_ON_ONCE is added if this function
is called with parent set to NULL.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/block/zram/zram_drv.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 3feadfb96114..d16d0630b178 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -606,15 +606,6 @@ static void free_block_bdev(struct zram *zram, unsigned long blk_idx)
 	atomic64_dec(&zram->stats.bd_count);
 }
 
-static void zram_page_end_io(struct bio *bio)
-{
-	struct page *page = bio_first_page_all(bio);
-
-	page_endio(page, op_is_write(bio_op(bio)),
-			blk_status_to_errno(bio->bi_status));
-	bio_put(bio);
-}
-
 /*
  * Returns 1 if the submission is successful.
  */
@@ -634,11 +625,10 @@ static int read_from_bdev_async(struct zram *zram, struct bio_vec *bvec,
 		return -EIO;
 	}
 
-	if (!parent)
-		bio->bi_end_io = zram_page_end_io;
-	else
-		bio_chain(bio, parent);
+	if (WARN_ON_ONCE(!parent))
+		return -EINVAL;
 
+	bio_chain(bio, parent);
 	submit_bio(bio);
 	return 1;
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] orangefs: use folios in orangefs_readahead
       [not found]   ` <CGME20230403132224eucas1p28f82802bc40d4feb5a30bb59c6536ab3@eucas1p2.samsung.com>
@ 2023-04-03 13:22     ` Pankaj Raghav
  2023-04-04 15:07       ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-03 13:22 UTC (permalink / raw)
  To: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch
  Cc: devel, mcgrof, linux-block, linux-mm, linux-kernel, gost.dev,
	linux-fsdevel, Pankaj Raghav

Convert orangefs_readahead() from using struct page to struct folio.
This conversion removes the call to page_endio() which is soon to be
removed, and simplifies the final page handling.

The page error flags is not required to be set in the error case as
orangefs doesn't depend on them.

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Tested-by: Mike Marshall <hubcap@omnibond.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/orangefs/inode.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index aefdf1d3be7c..9014bbcc8031 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -244,7 +244,7 @@ static void orangefs_readahead(struct readahead_control *rac)
 	struct iov_iter iter;
 	struct inode *inode = rac->mapping->host;
 	struct xarray *i_pages;
-	struct page *page;
+	struct folio *folio;
 	loff_t new_start = readahead_pos(rac);
 	int ret;
 	size_t new_len = 0;
@@ -275,9 +275,10 @@ static void orangefs_readahead(struct readahead_control *rac)
 		ret = 0;
 
 	/* clean up. */
-	while ((page = readahead_page(rac))) {
-		page_endio(page, false, ret);
-		put_page(page);
+	while ((folio = readahead_folio(rac))) {
+		if (!ret)
+			folio_mark_uptodate(folio);
+		folio_unlock(folio);
 	}
 }
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] mpage: split bi_end_io callback for reads and writes
       [not found]   ` <CGME20230403132224eucas1p21fd296fbd4af70220331bb19023f4169@eucas1p2.samsung.com>
@ 2023-04-03 13:22     ` Pankaj Raghav
  2023-04-04 15:09       ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-03 13:22 UTC (permalink / raw)
  To: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch
  Cc: devel, mcgrof, linux-block, linux-mm, linux-kernel, gost.dev,
	linux-fsdevel, Pankaj Raghav

Split the bi_end_io handler for reads and writes similar to other aops.
This is a prep patch before we convert end_io handlers to use folios.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/mpage.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/fs/mpage.c b/fs/mpage.c
index 22b9de5ddd68..3a545bf0f184 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -43,14 +43,28 @@
  * status of that page is hard.  See end_buffer_async_read() for the details.
  * There is no point in duplicating all that complexity.
  */
-static void mpage_end_io(struct bio *bio)
+static void mpage_read_end_io(struct bio *bio)
 {
 	struct bio_vec *bv;
 	struct bvec_iter_all iter_all;
 
 	bio_for_each_segment_all(bv, bio, iter_all) {
 		struct page *page = bv->bv_page;
-		page_endio(page, bio_op(bio),
+		page_endio(page, REQ_OP_READ,
+			   blk_status_to_errno(bio->bi_status));
+	}
+
+	bio_put(bio);
+}
+
+static void mpage_write_end_io(struct bio *bio)
+{
+	struct bio_vec *bv;
+	struct bvec_iter_all iter_all;
+
+	bio_for_each_segment_all(bv, bio, iter_all) {
+		struct page *page = bv->bv_page;
+		page_endio(page, REQ_OP_WRITE,
 			   blk_status_to_errno(bio->bi_status));
 	}
 
@@ -59,7 +73,11 @@ static void mpage_end_io(struct bio *bio)
 
 static struct bio *mpage_bio_submit(struct bio *bio)
 {
-	bio->bi_end_io = mpage_end_io;
+	if (op_is_write(bio_op(bio)))
+		bio->bi_end_io = mpage_write_end_io;
+	else
+		bio->bi_end_io = mpage_read_end_io;
+
 	guard_bio_eod(bio);
 	submit_bio(bio);
 	return NULL;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/5] mpage: use folios in bio end_io handler
       [not found]   ` <CGME20230403132225eucas1p15848db3c850e950b21b339d5861080e1@eucas1p1.samsung.com>
@ 2023-04-03 13:22     ` Pankaj Raghav
  2023-04-04 15:10       ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-03 13:22 UTC (permalink / raw)
  To: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch
  Cc: devel, mcgrof, linux-block, linux-mm, linux-kernel, gost.dev,
	linux-fsdevel, Pankaj Raghav

Use folios in the bio end_io handler. This conversion does the appropriate
handling on the folios in the respective end_io callback and removes the
call to page_endio(), which is soon to be removed.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/mpage.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/mpage.c b/fs/mpage.c
index 3a545bf0f184..6f43b7c9d4de 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -45,13 +45,15 @@
  */
 static void mpage_read_end_io(struct bio *bio)
 {
-	struct bio_vec *bv;
-	struct bvec_iter_all iter_all;
+	struct folio_iter fi;
+	int err = blk_status_to_errno(bio->bi_status);
 
-	bio_for_each_segment_all(bv, bio, iter_all) {
-		struct page *page = bv->bv_page;
-		page_endio(page, REQ_OP_READ,
-			   blk_status_to_errno(bio->bi_status));
+	bio_for_each_folio_all(fi, bio) {
+		if (!err)
+			folio_mark_uptodate(fi.folio);
+		else
+			folio_set_error(fi.folio);
+		folio_unlock(fi.folio);
 	}
 
 	bio_put(bio);
@@ -59,13 +61,15 @@ static void mpage_read_end_io(struct bio *bio)
 
 static void mpage_write_end_io(struct bio *bio)
 {
-	struct bio_vec *bv;
-	struct bvec_iter_all iter_all;
+	struct folio_iter fi;
+	int err = blk_status_to_errno(bio->bi_status);
 
-	bio_for_each_segment_all(bv, bio, iter_all) {
-		struct page *page = bv->bv_page;
-		page_endio(page, REQ_OP_WRITE,
-			   blk_status_to_errno(bio->bi_status));
+	bio_for_each_folio_all(fi, bio) {
+		if (err) {
+			folio_set_error(fi.folio);
+			mapping_set_error(fi.folio->mapping, err);
+		}
+		folio_end_writeback(fi.folio);
 	}
 
 	bio_put(bio);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/5] filemap: remove page_endio()
       [not found]   ` <CGME20230403132226eucas1p182e09f5da7bc0bd284d6a8494cd40903@eucas1p1.samsung.com>
@ 2023-04-03 13:22     ` Pankaj Raghav
  2023-04-04 15:10       ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-03 13:22 UTC (permalink / raw)
  To: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch
  Cc: devel, mcgrof, linux-block, linux-mm, linux-kernel, gost.dev,
	linux-fsdevel, Pankaj Raghav

page_endio() is not used anymore. Remove it.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 include/linux/pagemap.h |  2 --
 mm/filemap.c            | 30 ------------------------------
 2 files changed, 32 deletions(-)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index fdcd595d2294..73ee6ead90dd 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -1076,8 +1076,6 @@ int filemap_migrate_folio(struct address_space *mapping, struct folio *dst,
 #else
 #define filemap_migrate_folio NULL
 #endif
-void page_endio(struct page *page, bool is_write, int err);
-
 void folio_end_private_2(struct folio *folio);
 void folio_wait_private_2(struct folio *folio);
 int folio_wait_private_2_killable(struct folio *folio);
diff --git a/mm/filemap.c b/mm/filemap.c
index 6f3a7e53fccf..a770a207825d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1625,36 +1625,6 @@ void folio_end_writeback(struct folio *folio)
 }
 EXPORT_SYMBOL(folio_end_writeback);
 
-/*
- * After completing I/O on a page, call this routine to update the page
- * flags appropriately
- */
-void page_endio(struct page *page, bool is_write, int err)
-{
-	struct folio *folio = page_folio(page);
-
-	if (!is_write) {
-		if (!err) {
-			folio_mark_uptodate(folio);
-		} else {
-			folio_clear_uptodate(folio);
-			folio_set_error(folio);
-		}
-		folio_unlock(folio);
-	} else {
-		if (err) {
-			struct address_space *mapping;
-
-			folio_set_error(folio);
-			mapping = folio_mapping(folio);
-			if (mapping)
-				mapping_set_error(mapping, err);
-		}
-		folio_end_writeback(folio);
-	}
-}
-EXPORT_SYMBOL_GPL(page_endio);
-
 /**
  * __folio_lock - Get a lock on the folio, assuming we need to sleep to get it.
  * @folio: The folio to lock
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async
  2023-04-03 13:22     ` [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async Pankaj Raghav
@ 2023-04-03 21:19       ` Minchan Kim
  2023-04-04 15:06       ` Christoph Hellwig
  1 sibling, 0 replies; 15+ messages in thread
From: Minchan Kim @ 2023-04-03 21:19 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, martin, hubcap, brauner, viro, senozhatsky, akpm, willy,
	hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

On Mon, Apr 03, 2023 at 03:22:17PM +0200, Pankaj Raghav wrote:
> zram_bvec_read() is called with the bio set to NULL only in
> writeback_store() function. When a writeback is triggered,
> zram_bvec_read() is called only if ZRAM_WB flag is not set. That will
> result only calling zram_read_from_zspool() in __zram_bvec_read().
> 
> rw_page callback used to call read_from_bdev_async with a NULL parent
> bio but that has been removed since commit 3222d8c2a7f8
> ("block: remove ->rw_page").
> 
> We can now safely always call bio_chain() as read_from_bdev_async() will
> be called with a parent bio set. A WARN_ON_ONCE is added if this function
> is called with parent set to NULL.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
Acked-by: Minchan Kim <minchan@kernel.org>

Thanks.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async
  2023-04-03 13:22     ` [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async Pankaj Raghav
  2023-04-03 21:19       ` Minchan Kim
@ 2023-04-04 15:06       ` Christoph Hellwig
  2023-04-04 19:31         ` Andrew Morton
  2023-04-11  7:34         ` Pankaj Raghav
  1 sibling, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-04-04 15:06 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

On Mon, Apr 03, 2023 at 03:22:17PM +0200, Pankaj Raghav wrote:
> zram_bvec_read() is called with the bio set to NULL only in
> writeback_store() function. When a writeback is triggered,
> zram_bvec_read() is called only if ZRAM_WB flag is not set. That will
> result only calling zram_read_from_zspool() in __zram_bvec_read().
> 
> rw_page callback used to call read_from_bdev_async with a NULL parent
> bio but that has been removed since commit 3222d8c2a7f8
> ("block: remove ->rw_page").
> 
> We can now safely always call bio_chain() as read_from_bdev_async() will
> be called with a parent bio set. A WARN_ON_ONCE is added if this function
> is called with parent set to NULL.

I'm pretty sure this is wrong.  I've now sent a series to untangle
and fix up the zram I/O path, which should address the underlying
issue here.

It will obviously conflict with this patch, so maybe the best thing is
to get the other page_endio removals into their respective maintainer
trees, and then just do the final removal of the unused function after
-rc1.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/5] orangefs: use folios in orangefs_readahead
  2023-04-03 13:22     ` [PATCH v2 2/5] orangefs: use folios in orangefs_readahead Pankaj Raghav
@ 2023-04-04 15:07       ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-04-04 15:07 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/5] mpage: split bi_end_io callback for reads and writes
  2023-04-03 13:22     ` [PATCH v2 3/5] mpage: split bi_end_io callback for reads and writes Pankaj Raghav
@ 2023-04-04 15:09       ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-04-04 15:09 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

>  		struct page *page = bv->bv_page;
> -		page_endio(page, bio_op(bio),
> +		page_endio(page, REQ_OP_READ,
> +			   blk_status_to_errno(bio->bi_status));

Nit: I think we can do without the page local variable here.

> +	bio_for_each_segment_all(bv, bio, iter_all) {
> +		struct page *page = bv->bv_page;
> +		page_endio(page, REQ_OP_WRITE,
>  			   blk_status_to_errno(bio->bi_status));

Same here.

>  	}
>  
> @@ -59,7 +73,11 @@ static void mpage_end_io(struct bio *bio)
>  
>  static struct bio *mpage_bio_submit(struct bio *bio)
>  {
> -	bio->bi_end_io = mpage_end_io;
> +	if (op_is_write(bio_op(bio)))
> +		bio->bi_end_io = mpage_write_end_io;
> +	else
> +		bio->bi_end_io = mpage_read_end_io;
> +

I'd also split mpage_bio_submit as all allers are clearly either
for reads or writes.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/5] mpage: use folios in bio end_io handler
  2023-04-03 13:22     ` [PATCH v2 4/5] mpage: use folios in bio end_io handler Pankaj Raghav
@ 2023-04-04 15:10       ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-04-04 15:10 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

> +	bio_for_each_folio_all(fi, bio) {
> +		if (!err)
> +			folio_mark_uptodate(fi.folio);
> +		else
> +			folio_set_error(fi.folio);
> +		folio_unlock(fi.folio);

Super nitpicky, but I'd avoid inverted conditions unless there is a
very clear benefit.  I.e. I'd rather write this as:

		if (err)
			folio_set_error(fi.folio);
		else
			folio_mark_uptodate(fi.folio);

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/5] filemap: remove page_endio()
  2023-04-03 13:22     ` [PATCH v2 5/5] filemap: remove page_endio() Pankaj Raghav
@ 2023-04-04 15:10       ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-04-04 15:10 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async
  2023-04-04 15:06       ` Christoph Hellwig
@ 2023-04-04 19:31         ` Andrew Morton
  2023-04-05  6:07           ` Christoph Hellwig
  2023-04-11  7:34         ` Pankaj Raghav
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2023-04-04 19:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Pankaj Raghav, axboe, minchan, martin, hubcap, brauner, viro,
	senozhatsky, willy, hch, devel, mcgrof, linux-block, linux-mm,
	linux-kernel, gost.dev, linux-fsdevel

On Tue, 4 Apr 2023 08:06:55 -0700 Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, Apr 03, 2023 at 03:22:17PM +0200, Pankaj Raghav wrote:
> > zram_bvec_read() is called with the bio set to NULL only in
> > writeback_store() function. When a writeback is triggered,
> > zram_bvec_read() is called only if ZRAM_WB flag is not set. That will
> > result only calling zram_read_from_zspool() in __zram_bvec_read().
> > 
> > rw_page callback used to call read_from_bdev_async with a NULL parent
> > bio but that has been removed since commit 3222d8c2a7f8
> > ("block: remove ->rw_page").
> > 
> > We can now safely always call bio_chain() as read_from_bdev_async() will
> > be called with a parent bio set. A WARN_ON_ONCE is added if this function
> > is called with parent set to NULL.
> 
> I'm pretty sure this is wrong.

Thanks, I'll drop this v2 series.

>  I've now sent a series to untangle
> and fix up the zram I/O path, which should address the underlying
> issue here.

I can't find that series.

> It will obviously conflict with this patch, so maybe the best thing is
> to get the other page_endio removals into their respective maintainer
> trees, and then just do the final removal of the unused function after
> -rc1.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async
  2023-04-04 19:31         ` Andrew Morton
@ 2023-04-05  6:07           ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-04-05  6:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Pankaj Raghav, axboe, minchan, martin, hubcap,
	brauner, viro, senozhatsky, willy, hch, devel, mcgrof,
	linux-block, linux-mm, linux-kernel, gost.dev, linux-fsdevel

On Tue, Apr 04, 2023 at 12:31:31PM -0700, Andrew Morton wrote:
> >  I've now sent a series to untangle
> > and fix up the zram I/O path, which should address the underlying
> > issue here.
> 
> I can't find that series.

https://lore.kernel.org/linux-block/20230404150536.2142108-1-hch@lst.de/T/#t


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async
  2023-04-04 15:06       ` Christoph Hellwig
  2023-04-04 19:31         ` Andrew Morton
@ 2023-04-11  7:34         ` Pankaj Raghav
  1 sibling, 0 replies; 15+ messages in thread
From: Pankaj Raghav @ 2023-04-11  7:34 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, minchan, martin, hubcap, brauner, viro, senozhatsky, akpm,
	willy, hch, devel, mcgrof, linux-block, linux-mm, linux-kernel,
	gost.dev, linux-fsdevel

> 
> It will obviously conflict with this patch, so maybe the best thing is
> to get the other page_endio removals into their respective maintainer
> trees, and then just do the final removal of the unused function after
> -rc1.

Alright, I will drop the last patch that removes the page_endio function, and
send it after rc1. I will make the other changes as suggested by you.


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-04-11  7:34 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20230403132223eucas1p28adb1d36d39add989d46e9f175c07986@eucas1p2.samsung.com>
2023-04-03 13:22 ` [PATCH v2 0/5] remove page_endio() Pankaj Raghav
     [not found]   ` <CGME20230403132223eucas1p2a27e8239b8bda4fc16b675a9473fd61f@eucas1p2.samsung.com>
2023-04-03 13:22     ` [PATCH v2 1/5] zram: always chain bio to the parent in read_from_bdev_async Pankaj Raghav
2023-04-03 21:19       ` Minchan Kim
2023-04-04 15:06       ` Christoph Hellwig
2023-04-04 19:31         ` Andrew Morton
2023-04-05  6:07           ` Christoph Hellwig
2023-04-11  7:34         ` Pankaj Raghav
     [not found]   ` <CGME20230403132224eucas1p28f82802bc40d4feb5a30bb59c6536ab3@eucas1p2.samsung.com>
2023-04-03 13:22     ` [PATCH v2 2/5] orangefs: use folios in orangefs_readahead Pankaj Raghav
2023-04-04 15:07       ` Christoph Hellwig
     [not found]   ` <CGME20230403132224eucas1p21fd296fbd4af70220331bb19023f4169@eucas1p2.samsung.com>
2023-04-03 13:22     ` [PATCH v2 3/5] mpage: split bi_end_io callback for reads and writes Pankaj Raghav
2023-04-04 15:09       ` Christoph Hellwig
     [not found]   ` <CGME20230403132225eucas1p15848db3c850e950b21b339d5861080e1@eucas1p1.samsung.com>
2023-04-03 13:22     ` [PATCH v2 4/5] mpage: use folios in bio end_io handler Pankaj Raghav
2023-04-04 15:10       ` Christoph Hellwig
     [not found]   ` <CGME20230403132226eucas1p182e09f5da7bc0bd284d6a8494cd40903@eucas1p1.samsung.com>
2023-04-03 13:22     ` [PATCH v2 5/5] filemap: remove page_endio() Pankaj Raghav
2023-04-04 15:10       ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).