From: Shida Zhang Don't call bio->bi_end_io() directly. Use the bio_endio() helper function instead, which handles completion more safely and uniformly. Suggested-by: Christoph Hellwig Signed-off-by: Shida Zhang --- drivers/md/bcache/request.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c index af345dc6fde..82fdea7dea7 100644 --- a/drivers/md/bcache/request.c +++ b/drivers/md/bcache/request.c @@ -1104,7 +1104,7 @@ static void detached_dev_end_io(struct bio *bio) } kfree(ddip); - bio->bi_end_io(bio); + bio_endio(bio); } static void detached_dev_do_request(struct bcache_device *d, struct bio *bio, @@ -1121,7 +1121,7 @@ static void detached_dev_do_request(struct bcache_device *d, struct bio *bio, ddip = kzalloc(sizeof(struct detached_dev_io_private), GFP_NOIO); if (!ddip) { bio->bi_status = BLK_STS_RESOURCE; - bio->bi_end_io(bio); + bio_endio(bio); return; } @@ -1136,7 +1136,7 @@ static void detached_dev_do_request(struct bcache_device *d, struct bio *bio, if ((bio_op(bio) == REQ_OP_DISCARD) && !bdev_max_discard_sectors(dc->bdev)) - bio->bi_end_io(bio); + detached_dev_end_io(bio); else submit_bio_noacct(bio); } -- 2.34.1 From: Shida Zhang Now that all potential callers of bio_chain_endio have been eliminated, completely prohibit any future calls to this function. Suggested-by: Ming Lei Suggested-by: Andreas Gruenbacher Suggested-by: Christoph Hellwig Signed-off-by: Shida Zhang --- block/bio.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/block/bio.c b/block/bio.c index b3a79285c27..1b5e4577f4c 100644 --- a/block/bio.c +++ b/block/bio.c @@ -320,9 +320,13 @@ static struct bio *__bio_chain_endio(struct bio *bio) return parent; } +/** + * This function should only be used as a flag and must never be called. + * If execution reaches here, it indicates a serious programming error. + */ static void bio_chain_endio(struct bio *bio) { - bio_endio(__bio_chain_endio(bio)); + BUG_ON(1); } /** -- 2.34.1 From: Shida Zhang Andreas point out that multiple completions can race setting bi_status. The check (parent->bi_status) and the subsequent write are not an atomic operation. The value of parent->bi_status could have changed between the time you read it for the if check and the time you write to it. So we use cmpxchg to fix the race, as suggested by Christoph. Suggested-by: Andreas Gruenbacher Suggested-by: Christoph Hellwig Suggested-by: Caleb Sander Mateos Signed-off-by: Shida Zhang --- block/bio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/bio.c b/block/bio.c index 1b5e4577f4c..097c1cd2054 100644 --- a/block/bio.c +++ b/block/bio.c @@ -314,8 +314,9 @@ static struct bio *__bio_chain_endio(struct bio *bio) { struct bio *parent = bio->bi_private; - if (bio->bi_status && !parent->bi_status) - parent->bi_status = bio->bi_status; + if (bio->bi_status) + cmpxchg(&parent->bi_status, 0, bio->bi_status); + bio_put(bio); return parent; } -- 2.34.1 From: Shida Zhang Export the bio_chain_and_submit function to make it available as a common utility. This will allow replacing repetitive bio chaining patterns found in multiple locations throughout the codebase. Signed-off-by: Shida Zhang --- block/bio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/bio.c b/block/bio.c index 097c1cd2054..7aa4a1d3672 100644 --- a/block/bio.c +++ b/block/bio.c @@ -368,6 +368,7 @@ struct bio *bio_chain_and_submit(struct bio *prev, struct bio *new) } return new; } +EXPORT_SYMBOL_GPL(bio_chain_and_submit); struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, unsigned int nr_pages, blk_opf_t opf, gfp_t gfp) -- 2.34.1 From: Shida Zhang Replace duplicate bio chaining logic with the common bio_chain_and_submit helper function. Signed-off-by: Shida Zhang --- fs/xfs/xfs_bio_io.c | 3 +-- fs/xfs/xfs_buf.c | 3 +-- fs/xfs/xfs_log.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/xfs/xfs_bio_io.c b/fs/xfs/xfs_bio_io.c index 2a736d10eaf..4a6577b0789 100644 --- a/fs/xfs/xfs_bio_io.c +++ b/fs/xfs/xfs_bio_io.c @@ -38,8 +38,7 @@ xfs_rw_bdev( bio_max_vecs(count - done), prev->bi_opf, GFP_KERNEL); bio->bi_iter.bi_sector = bio_end_sector(prev); - bio_chain(prev, bio); - submit_bio(prev); + bio_chain_and_submit(prev, bio); } done += added; } while (done < count); diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 773d959965d..c26bd28edb4 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1357,8 +1357,7 @@ xfs_buf_submit_bio( split = bio_split(bio, bp->b_maps[map].bm_len, GFP_NOFS, &fs_bio_set); split->bi_iter.bi_sector = bp->b_maps[map].bm_bn; - bio_chain(split, bio); - submit_bio(split); + bio_chain_and_submit(split, bio); } bio->bi_iter.bi_sector = bp->b_maps[map].bm_bn; submit_bio(bio); diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 603e85c1ab4..f4c9ad1d148 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -1687,8 +1687,7 @@ xlog_write_iclog( split = bio_split(&iclog->ic_bio, log->l_logBBsize - bno, GFP_NOIO, &fs_bio_set); - bio_chain(split, &iclog->ic_bio); - submit_bio(split); + bio_chain_and_submit(split, &iclog->ic_bio); /* restart at logical offset zero for the remainder */ iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart; -- 2.34.1 From: Shida Zhang Replace duplicate bio chaining logic with the common bio_chain_and_submit helper function. Signed-off-by: Shida Zhang --- fs/squashfs/block.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index a05e3793f93..5818e473255 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -126,8 +126,7 @@ static int squashfs_bio_read_cached(struct bio *fullbio, if (bio) { bio_trim(bio, start_idx * PAGE_SECTORS, (end_idx - start_idx) * PAGE_SECTORS); - bio_chain(bio, new); - submit_bio(bio); + bio_chain_and_submit(bio, new); } bio = new; -- 2.34.1 From: Shida Zhang Replace duplicate bio chaining logic with the common bio_chain_and_submit helper function. Signed-off-by: Shida Zhang --- fs/ntfs3/fsntfs.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c index c7a2f191254..35685ee4ed2 100644 --- a/fs/ntfs3/fsntfs.c +++ b/fs/ntfs3/fsntfs.c @@ -1514,11 +1514,7 @@ int ntfs_bio_pages(struct ntfs_sb_info *sbi, const struct runs_tree *run, len = ((u64)clen << cluster_bits) - off; new_bio: new = bio_alloc(bdev, nr_pages - page_idx, op, GFP_NOFS); - if (bio) { - bio_chain(bio, new); - submit_bio(bio); - } - bio = new; + bio = bio_chain_and_submit(bio, new); bio->bi_iter.bi_sector = lbo >> 9; while (len) { @@ -1611,11 +1607,7 @@ int ntfs_bio_fill_1(struct ntfs_sb_info *sbi, const struct runs_tree *run) len = (u64)clen << cluster_bits; new_bio: new = bio_alloc(bdev, BIO_MAX_VECS, REQ_OP_WRITE, GFP_NOFS); - if (bio) { - bio_chain(bio, new); - submit_bio(bio); - } - bio = new; + bio = bio_chain_and_submit(bio, new); bio->bi_iter.bi_sector = lbo >> 9; for (;;) { -- 2.34.1 From: Shida Zhang Replace duplicate bio chaining logic with the common bio_chain_and_submit helper function. Signed-off-by: Shida Zhang --- drivers/block/zram/zram_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index a4307465753..084de60ebaf 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -730,8 +730,7 @@ static void read_from_bdev_async(struct zram *zram, struct page *page, bio = bio_alloc(zram->bdev, 1, parent->bi_opf, GFP_NOIO); bio->bi_iter.bi_sector = entry * (PAGE_SIZE >> 9); __bio_add_page(bio, page, PAGE_SIZE, 0); - bio_chain(bio, parent); - submit_bio(bio); + bio_chain_and_submit(bio, parent); } static int zram_writeback_slots(struct zram *zram, struct zram_pp_ctl *ctl) -- 2.34.1 From: Shida Zhang Replace duplicate bio chaining logic with the common bio_chain_and_submit helper function. Signed-off-by: Shida Zhang --- drivers/nvdimm/nd_virtio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c index c3f07be4aa2..e6ec7ceee9b 100644 --- a/drivers/nvdimm/nd_virtio.c +++ b/drivers/nvdimm/nd_virtio.c @@ -122,8 +122,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio) return -ENOMEM; bio_clone_blkg_association(child, bio); child->bi_iter.bi_sector = -1; - bio_chain(child, bio); - submit_bio(child); + bio_chain_and_submit(child, bio); return 0; } if (virtio_pmem_flush(nd_region)) -- 2.34.1