linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 00/20] bio: check return values of bio_add_page
@ 2023-05-31 11:50 Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 01/20] swap: use __bio_add_page to add page to bio Johannes Thumshirn
                   ` (20 more replies)
  0 siblings, 21 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn

We have two functions for adding a page to a bio, __bio_add_page() which is
used to add a single page to a freshly created bio and bio_add_page() which is
used to add a page to an existing bio.

While __bio_add_page() is expected to succeed, bio_add_page() can fail.

This series converts the callers of bio_add_page() which can easily use
__bio_add_page() to using it and checks the return of bio_add_page() for
callers that don't work on a freshly created bio.

Lastly it marks bio_add_page() as __must_check so we don't have to go again
and audit all callers.

Changes to v6:
- Incorporated comments from Mike and Christoph
- Collected Reviewed-bys

Changes to v5:
- Rebased onto latest Linus' master
- Removed now superfluous BUG_ON() in fs/buffer.c (Gou)
- Removed dead cleanup code in dm-crypt.c (Mikulas)

Changes to v4:
- Rebased onto latest Linus' master
- Dropped already merged patches
- Added Sergey's Reviewed-by

Changes to v3:
- Added __bio_add_folio and use it in iomap (Willy)
- Mark bio_add_folio must check (Willy)
- s/GFS/GFS2/ (Andreas)

Changes to v2:
- Removed 'wont fail' comments pointed out by Song

Changes to v1:
- Removed pointless comment pointed out by Willy
- Changed commit messages pointed out by Damien
- Colledted Damien's Reviews and Acks

Johannes Thumshirn (20):
  swap: use __bio_add_page to add page to bio
  drbd: use __bio_add_page to add page to bio
  dm: dm-zoned: use __bio_add_page for adding single metadata page
  fs: buffer: use __bio_add_page to add single page to bio
  md: use __bio_add_page to add single page
  md: raid5-log: use __bio_add_page to add single page
  md: raid5: use __bio_add_page to add single page to new bio
  jfs: logmgr: use __bio_add_page to add single page to bio
  gfs2: use __bio_add_page for adding single page to bio
  zonefs: use __bio_add_page for adding single page to bio
  zram: use __bio_add_page for adding single page to bio
  floppy: use __bio_add_page for adding single page to bio
  md: check for failure when adding pages in alloc_behind_master_bio
  md: raid1: use __bio_add_page for adding single page to bio
  md: raid1: check if adding pages to resync bio fails
  dm-crypt: use __bio_add_page to add single page to clone bio
  block: mark bio_add_page as __must_check
  block: add bio_add_folio_nofail
  fs: iomap: use bio_add_folio_nofail where possible
  block: mark bio_add_folio as __must_check

 block/bio.c                      |  8 ++++++++
 drivers/block/drbd/drbd_bitmap.c |  4 +---
 drivers/block/floppy.c           |  2 +-
 drivers/block/zram/zram_drv.c    |  2 +-
 drivers/md/dm-crypt.c            |  3 +--
 drivers/md/dm-zoned-metadata.c   |  6 +++---
 drivers/md/md.c                  |  4 ++--
 drivers/md/raid1-10.c            | 11 ++++++-----
 drivers/md/raid1.c               |  7 +++++--
 drivers/md/raid10.c              | 20 ++++++++++----------
 drivers/md/raid5-cache.c         |  2 +-
 drivers/md/raid5-ppl.c           |  4 ++--
 fs/buffer.c                      |  3 +--
 fs/gfs2/ops_fstype.c             |  2 +-
 fs/iomap/buffered-io.c           |  6 +++---
 fs/jfs/jfs_logmgr.c              |  4 ++--
 fs/zonefs/super.c                |  2 +-
 include/linux/bio.h              |  8 ++++++--
 mm/page_io.c                     |  8 ++++----
 19 files changed, 59 insertions(+), 47 deletions(-)


base-commit: 48b1320a674e1ff5de2fad8606bee38f724594dc
-- 
2.40.1


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

* [PATCH v7 01/20] swap: use __bio_add_page to add page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 02/20] drbd: " Johannes Thumshirn
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The swap code only adds a single page to a newly created bio. So use
__bio_add_page() to add the page which is guaranteed to succeed in this
case.

This brings us closer to marking bio_add_page() as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 mm/page_io.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/page_io.c b/mm/page_io.c
index 87b682d18850..684cd3c7b59b 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -338,7 +338,7 @@ static void swap_writepage_bdev_sync(struct page *page,
 	bio_init(&bio, sis->bdev, &bv, 1,
 		 REQ_OP_WRITE | REQ_SWAP | wbc_to_write_flags(wbc));
 	bio.bi_iter.bi_sector = swap_page_sector(page);
-	bio_add_page(&bio, page, thp_size(page), 0);
+	__bio_add_page(&bio, page, thp_size(page), 0);
 
 	bio_associate_blkg_from_page(&bio, page);
 	count_swpout_vm_event(page);
@@ -360,7 +360,7 @@ static void swap_writepage_bdev_async(struct page *page,
 			GFP_NOIO);
 	bio->bi_iter.bi_sector = swap_page_sector(page);
 	bio->bi_end_io = end_swap_bio_write;
-	bio_add_page(bio, page, thp_size(page), 0);
+	__bio_add_page(bio, page, thp_size(page), 0);
 
 	bio_associate_blkg_from_page(bio, page);
 	count_swpout_vm_event(page);
@@ -468,7 +468,7 @@ static void swap_readpage_bdev_sync(struct page *page,
 
 	bio_init(&bio, sis->bdev, &bv, 1, REQ_OP_READ);
 	bio.bi_iter.bi_sector = swap_page_sector(page);
-	bio_add_page(&bio, page, thp_size(page), 0);
+	__bio_add_page(&bio, page, thp_size(page), 0);
 	/*
 	 * Keep this task valid during swap readpage because the oom killer may
 	 * attempt to access it in the page fault retry time check.
@@ -488,7 +488,7 @@ static void swap_readpage_bdev_async(struct page *page,
 	bio = bio_alloc(sis->bdev, 1, REQ_OP_READ, GFP_KERNEL);
 	bio->bi_iter.bi_sector = swap_page_sector(page);
 	bio->bi_end_io = end_swap_bio_read;
-	bio_add_page(bio, page, thp_size(page), 0);
+	__bio_add_page(bio, page, thp_size(page), 0);
 	count_vm_event(PSWPIN);
 	submit_bio(bio);
 }
-- 
2.40.1


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

* [PATCH v7 02/20] drbd: use __bio_add_page to add page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 01/20] swap: use __bio_add_page to add page to bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 03/20] dm: dm-zoned: use __bio_add_page for adding single metadata page Johannes Thumshirn
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The drbd code only adds a single page to a newly created bio. So use
__bio_add_page() to add the page which is guaranteed to succeed in this
case.

This brings us closer to marking bio_add_page() as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/block/drbd/drbd_bitmap.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 6ac8c54b44c7..85ca000a0564 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -1043,9 +1043,7 @@ static void bm_page_io_async(struct drbd_bm_aio_ctx *ctx, int page_nr) __must_ho
 	bio = bio_alloc_bioset(device->ldev->md_bdev, 1, op, GFP_NOIO,
 			&drbd_md_io_bio_set);
 	bio->bi_iter.bi_sector = on_disk_sector;
-	/* bio_add_page of a single page to an empty bio will always succeed,
-	 * according to api.  Do we want to assert that? */
-	bio_add_page(bio, page, len, 0);
+	__bio_add_page(bio, page, len, 0);
 	bio->bi_private = ctx;
 	bio->bi_end_io = drbd_bm_endio;
 
-- 
2.40.1


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

* [PATCH v7 03/20] dm: dm-zoned: use __bio_add_page for adding single metadata page
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 01/20] swap: use __bio_add_page to add page to bio Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 02/20] drbd: " Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 04/20] fs: buffer: use __bio_add_page to add single page to bio Johannes Thumshirn
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

dm-zoned uses bio_add_page() for adding a single page to a freshly created
metadata bio.

Use __bio_add_page() instead as adding a single page to a new bio is
always guaranteed to succeed.

This brings us a step closer to marking bio_add_page() __must_check

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/dm-zoned-metadata.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
index 8f0896a6990b..9d3cca8e3dc9 100644
--- a/drivers/md/dm-zoned-metadata.c
+++ b/drivers/md/dm-zoned-metadata.c
@@ -577,7 +577,7 @@ static struct dmz_mblock *dmz_get_mblock_slow(struct dmz_metadata *zmd,
 	bio->bi_iter.bi_sector = dmz_blk2sect(block);
 	bio->bi_private = mblk;
 	bio->bi_end_io = dmz_mblock_bio_end_io;
-	bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0);
+	__bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0);
 	submit_bio(bio);
 
 	return mblk;
@@ -728,7 +728,7 @@ static int dmz_write_mblock(struct dmz_metadata *zmd, struct dmz_mblock *mblk,
 	bio->bi_iter.bi_sector = dmz_blk2sect(block);
 	bio->bi_private = mblk;
 	bio->bi_end_io = dmz_mblock_bio_end_io;
-	bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0);
+	__bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0);
 	submit_bio(bio);
 
 	return 0;
@@ -752,7 +752,7 @@ static int dmz_rdwr_block(struct dmz_dev *dev, enum req_op op,
 	bio = bio_alloc(dev->bdev, 1, op | REQ_SYNC | REQ_META | REQ_PRIO,
 			GFP_NOIO);
 	bio->bi_iter.bi_sector = dmz_blk2sect(block);
-	bio_add_page(bio, page, DMZ_BLOCK_SIZE, 0);
+	__bio_add_page(bio, page, DMZ_BLOCK_SIZE, 0);
 	ret = submit_bio_wait(bio);
 	bio_put(bio);
 
-- 
2.40.1


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

* [PATCH v7 04/20] fs: buffer: use __bio_add_page to add single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (2 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 03/20] dm: dm-zoned: use __bio_add_page for adding single metadata page Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 05/20] md: use __bio_add_page to add single page Johannes Thumshirn
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Gou Hao, Damien Le Moal

The buffer_head submission code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Gou Hao <gouhao@uniontech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/buffer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index a7fc561758b1..63da30ce946a 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2760,8 +2760,7 @@ static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh,
 
 	bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 
-	bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh));
-	BUG_ON(bio->bi_iter.bi_size != bh->b_size);
+	__bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh));
 
 	bio->bi_end_io = end_bio_bh_io_sync;
 	bio->bi_private = bh;
-- 
2.40.1


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

* [PATCH v7 05/20] md: use __bio_add_page to add single page
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (3 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 04/20] fs: buffer: use __bio_add_page to add single page to bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 06/20] md: raid5-log: " Johannes Thumshirn
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The md-raid superblock writing code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Signed-of_-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Song Liu <song@kernel.org>
---
 drivers/md/md.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8e344b4b3444..6a559a7e89c0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -938,7 +938,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
 	atomic_inc(&rdev->nr_pending);
 
 	bio->bi_iter.bi_sector = sector;
-	bio_add_page(bio, page, size, 0);
+	__bio_add_page(bio, page, size, 0);
 	bio->bi_private = rdev;
 	bio->bi_end_io = super_written;
 
@@ -979,7 +979,7 @@ int sync_page_io(struct md_rdev *rdev, sector_t sector, int size,
 		bio.bi_iter.bi_sector = sector + rdev->new_data_offset;
 	else
 		bio.bi_iter.bi_sector = sector + rdev->data_offset;
-	bio_add_page(&bio, page, size, 0);
+	__bio_add_page(&bio, page, size, 0);
 
 	submit_bio_wait(&bio);
 
-- 
2.40.1


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

* [PATCH v7 06/20] md: raid5-log: use __bio_add_page to add single page
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (4 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 05/20] md: use __bio_add_page to add single page Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 07/20] md: raid5: use __bio_add_page to add single page to new bio Johannes Thumshirn
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The raid5 log metadata submission code uses bio_add_page() to add a page
to a newly created bio. bio_add_page() can fail, but the return value is
never checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/raid5-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 46182b955aef..852b265c5db4 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -792,7 +792,7 @@ static struct r5l_io_unit *r5l_new_meta(struct r5l_log *log)
 	io->current_bio = r5l_bio_alloc(log);
 	io->current_bio->bi_end_io = r5l_log_endio;
 	io->current_bio->bi_private = io;
-	bio_add_page(io->current_bio, io->meta_page, PAGE_SIZE, 0);
+	__bio_add_page(io->current_bio, io->meta_page, PAGE_SIZE, 0);
 
 	r5_reserve_log_entry(log, io);
 
-- 
2.40.1


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

* [PATCH v7 07/20] md: raid5: use __bio_add_page to add single page to new bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (5 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 06/20] md: raid5-log: " Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 08/20] jfs: logmgr: use __bio_add_page to add single page to bio Johannes Thumshirn
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The raid5-ppl submission code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked. For adding consecutive pages, the return is actually checked and
a new bio is allocated if adding the page fails.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/raid5-ppl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index e495939bb3e0..eaea57aee602 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -465,7 +465,7 @@ static void ppl_submit_iounit(struct ppl_io_unit *io)
 
 	bio->bi_end_io = ppl_log_endio;
 	bio->bi_iter.bi_sector = log->next_io_sector;
-	bio_add_page(bio, io->header_page, PAGE_SIZE, 0);
+	__bio_add_page(bio, io->header_page, PAGE_SIZE, 0);
 
 	pr_debug("%s: log->current_io_sector: %llu\n", __func__,
 	    (unsigned long long)log->next_io_sector);
@@ -496,7 +496,7 @@ static void ppl_submit_iounit(struct ppl_io_unit *io)
 					       prev->bi_opf, GFP_NOIO,
 					       &ppl_conf->bs);
 			bio->bi_iter.bi_sector = bio_end_sector(prev);
-			bio_add_page(bio, sh->ppl_page, PAGE_SIZE, 0);
+			__bio_add_page(bio, sh->ppl_page, PAGE_SIZE, 0);
 
 			bio_chain(bio, prev);
 			ppl_submit_iounit_bio(io, prev);
-- 
2.40.1


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

* [PATCH v7 08/20] jfs: logmgr: use __bio_add_page to add single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (6 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 07/20] md: raid5: use __bio_add_page to add single page to new bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 09/20] gfs2: use __bio_add_page for adding " Johannes Thumshirn
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal, Dave Kleikamp

The JFS IO code uses bio_add_page() to add a page to a newly created bio.
bio_add_page() can fail, but the return value is never checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Dave Kleikamp <dave.kleikamp@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/jfs/jfs_logmgr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index 695415cbfe98..15c645827dec 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1974,7 +1974,7 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
 
 	bio = bio_alloc(log->bdev, 1, REQ_OP_READ, GFP_NOFS);
 	bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9);
-	bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
+	__bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
 	BUG_ON(bio->bi_iter.bi_size != LOGPSIZE);
 
 	bio->bi_end_io = lbmIODone;
@@ -2115,7 +2115,7 @@ static void lbmStartIO(struct lbuf * bp)
 
 	bio = bio_alloc(log->bdev, 1, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
 	bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9);
-	bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
+	__bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
 	BUG_ON(bio->bi_iter.bi_size != LOGPSIZE);
 
 	bio->bi_end_io = lbmIODone;
-- 
2.40.1


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

* [PATCH v7 09/20] gfs2: use __bio_add_page for adding single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (7 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 08/20] jfs: logmgr: use __bio_add_page to add single page to bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 10/20] zonefs: " Johannes Thumshirn
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The GFS2 superblock reading code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/gfs2/ops_fstype.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 9af9ddb61ca0..cd962985b058 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -254,7 +254,7 @@ static int gfs2_read_super(struct gfs2_sbd *sdp, sector_t sector, int silent)
 
 	bio = bio_alloc(sb->s_bdev, 1, REQ_OP_READ | REQ_META, GFP_NOFS);
 	bio->bi_iter.bi_sector = sector * (sb->s_blocksize >> 9);
-	bio_add_page(bio, page, PAGE_SIZE, 0);
+	__bio_add_page(bio, page, PAGE_SIZE, 0);
 
 	bio->bi_end_io = end_bio_io_page;
 	bio->bi_private = page;
-- 
2.40.1


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

* [PATCH v7 10/20] zonefs: use __bio_add_page for adding single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (8 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 09/20] gfs2: use __bio_add_page for adding " Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 11/20] zram: " Johannes Thumshirn
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The zonefs superblock reading code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is
never checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Acked-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/zonefs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index 23b8b299c64e..9350221abfc5 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -1128,7 +1128,7 @@ static int zonefs_read_super(struct super_block *sb)
 
 	bio_init(&bio, sb->s_bdev, &bio_vec, 1, REQ_OP_READ);
 	bio.bi_iter.bi_sector = 0;
-	bio_add_page(&bio, page, PAGE_SIZE, 0);
+	__bio_add_page(&bio, page, PAGE_SIZE, 0);
 
 	ret = submit_bio_wait(&bio);
 	if (ret)
-- 
2.40.1


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

* [PATCH v7 11/20] zram: use __bio_add_page for adding single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (9 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 10/20] zonefs: " Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 12/20] floppy: " Johannes Thumshirn
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal, Sergey Senozhatsky

The zram writeback code uses bio_add_page() to add a page to a newly
created bio. bio_add_page() can fail, but the return value is never
checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/block/zram/zram_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f6d90f1ba5cf..b86691d2133e 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -700,7 +700,7 @@ static ssize_t writeback_store(struct device *dev,
 		bio_init(&bio, zram->bdev, &bio_vec, 1,
 			 REQ_OP_WRITE | REQ_SYNC);
 		bio.bi_iter.bi_sector = blk_idx * (PAGE_SIZE >> 9);
-		bio_add_page(&bio, page, PAGE_SIZE, 0);
+		__bio_add_page(&bio, page, PAGE_SIZE, 0);
 
 		/*
 		 * XXX: A single page IO would be inefficient for write
-- 
2.40.1


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

* [PATCH v7 12/20] floppy: use __bio_add_page for adding single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (10 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 11/20] zram: " Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 13/20] md: check for failure when adding pages in alloc_behind_master_bio Johannes Thumshirn
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The floppy code uses bio_add_page() to add a page to a newly created bio.
bio_add_page() can fail, but the return value is never checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/block/floppy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index cec2c20f5e59..28ec6b442e9c 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4147,7 +4147,7 @@ static int __floppy_read_block_0(struct block_device *bdev, int drive)
 	cbdata.drive = drive;
 
 	bio_init(&bio, bdev, &bio_vec, 1, REQ_OP_READ);
-	bio_add_page(&bio, page, block_size(bdev), 0);
+	__bio_add_page(&bio, page, block_size(bdev), 0);
 
 	bio.bi_iter.bi_sector = 0;
 	bio.bi_flags |= (1 << BIO_QUIET);
-- 
2.40.1


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

* [PATCH v7 13/20] md: check for failure when adding pages in alloc_behind_master_bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (11 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 12/20] floppy: " Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 14/20] md: raid1: use __bio_add_page for adding single page to bio Johannes Thumshirn
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

alloc_behind_master_bio() can possibly add multiple pages to a bio, but it
is not checking for the return value of bio_add_page() if adding really
succeeded.

Check if the page adding succeeded and if not bail out.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Song Liu <song@kernel.org>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/raid1.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 68a9e2d9985b..8283ef177f6c 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1147,7 +1147,10 @@ static void alloc_behind_master_bio(struct r1bio *r1_bio,
 		if (unlikely(!page))
 			goto free_pages;
 
-		bio_add_page(behind_bio, page, len, 0);
+		if (!bio_add_page(behind_bio, page, len, 0)) {
+			free_page(page);
+			goto free_pages;
+		}
 
 		size -= len;
 		i++;
-- 
2.40.1


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

* [PATCH v7 14/20] md: raid1: use __bio_add_page for adding single page to bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (12 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 13/20] md: check for failure when adding pages in alloc_behind_master_bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 15/20] md: raid1: check if adding pages to resync bio fails Johannes Thumshirn
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

The sync request code uses bio_add_page() to add a page to a newly created bio.
bio_add_page() can fail, but the return value is never checked.

Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.

This brings us a step closer to marking bio_add_page() as __must_check.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/raid1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 8283ef177f6c..ff89839455ec 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2917,7 +2917,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 				 * won't fail because the vec table is big
 				 * enough to hold all these pages
 				 */
-				bio_add_page(bio, page, len, 0);
+				__bio_add_page(bio, page, len, 0);
 			}
 		}
 		nr_sectors += len>>9;
-- 
2.40.1


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

* [PATCH v7 15/20] md: raid1: check if adding pages to resync bio fails
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (13 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 14/20] md: raid1: use __bio_add_page for adding single page to bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 16/20] dm-crypt: use __bio_add_page to add single page to clone bio Johannes Thumshirn
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

Check if adding pages to resync bio fails and if bail out.

As the comment above suggests this cannot happen, WARN if it actually
happens. Technically __bio_add_pages() would be sufficient here, but
asserting the pages actually get added to the bio is preferred.

This way we can mark bio_add_pages as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/raid1-10.c | 11 ++++++-----
 drivers/md/raid10.c   | 20 ++++++++++----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index e61f6cad4e08..cd349e69ed77 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -101,11 +101,12 @@ static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
 		struct page *page = resync_fetch_page(rp, idx);
 		int len = min_t(int, size, PAGE_SIZE);
 
-		/*
-		 * won't fail because the vec table is big
-		 * enough to hold all these pages
-		 */
-		bio_add_page(bio, page, len, 0);
+		if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
+			bio->bi_status = BLK_STS_RESOURCE;
+			bio_endio(bio);
+			return;
+		}
+
 		size -= len;
 	} while (idx++ < RESYNC_PAGES && size > 0);
 }
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 4fcfcb350d2b..381c21f7fb06 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3819,11 +3819,11 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 		for (bio= biolist ; bio ; bio=bio->bi_next) {
 			struct resync_pages *rp = get_resync_pages(bio);
 			page = resync_fetch_page(rp, page_idx);
-			/*
-			 * won't fail because the vec table is big enough
-			 * to hold all these pages
-			 */
-			bio_add_page(bio, page, len, 0);
+			if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
+				bio->bi_status = BLK_STS_RESOURCE;
+				bio_endio(bio);
+				goto giveup;
+			}
 		}
 		nr_sectors += len>>9;
 		sector_nr += len>>9;
@@ -4997,11 +4997,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
 		if (len > PAGE_SIZE)
 			len = PAGE_SIZE;
 		for (bio = blist; bio ; bio = bio->bi_next) {
-			/*
-			 * won't fail because the vec table is big enough
-			 * to hold all these pages
-			 */
-			bio_add_page(bio, page, len, 0);
+			if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
+				bio->bi_status = BLK_STS_RESOURCE;
+				bio_endio(bio);
+				return sectors_done;
+			}
 		}
 		sector_nr += len >> 9;
 		nr_sectors += len >> 9;
-- 
2.40.1


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

* [PATCH v7 16/20] dm-crypt: use __bio_add_page to add single page to clone bio
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (14 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 15/20] md: raid1: check if adding pages to resync bio fails Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 11:50 ` [PATCH v7 17/20] block: mark bio_add_page as __must_check Johannes Thumshirn
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn

crypt_alloc_buffer() already allocates enough entries in the clone bio's
vector, so adding a page to the bio can't fail. Use __bio_add_page() to
reflect this.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/md/dm-crypt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 8b47b913ee83..09e37ebf7cc8 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1693,8 +1693,7 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size)
 
 		len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
 
-		bio_add_page(clone, page, len, 0);
-
+		__bio_add_page(clone, page, len, 0);
 		remaining_size -= len;
 	}
 
-- 
2.40.1


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

* [PATCH v7 17/20] block: mark bio_add_page as __must_check
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (15 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 16/20] dm-crypt: use __bio_add_page to add single page to clone bio Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 12:32   ` Christoph Hellwig
  2023-05-31 11:50 ` [PATCH v7 18/20] block: add bio_add_folio_nofail Johannes Thumshirn
                   ` (3 subsequent siblings)
  20 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn,
	Damien Le Moal

Now that all users of bio_add_page check for the return value, mark
bio_add_page as __must_check.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 include/linux/bio.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index b3e7529ff55e..ea2d937d3cba 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -465,7 +465,8 @@ extern void bio_uninit(struct bio *);
 void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf);
 void bio_chain(struct bio *, struct bio *);
 
-int bio_add_page(struct bio *, struct page *, unsigned len, unsigned off);
+int __must_check bio_add_page(struct bio *bio, struct page *page, unsigned len,
+			      unsigned off);
 bool bio_add_folio(struct bio *, struct folio *, size_t len, size_t off);
 extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *,
 			   unsigned int, unsigned int);
-- 
2.40.1


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

* [PATCH v7 18/20] block: add bio_add_folio_nofail
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (16 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 17/20] block: mark bio_add_page as __must_check Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 12:32   ` Christoph Hellwig
  2023-05-31 11:50 ` [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible Johannes Thumshirn
                   ` (2 subsequent siblings)
  20 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn

Just like for bio_add_pages() add a no-fail variant for bio_add_folio().

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 block/bio.c         | 8 ++++++++
 include/linux/bio.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/block/bio.c b/block/bio.c
index 043944fd46eb..07bc5139f9db 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1138,6 +1138,14 @@ int bio_add_page(struct bio *bio, struct page *page,
 }
 EXPORT_SYMBOL(bio_add_page);
 
+void bio_add_folio_nofail(struct bio *bio, struct folio *folio, size_t len,
+			  size_t off)
+{
+	WARN_ON_ONCE(len > UINT_MAX);
+	WARN_ON_ONCE(off > UINT_MAX);
+	__bio_add_page(bio, &folio->page, len, off);
+}
+
 /**
  * bio_add_folio - Attempt to add part of a folio to a bio.
  * @bio: BIO to add to.
diff --git a/include/linux/bio.h b/include/linux/bio.h
index ea2d937d3cba..f907d75af205 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -474,6 +474,8 @@ int bio_add_zone_append_page(struct bio *bio, struct page *page,
 			     unsigned int len, unsigned int offset);
 void __bio_add_page(struct bio *bio, struct page *page,
 		unsigned int len, unsigned int off);
+void bio_add_folio_nofail(struct bio *bio, struct folio *folio, size_t len,
+			  size_t off);
 int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter);
 void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter);
 void __bio_release_pages(struct bio *bio, bool mark_dirty);
-- 
2.40.1


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

* [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (17 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 18/20] block: add bio_add_folio_nofail Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 22:36   ` Dave Chinner
  2023-05-31 11:50 ` [PATCH v7 20/20] block: mark bio_add_folio as __must_check Johannes Thumshirn
  2023-05-31 15:50 ` [PATCH v7 00/20] bio: check return values of bio_add_page Jens Axboe
  20 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn

When the iomap buffered-io code can't add a folio to a bio, it allocates a
new bio and adds the folio to that one. This is done using bio_add_folio(),
but doesn't check for errors.

As adding a folio to a newly created bio can't fail, use the newly
introduced bio_add_folio_nofail() function.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/iomap/buffered-io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 063133ec77f4..0edab9deae2a 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -312,7 +312,7 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
 			ctx->bio->bi_opf |= REQ_RAHEAD;
 		ctx->bio->bi_iter.bi_sector = sector;
 		ctx->bio->bi_end_io = iomap_read_end_io;
-		bio_add_folio(ctx->bio, folio, plen, poff);
+		bio_add_folio_nofail(ctx->bio, folio, plen, poff);
 	}
 
 done:
@@ -539,7 +539,7 @@ static int iomap_read_folio_sync(loff_t block_start, struct folio *folio,
 
 	bio_init(&bio, iomap->bdev, &bvec, 1, REQ_OP_READ);
 	bio.bi_iter.bi_sector = iomap_sector(iomap, block_start);
-	bio_add_folio(&bio, folio, plen, poff);
+	bio_add_folio_nofail(&bio, folio, plen, poff);
 	return submit_bio_wait(&bio);
 }
 
@@ -1582,7 +1582,7 @@ iomap_add_to_ioend(struct inode *inode, loff_t pos, struct folio *folio,
 
 	if (!bio_add_folio(wpc->ioend->io_bio, folio, len, poff)) {
 		wpc->ioend->io_bio = iomap_chain_bio(wpc->ioend->io_bio);
-		bio_add_folio(wpc->ioend->io_bio, folio, len, poff);
+		bio_add_folio_nofail(wpc->ioend->io_bio, folio, len, poff);
 	}
 
 	if (iop)
-- 
2.40.1


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

* [PATCH v7 20/20] block: mark bio_add_folio as __must_check
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (18 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible Johannes Thumshirn
@ 2023-05-31 11:50 ` Johannes Thumshirn
  2023-05-31 12:34   ` Christoph Hellwig
  2023-05-31 15:50 ` [PATCH v7 00/20] bio: check return values of bio_add_page Jens Axboe
  20 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2023-05-31 11:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7, Johannes Thumshirn

Now that all callers of bio_add_folio() check the return value, mark it as
__must_check.

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 include/linux/bio.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index f907d75af205..c7a9425d19ee 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -467,7 +467,8 @@ void bio_chain(struct bio *, struct bio *);
 
 int __must_check bio_add_page(struct bio *bio, struct page *page, unsigned len,
 			      unsigned off);
-bool bio_add_folio(struct bio *, struct folio *, size_t len, size_t off);
+bool __must_check bio_add_folio(struct bio *bio, struct folio *folio,
+				size_t len, size_t off);
 extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *,
 			   unsigned int, unsigned int);
 int bio_add_zone_append_page(struct bio *bio, struct page *page,
-- 
2.40.1


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

* Re: [PATCH v7 17/20] block: mark bio_add_page as __must_check
  2023-05-31 11:50 ` [PATCH v7 17/20] block: mark bio_add_page as __must_check Johannes Thumshirn
@ 2023-05-31 12:32   ` Christoph Hellwig
  0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2023-05-31 12:32 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Jens Axboe, Christoph Hellwig, Hannes Reinecke,
	Chaitanya Kulkarni, Damien Le Moal, Ming Lei, linux-block,
	linux-fsdevel, linux-mm, dm-devel, Song Liu, linux-raid,
	Mike Snitzer, Matthew Wilcox, Dave Kleikamp, jfs-discussion,
	cluster-devel, Bob Peterson, Andreas Gruenbacher,
	Mikulas Patocka, gouha7, Damien Le Moal

Looks good:

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

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

* Re: [PATCH v7 18/20] block: add bio_add_folio_nofail
  2023-05-31 11:50 ` [PATCH v7 18/20] block: add bio_add_folio_nofail Johannes Thumshirn
@ 2023-05-31 12:32   ` Christoph Hellwig
  0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2023-05-31 12:32 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Jens Axboe, Christoph Hellwig, Hannes Reinecke,
	Chaitanya Kulkarni, Damien Le Moal, Ming Lei, linux-block,
	linux-fsdevel, linux-mm, dm-devel, Song Liu, linux-raid,
	Mike Snitzer, Matthew Wilcox, Dave Kleikamp, jfs-discussion,
	cluster-devel, Bob Peterson, Andreas Gruenbacher,
	Mikulas Patocka, gouha7

Looks good:

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

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

* Re: [PATCH v7 20/20] block: mark bio_add_folio as __must_check
  2023-05-31 11:50 ` [PATCH v7 20/20] block: mark bio_add_folio as __must_check Johannes Thumshirn
@ 2023-05-31 12:34   ` Christoph Hellwig
  0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2023-05-31 12:34 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Jens Axboe, Christoph Hellwig, Hannes Reinecke,
	Chaitanya Kulkarni, Damien Le Moal, Ming Lei, linux-block,
	linux-fsdevel, linux-mm, dm-devel, Song Liu, linux-raid,
	Mike Snitzer, Matthew Wilcox, Dave Kleikamp, jfs-discussion,
	cluster-devel, Bob Peterson, Andreas Gruenbacher,
	Mikulas Patocka, gouha7

Looks good:

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

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

* Re: [PATCH v7 00/20] bio: check return values of bio_add_page
  2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
                   ` (19 preceding siblings ...)
  2023-05-31 11:50 ` [PATCH v7 20/20] block: mark bio_add_folio as __must_check Johannes Thumshirn
@ 2023-05-31 15:50 ` Jens Axboe
  20 siblings, 0 replies; 28+ messages in thread
From: Jens Axboe @ 2023-05-31 15:50 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Christoph Hellwig, Hannes Reinecke, Chaitanya Kulkarni,
	Damien Le Moal, Ming Lei, linux-block, linux-fsdevel, linux-mm,
	dm-devel, Song Liu, linux-raid, Mike Snitzer, Matthew Wilcox,
	Dave Kleikamp, jfs-discussion, cluster-devel, Bob Peterson,
	Andreas Gruenbacher, Mikulas Patocka, gouha7


On Wed, 31 May 2023 04:50:23 -0700, Johannes Thumshirn wrote:
> We have two functions for adding a page to a bio, __bio_add_page() which is
> used to add a single page to a freshly created bio and bio_add_page() which is
> used to add a page to an existing bio.
> 
> While __bio_add_page() is expected to succeed, bio_add_page() can fail.
> 
> This series converts the callers of bio_add_page() which can easily use
> __bio_add_page() to using it and checks the return of bio_add_page() for
> callers that don't work on a freshly created bio.
> 
> [...]

Applied, thanks!

[01/20] swap: use __bio_add_page to add page to bio
        commit: cb58bf91b138c1a8b18cca9503308789e26e3522
[02/20] drbd: use __bio_add_page to add page to bio
        commit: 8f11f79f193c935da617375ba5ea4e768a73a094
[03/20] dm: dm-zoned: use __bio_add_page for adding single metadata page
        commit: fc8ac3e539561aff1c0a255d701d9412d425373c
[04/20] fs: buffer: use __bio_add_page to add single page to bio
        commit: 741af75d4027b1229fc6e62f4e3c4378dfe04897
[05/20] md: use __bio_add_page to add single page
        commit: 3c383235c51dcd6198d37ac3ac06e2acad79f981
[06/20] md: raid5-log: use __bio_add_page to add single page
        commit: b0a2f17cad9d3fa564d67c543f5d19343401fefd
[07/20] md: raid5: use __bio_add_page to add single page to new bio
        commit: 6eea4ff8528d6a5b9f0eeb47992e48a8f44b5b8f
[08/20] jfs: logmgr: use __bio_add_page to add single page to bio
        commit: 2896db174ced7a800863223f9e74543b98271ba0
[09/20] gfs2: use __bio_add_page for adding single page to bio
        commit: effa7ddeeba782406c81b572791a142fbdaf6b05
[10/20] zonefs: use __bio_add_page for adding single page to bio
        commit: 0fa5b08cf6e17b0a64ffcc5894d8efe186691ab8
[11/20] zram: use __bio_add_page for adding single page to bio
        commit: 34848c910b911838e1e83e1370cb988b578c8860
[12/20] floppy: use __bio_add_page for adding single page to bio
        commit: 5225229b8fdfb3e65520c43547ecf9a737161c3f
[13/20] md: check for failure when adding pages in alloc_behind_master_bio
        commit: 6473bc325644b9c8473e6c92bfb520a68dce1e12
[14/20] md: raid1: use __bio_add_page for adding single page to bio
        commit: 2f9848178cfa4ac68a5b46e63e5163a09b8bd80f
[15/20] md: raid1: check if adding pages to resync bio fails
        commit: 33332be32fe91ff54ff326b3a1608973544e835a
[16/20] dm-crypt: use __bio_add_page to add single page to clone bio
        commit: 9be63ecfdd63f957b9ed25eaf85666d22a02d7a5
[17/20] block: mark bio_add_page as __must_check
        commit: 5b3e39c1cc8e1cf31a398830dd665eb15546b4f7
[18/20] block: add bio_add_folio_nofail
        commit: 42205551d1d43b1b42942fb7ef023cf954136cea
[19/20] fs: iomap: use bio_add_folio_nofail where possible
        commit: f31c58ab3ddaf64503d7988197602d7443d5be37
[20/20] block: mark bio_add_folio as __must_check
        commit: 9320744e4dbe10df6059b2b6531946c200a0ba3b

Best regards,
-- 
Jens Axboe




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

* Re: [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible
  2023-05-31 11:50 ` [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible Johannes Thumshirn
@ 2023-05-31 22:36   ` Dave Chinner
  2023-06-01  4:20     ` Christoph Hellwig
  0 siblings, 1 reply; 28+ messages in thread
From: Dave Chinner @ 2023-05-31 22:36 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Jens Axboe, Christoph Hellwig, Hannes Reinecke,
	Chaitanya Kulkarni, Damien Le Moal, Ming Lei, linux-block,
	linux-fsdevel, linux-mm, dm-devel, Song Liu, linux-raid,
	Mike Snitzer, Matthew Wilcox, Dave Kleikamp, jfs-discussion,
	cluster-devel, Bob Peterson, Andreas Gruenbacher,
	Mikulas Patocka, gouha7

On Wed, May 31, 2023 at 04:50:42AM -0700, Johannes Thumshirn wrote:
> When the iomap buffered-io code can't add a folio to a bio, it allocates a
> new bio and adds the folio to that one. This is done using bio_add_folio(),
> but doesn't check for errors.
> 
> As adding a folio to a newly created bio can't fail, use the newly
> introduced bio_add_folio_nofail() function.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/iomap/buffered-io.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 063133ec77f4..0edab9deae2a 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -312,7 +312,7 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
>  			ctx->bio->bi_opf |= REQ_RAHEAD;
>  		ctx->bio->bi_iter.bi_sector = sector;
>  		ctx->bio->bi_end_io = iomap_read_end_io;
> -		bio_add_folio(ctx->bio, folio, plen, poff);
> +		bio_add_folio_nofail(ctx->bio, folio, plen, poff);
>  	}
>  
>  done:
> @@ -539,7 +539,7 @@ static int iomap_read_folio_sync(loff_t block_start, struct folio *folio,
>  
>  	bio_init(&bio, iomap->bdev, &bvec, 1, REQ_OP_READ);
>  	bio.bi_iter.bi_sector = iomap_sector(iomap, block_start);
> -	bio_add_folio(&bio, folio, plen, poff);
> +	bio_add_folio_nofail(&bio, folio, plen, poff);
>  	return submit_bio_wait(&bio);
>  }
>  
> @@ -1582,7 +1582,7 @@ iomap_add_to_ioend(struct inode *inode, loff_t pos, struct folio *folio,
>  
>  	if (!bio_add_folio(wpc->ioend->io_bio, folio, len, poff)) {
>  		wpc->ioend->io_bio = iomap_chain_bio(wpc->ioend->io_bio);
> -		bio_add_folio(wpc->ioend->io_bio, folio, len, poff);
> +		bio_add_folio_nofail(wpc->ioend->io_bio, folio, len, poff);
>  	}

We lose adjacent page merging with this change.

We've had performance regressions in the past that have been
attributed to either the page allocator not handing out sequential
adjacent pages for sequential writes and/or bios not merging
adjacent pages. Some hardware is much more performant when it only
has to do a single large DMA instead of (potentially) hundreds of
single page DMAs for the same amount of data...

What performance regression testing has been done on this change?

-Dave.
-- 
Dave Chinner
david@fromorbit.com

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

* Re: [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible
  2023-05-31 22:36   ` Dave Chinner
@ 2023-06-01  4:20     ` Christoph Hellwig
  2023-06-01  4:26       ` Dave Chinner
  0 siblings, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2023-06-01  4:20 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Johannes Thumshirn, Jens Axboe, Christoph Hellwig,
	Hannes Reinecke, Chaitanya Kulkarni, Damien Le Moal, Ming Lei,
	linux-block, linux-fsdevel, linux-mm, dm-devel, Song Liu,
	linux-raid, Mike Snitzer, Matthew Wilcox, Dave Kleikamp,
	jfs-discussion, cluster-devel, Bob Peterson, Andreas Gruenbacher,
	Mikulas Patocka, gouha7

On Thu, Jun 01, 2023 at 08:36:59AM +1000, Dave Chinner wrote:
> We lose adjacent page merging with this change.

This is only used for adding the first folio to a brand new bio,
so there is nothing to merge with yet at this point.

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

* Re: [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible
  2023-06-01  4:20     ` Christoph Hellwig
@ 2023-06-01  4:26       ` Dave Chinner
  0 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2023-06-01  4:26 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Johannes Thumshirn, Jens Axboe, Hannes Reinecke,
	Chaitanya Kulkarni, Damien Le Moal, Ming Lei, linux-block,
	linux-fsdevel, linux-mm, dm-devel, Song Liu, linux-raid,
	Mike Snitzer, Matthew Wilcox, Dave Kleikamp, jfs-discussion,
	cluster-devel, Bob Peterson, Andreas Gruenbacher,
	Mikulas Patocka, gouha7

On Thu, Jun 01, 2023 at 06:20:21AM +0200, Christoph Hellwig wrote:
> On Thu, Jun 01, 2023 at 08:36:59AM +1000, Dave Chinner wrote:
> > We lose adjacent page merging with this change.
> 
> This is only used for adding the first folio to a brand new bio,
> so there is nothing to merge with yet at this point.

Ah, sorry, missed that. Carry on. :)

-Dave.
-- 
Dave Chinner
david@fromorbit.com

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

end of thread, other threads:[~2023-06-01  4:26 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 11:50 [PATCH v7 00/20] bio: check return values of bio_add_page Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 01/20] swap: use __bio_add_page to add page to bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 02/20] drbd: " Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 03/20] dm: dm-zoned: use __bio_add_page for adding single metadata page Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 04/20] fs: buffer: use __bio_add_page to add single page to bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 05/20] md: use __bio_add_page to add single page Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 06/20] md: raid5-log: " Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 07/20] md: raid5: use __bio_add_page to add single page to new bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 08/20] jfs: logmgr: use __bio_add_page to add single page to bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 09/20] gfs2: use __bio_add_page for adding " Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 10/20] zonefs: " Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 11/20] zram: " Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 12/20] floppy: " Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 13/20] md: check for failure when adding pages in alloc_behind_master_bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 14/20] md: raid1: use __bio_add_page for adding single page to bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 15/20] md: raid1: check if adding pages to resync bio fails Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 16/20] dm-crypt: use __bio_add_page to add single page to clone bio Johannes Thumshirn
2023-05-31 11:50 ` [PATCH v7 17/20] block: mark bio_add_page as __must_check Johannes Thumshirn
2023-05-31 12:32   ` Christoph Hellwig
2023-05-31 11:50 ` [PATCH v7 18/20] block: add bio_add_folio_nofail Johannes Thumshirn
2023-05-31 12:32   ` Christoph Hellwig
2023-05-31 11:50 ` [PATCH v7 19/20] fs: iomap: use bio_add_folio_nofail where possible Johannes Thumshirn
2023-05-31 22:36   ` Dave Chinner
2023-06-01  4:20     ` Christoph Hellwig
2023-06-01  4:26       ` Dave Chinner
2023-05-31 11:50 ` [PATCH v7 20/20] block: mark bio_add_folio as __must_check Johannes Thumshirn
2023-05-31 12:34   ` Christoph Hellwig
2023-05-31 15:50 ` [PATCH v7 00/20] bio: check return values of bio_add_page Jens Axboe

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).