* cleanup bio_kmalloc v2 @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Hi Jens, this series finishes off the bio allocation interface cleanups by dealing with the weirdest member of the famility. bio_kmalloc combines a kmalloc for the bio and bio_vecs with a hidden bio_init call and magic cleanup semantics. This series moves a few callers away from bio_kmalloc and then turns bio_kmalloc into a simple wrapper for a slab allocation of a bio and the inline biovecs. The callers need to manually call bio_init instead with all that entails and the magic that turns bio_put into a kfree goes away as well, allowing for a proper debug check in bio_put that catches accidental use on a bio_init()ed bio. Changes since v1: - update a pre-existing comment per maintainer suggestion Diffstat: block/bio.c | 47 ++++++++++++++----------------------- block/blk-crypto-fallback.c | 14 ++++++----- block/blk-map.c | 42 +++++++++++++++++++++------------ drivers/block/pktcdvd.c | 34 +++++++++++--------------- drivers/md/bcache/debug.c | 10 ++++--- drivers/md/dm-bufio.c | 9 +++---- drivers/md/raid1.c | 12 ++++++--- drivers/md/raid10.c | 21 +++++++++++----- drivers/target/target_core_pscsi.c | 36 ++++------------------------ fs/btrfs/disk-io.c | 8 +++--- fs/btrfs/volumes.c | 11 -------- fs/btrfs/volumes.h | 2 - fs/squashfs/block.c | 14 +++-------- include/linux/bio.h | 2 - 14 files changed, 116 insertions(+), 146 deletions(-) ^ permalink raw reply [flat|nested] 38+ messages in thread
* [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs Hi Jens, this series finishes off the bio allocation interface cleanups by dealing with the weirdest member of the famility. bio_kmalloc combines a kmalloc for the bio and bio_vecs with a hidden bio_init call and magic cleanup semantics. This series moves a few callers away from bio_kmalloc and then turns bio_kmalloc into a simple wrapper for a slab allocation of a bio and the inline biovecs. The callers need to manually call bio_init instead with all that entails and the magic that turns bio_put into a kfree goes away as well, allowing for a proper debug check in bio_put that catches accidental use on a bio_init()ed bio. Changes since v1: - update a pre-existing comment per maintainer suggestion Diffstat: block/bio.c | 47 ++++++++++++++----------------------- block/blk-crypto-fallback.c | 14 ++++++----- block/blk-map.c | 42 +++++++++++++++++++++------------ drivers/block/pktcdvd.c | 34 +++++++++++--------------- drivers/md/bcache/debug.c | 10 ++++--- drivers/md/dm-bufio.c | 9 +++---- drivers/md/raid1.c | 12 ++++++--- drivers/md/raid10.c | 21 +++++++++++----- drivers/target/target_core_pscsi.c | 36 ++++------------------------ fs/btrfs/disk-io.c | 8 +++--- fs/btrfs/volumes.c | 11 -------- fs/btrfs/volumes.h | 2 - fs/squashfs/block.c | 14 +++-------- include/linux/bio.h | 2 - 14 files changed, 116 insertions(+), 146 deletions(-) -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 1/5] btrfs: simplify ->flush_bio handling 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 6:15 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Use and embedded bios that is initialized when used instead of bio_kmalloc plus bio_reset. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: David Sterba <dsterba@suse.com> --- fs/btrfs/disk-io.c | 8 +++++--- fs/btrfs/volumes.c | 11 ----------- fs/btrfs/volumes.h | 4 ++-- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 505ba21230b1f..7d4229c769ac6 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4127,6 +4127,7 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors) */ static void btrfs_end_empty_barrier(struct bio *bio) { + bio_uninit(bio); complete(bio->bi_private); } @@ -4136,7 +4137,7 @@ static void btrfs_end_empty_barrier(struct bio *bio) */ static void write_dev_flush(struct btrfs_device *device) { - struct bio *bio = device->flush_bio; + struct bio *bio = &device->flush_bio; #ifndef CONFIG_BTRFS_FS_CHECK_INTEGRITY /* @@ -4154,7 +4155,8 @@ static void write_dev_flush(struct btrfs_device *device) return; #endif - bio_reset(bio, device->bdev, REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH); + bio_init(bio, device->bdev, NULL, 0, + REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH); bio->bi_end_io = btrfs_end_empty_barrier; init_completion(&device->flush_wait); bio->bi_private = &device->flush_wait; @@ -4168,7 +4170,7 @@ static void write_dev_flush(struct btrfs_device *device) */ static blk_status_t wait_dev_flush(struct btrfs_device *device) { - struct bio *bio = device->flush_bio; + struct bio *bio = &device->flush_bio; if (!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state)) return BLK_STS_OK; diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index b07d382d53a86..7015b3096f724 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -405,7 +405,6 @@ void btrfs_free_device(struct btrfs_device *device) WARN_ON(!list_empty(&device->post_commit_list)); rcu_string_free(device->name); extent_io_tree_release(&device->alloc_state); - bio_put(device->flush_bio); btrfs_destroy_dev_zone_info(device); kfree(device); } @@ -6962,16 +6961,6 @@ struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info, if (!dev) return ERR_PTR(-ENOMEM); - /* - * Preallocate a bio that's always going to be used for flushing device - * barriers and matches the device lifespan - */ - dev->flush_bio = bio_kmalloc(GFP_KERNEL, 0); - if (!dev->flush_bio) { - kfree(dev); - return ERR_PTR(-ENOMEM); - } - INIT_LIST_HEAD(&dev->dev_list); INIT_LIST_HEAD(&dev->dev_alloc_list); INIT_LIST_HEAD(&dev->post_commit_list); diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 005c9e2a491a1..dbd6cc5e5cd3b 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -116,8 +116,8 @@ struct btrfs_device { /* bytes used on the current transaction */ u64 commit_bytes_used; - /* for sending down flush barriers */ - struct bio *flush_bio; + /* Bio used for flushing device barriers */ + struct bio flush_bio; struct completion flush_wait; /* per-device scrub information */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [dm-devel] [PATCH 1/5] btrfs: simplify ->flush_bio handling @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs Use and embedded bios that is initialized when used instead of bio_kmalloc plus bio_reset. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: David Sterba <dsterba@suse.com> --- fs/btrfs/disk-io.c | 8 +++++--- fs/btrfs/volumes.c | 11 ----------- fs/btrfs/volumes.h | 4 ++-- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 505ba21230b1f..7d4229c769ac6 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4127,6 +4127,7 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors) */ static void btrfs_end_empty_barrier(struct bio *bio) { + bio_uninit(bio); complete(bio->bi_private); } @@ -4136,7 +4137,7 @@ static void btrfs_end_empty_barrier(struct bio *bio) */ static void write_dev_flush(struct btrfs_device *device) { - struct bio *bio = device->flush_bio; + struct bio *bio = &device->flush_bio; #ifndef CONFIG_BTRFS_FS_CHECK_INTEGRITY /* @@ -4154,7 +4155,8 @@ static void write_dev_flush(struct btrfs_device *device) return; #endif - bio_reset(bio, device->bdev, REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH); + bio_init(bio, device->bdev, NULL, 0, + REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH); bio->bi_end_io = btrfs_end_empty_barrier; init_completion(&device->flush_wait); bio->bi_private = &device->flush_wait; @@ -4168,7 +4170,7 @@ static void write_dev_flush(struct btrfs_device *device) */ static blk_status_t wait_dev_flush(struct btrfs_device *device) { - struct bio *bio = device->flush_bio; + struct bio *bio = &device->flush_bio; if (!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state)) return BLK_STS_OK; diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index b07d382d53a86..7015b3096f724 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -405,7 +405,6 @@ void btrfs_free_device(struct btrfs_device *device) WARN_ON(!list_empty(&device->post_commit_list)); rcu_string_free(device->name); extent_io_tree_release(&device->alloc_state); - bio_put(device->flush_bio); btrfs_destroy_dev_zone_info(device); kfree(device); } @@ -6962,16 +6961,6 @@ struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info, if (!dev) return ERR_PTR(-ENOMEM); - /* - * Preallocate a bio that's always going to be used for flushing device - * barriers and matches the device lifespan - */ - dev->flush_bio = bio_kmalloc(GFP_KERNEL, 0); - if (!dev->flush_bio) { - kfree(dev); - return ERR_PTR(-ENOMEM); - } - INIT_LIST_HEAD(&dev->dev_list); INIT_LIST_HEAD(&dev->dev_alloc_list); INIT_LIST_HEAD(&dev->post_commit_list); diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 005c9e2a491a1..dbd6cc5e5cd3b 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -116,8 +116,8 @@ struct btrfs_device { /* bytes used on the current transaction */ u64 commit_bytes_used; - /* for sending down flush barriers */ - struct bio *flush_bio; + /* Bio used for flushing device barriers */ + struct bio flush_bio; struct completion flush_wait; /* per-device scrub information */ -- 2.30.2 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 1/5] btrfs: simplify ->flush_bio handling 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 22:07 ` Chaitanya Kulkarni -1 siblings, 0 replies; 38+ messages in thread From: Chaitanya Kulkarni @ 2022-03-08 22:07 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On 3/7/22 22:15, Christoph Hellwig wrote: > Use and embedded bios that is initialized when used instead of > bio_kmalloc plus bio_reset. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: David Sterba <dsterba@suse.com> > --- Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 1/5] btrfs: simplify ->flush_bio handling @ 2022-03-08 22:07 ` Chaitanya Kulkarni 0 siblings, 0 replies; 38+ messages in thread From: Chaitanya Kulkarni @ 2022-03-08 22:07 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs On 3/7/22 22:15, Christoph Hellwig wrote: > Use and embedded bios that is initialized when used instead of > bio_kmalloc plus bio_reset. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: David Sterba <dsterba@suse.com> > --- Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 2/5] squashfs: always use bio_kmalloc in squashfs_bio_read 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 6:15 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs If a plain kmalloc that is not backed by a mempool is safe here for a large read (and the actual page allocations), it must also be for a small one, so simplify the code a bit. Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Phillip Lougher <phillip@squashfs.org.uk> --- fs/squashfs/block.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 622c844f6d118..4311a32218928 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -86,16 +86,11 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, int error, i; struct bio *bio; - if (page_count <= BIO_MAX_VECS) { - bio = bio_alloc(sb->s_bdev, page_count, REQ_OP_READ, GFP_NOIO); - } else { - bio = bio_kmalloc(GFP_NOIO, page_count); - bio_set_dev(bio, sb->s_bdev); - bio->bi_opf = REQ_OP_READ; - } - + bio = bio_kmalloc(GFP_NOIO, page_count); if (!bio) return -ENOMEM; + bio_set_dev(bio, sb->s_bdev); + bio->bi_opf = REQ_OP_READ; bio->bi_iter.bi_sector = block * (msblk->devblksize >> SECTOR_SHIFT); -- 2.30.2 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [dm-devel] [PATCH 2/5] squashfs: always use bio_kmalloc in squashfs_bio_read @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs If a plain kmalloc that is not backed by a mempool is safe here for a large read (and the actual page allocations), it must also be for a small one, so simplify the code a bit. Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Phillip Lougher <phillip@squashfs.org.uk> --- fs/squashfs/block.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 622c844f6d118..4311a32218928 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -86,16 +86,11 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, int error, i; struct bio *bio; - if (page_count <= BIO_MAX_VECS) { - bio = bio_alloc(sb->s_bdev, page_count, REQ_OP_READ, GFP_NOIO); - } else { - bio = bio_kmalloc(GFP_NOIO, page_count); - bio_set_dev(bio, sb->s_bdev); - bio->bi_opf = REQ_OP_READ; - } - + bio = bio_kmalloc(GFP_NOIO, page_count); if (!bio) return -ENOMEM; + bio_set_dev(bio, sb->s_bdev); + bio->bi_opf = REQ_OP_READ; bio->bi_iter.bi_sector = block * (msblk->devblksize >> SECTOR_SHIFT); -- 2.30.2 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 3/5] target/pscsi: remove pscsi_get_bio 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 6:15 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Remove pscsi_get_bio and simplify the code flow in the only caller. Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Martin K. Petersen <martin.petersen@oracle.com> --- drivers/target/target_core_pscsi.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index 0fae71ac5cc8a..bd8ae07273f14 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -823,23 +823,6 @@ static void pscsi_bi_endio(struct bio *bio) bio_put(bio); } -static inline struct bio *pscsi_get_bio(int nr_vecs) -{ - struct bio *bio; - /* - * Use bio_malloc() following the comment in for bio -> struct request - * in block/blk-core.c:blk_make_request() - */ - bio = bio_kmalloc(GFP_KERNEL, nr_vecs); - if (!bio) { - pr_err("PSCSI: bio_kmalloc() failed\n"); - return NULL; - } - bio->bi_end_io = pscsi_bi_endio; - - return bio; -} - static sense_reason_t pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, struct request *req) @@ -880,12 +863,10 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, if (!bio) { new_bio: nr_vecs = bio_max_segs(nr_pages); - /* - * Calls bio_kmalloc() and sets bio->bi_end_io() - */ - bio = pscsi_get_bio(nr_vecs); + bio = bio_kmalloc(GFP_KERNEL, nr_vecs); if (!bio) goto fail; + bio->bi_end_io = pscsi_bi_endio; if (rw) bio_set_op_attrs(bio, REQ_OP_WRITE, 0); @@ -914,11 +895,6 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, goto fail; } - /* - * Clear the pointer so that another bio will - * be allocated with pscsi_get_bio() above. - */ - bio = NULL; goto new_bio; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [dm-devel] [PATCH 3/5] target/pscsi: remove pscsi_get_bio @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs Remove pscsi_get_bio and simplify the code flow in the only caller. Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Martin K. Petersen <martin.petersen@oracle.com> --- drivers/target/target_core_pscsi.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index 0fae71ac5cc8a..bd8ae07273f14 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -823,23 +823,6 @@ static void pscsi_bi_endio(struct bio *bio) bio_put(bio); } -static inline struct bio *pscsi_get_bio(int nr_vecs) -{ - struct bio *bio; - /* - * Use bio_malloc() following the comment in for bio -> struct request - * in block/blk-core.c:blk_make_request() - */ - bio = bio_kmalloc(GFP_KERNEL, nr_vecs); - if (!bio) { - pr_err("PSCSI: bio_kmalloc() failed\n"); - return NULL; - } - bio->bi_end_io = pscsi_bi_endio; - - return bio; -} - static sense_reason_t pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, struct request *req) @@ -880,12 +863,10 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, if (!bio) { new_bio: nr_vecs = bio_max_segs(nr_pages); - /* - * Calls bio_kmalloc() and sets bio->bi_end_io() - */ - bio = pscsi_get_bio(nr_vecs); + bio = bio_kmalloc(GFP_KERNEL, nr_vecs); if (!bio) goto fail; + bio->bi_end_io = pscsi_bi_endio; if (rw) bio_set_op_attrs(bio, REQ_OP_WRITE, 0); @@ -914,11 +895,6 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, goto fail; } - /* - * Clear the pointer so that another bio will - * be allocated with pscsi_get_bio() above. - */ - bio = NULL; goto new_bio; } -- 2.30.2 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 3/5] target/pscsi: remove pscsi_get_bio 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 22:08 ` Chaitanya Kulkarni -1 siblings, 0 replies; 38+ messages in thread From: Chaitanya Kulkarni @ 2022-03-08 22:08 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On 3/7/22 22:15, Christoph Hellwig wrote: > Remove pscsi_get_bio and simplify the code flow in the only caller. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Martin K. Petersen <martin.petersen@oracle.com> > --- Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 3/5] target/pscsi: remove pscsi_get_bio @ 2022-03-08 22:08 ` Chaitanya Kulkarni 0 siblings, 0 replies; 38+ messages in thread From: Chaitanya Kulkarni @ 2022-03-08 22:08 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs On 3/7/22 22:15, Christoph Hellwig wrote: > Remove pscsi_get_bio and simplify the code flow in the only caller. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Martin K. Petersen <martin.petersen@oracle.com> > --- Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 6:15 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Remove the magic autofree semantics and require the callers to explicitly call bio_init to initialize the bio. This allows bio_free to catch accidental bio_put calls on bio_init()ed bios as well. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bio.c | 47 ++++++++++++------------------ block/blk-crypto-fallback.c | 14 +++++---- block/blk-map.c | 42 ++++++++++++++++---------- drivers/block/pktcdvd.c | 25 ++++++++-------- drivers/md/bcache/debug.c | 10 ++++--- drivers/md/dm-bufio.c | 9 +++--- drivers/md/raid1.c | 12 +++++--- drivers/md/raid10.c | 21 ++++++++----- drivers/target/target_core_pscsi.c | 10 +++---- fs/squashfs/block.c | 9 +++--- include/linux/bio.h | 2 +- 11 files changed, 108 insertions(+), 93 deletions(-) diff --git a/block/bio.c b/block/bio.c index 151cace2dbe16..1fe243e61e55d 100644 --- a/block/bio.c +++ b/block/bio.c @@ -224,24 +224,13 @@ EXPORT_SYMBOL(bio_uninit); static void bio_free(struct bio *bio) { struct bio_set *bs = bio->bi_pool; - void *p; - - bio_uninit(bio); + void *p = bio; - if (bs) { - bvec_free(&bs->bvec_pool, bio->bi_io_vec, bio->bi_max_vecs); + WARN_ON_ONCE(!bs); - /* - * If we have front padding, adjust the bio pointer before freeing - */ - p = bio; - p -= bs->front_pad; - - mempool_free(p, &bs->bio_pool); - } else { - /* Bio was allocated by bio_kmalloc() */ - kfree(bio); - } + bio_uninit(bio); + bvec_free(&bs->bvec_pool, bio->bi_io_vec, bio->bi_max_vecs); + mempool_free(p - bs->front_pad, &bs->bio_pool); } /* @@ -529,28 +518,28 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs, EXPORT_SYMBOL(bio_alloc_bioset); /** - * bio_kmalloc - kmalloc a bio for I/O + * bio_kmalloc - kmalloc a bio + * @nr_vecs: number of bio_vecs to allocate * @gfp_mask: the GFP_* mask given to the slab allocator - * @nr_iovecs: number of iovecs to pre-allocate * - * Use kmalloc to allocate and initialize a bio. + * Use kmalloc to allocate a bio (including bvecs). The bio must be initialized + * using bio_init() before use. To free a bio returned from this function use + * kfree() after calling bio_uninit(). A bio returned from this function can + * be reused by calling bio_uninit() before calling bio_init() again. + * + * Note that unlike bio_alloc() or bio_alloc_bioset() allocations from this + * function are not backed by a mempool can can fail. Do not use this function + * for allocations in the file system I/O path. * * Returns: Pointer to new bio on success, NULL on failure. */ -struct bio *bio_kmalloc(gfp_t gfp_mask, unsigned short nr_iovecs) +struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask) { struct bio *bio; - if (nr_iovecs > UIO_MAXIOV) + if (nr_vecs > UIO_MAXIOV) return NULL; - - bio = kmalloc(struct_size(bio, bi_inline_vecs, nr_iovecs), gfp_mask); - if (unlikely(!bio)) - return NULL; - bio_init(bio, NULL, nr_iovecs ? bio->bi_inline_vecs : NULL, nr_iovecs, - 0); - bio->bi_pool = NULL; - return bio; + return kmalloc(struct_size(bio, bi_inline_vecs, nr_vecs), gfp_mask); } EXPORT_SYMBOL(bio_kmalloc); diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c index 18c8eafe20b94..1a624b75da94f 100644 --- a/block/blk-crypto-fallback.c +++ b/block/blk-crypto-fallback.c @@ -152,23 +152,25 @@ static void blk_crypto_fallback_encrypt_endio(struct bio *enc_bio) src_bio->bi_status = enc_bio->bi_status; - bio_put(enc_bio); + bio_uninit(enc_bio); + kfree(enc_bio); bio_endio(src_bio); } static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src) { + unsigned int nr_segs = bio_segments(bio_src); struct bvec_iter iter; struct bio_vec bv; struct bio *bio; - bio = bio_kmalloc(GFP_NOIO, bio_segments(bio_src)); + bio = bio_kmalloc(nr_segs, GFP_NOIO); if (!bio) return NULL; - bio->bi_bdev = bio_src->bi_bdev; + bio_init(bio, bio_src->bi_bdev, bio->bi_inline_vecs, nr_segs, + bio_src->bi_opf); if (bio_flagged(bio_src, BIO_REMAPPED)) bio_set_flag(bio, BIO_REMAPPED); - bio->bi_opf = bio_src->bi_opf; bio->bi_ioprio = bio_src->bi_ioprio; bio->bi_write_hint = bio_src->bi_write_hint; bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; @@ -364,8 +366,8 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr) blk_crypto_put_keyslot(slot); out_put_enc_bio: if (enc_bio) - bio_put(enc_bio); - + bio_uninit(enc_bio); + kfree(enc_bio); return ret; } diff --git a/block/blk-map.c b/block/blk-map.c index 4526adde01564..59be60e69e7af 100644 --- a/block/blk-map.c +++ b/block/blk-map.c @@ -152,10 +152,10 @@ static int bio_copy_user_iov(struct request *rq, struct rq_map_data *map_data, nr_pages = bio_max_segs(DIV_ROUND_UP(offset + len, PAGE_SIZE)); ret = -ENOMEM; - bio = bio_kmalloc(gfp_mask, nr_pages); + bio = bio_kmalloc(nr_pages, gfp_mask); if (!bio) goto out_bmd; - bio->bi_opf |= req_op(rq); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_pages, req_op(rq)); if (map_data) { nr_pages = 1 << map_data->page_order; @@ -224,7 +224,8 @@ static int bio_copy_user_iov(struct request *rq, struct rq_map_data *map_data, cleanup: if (!map_data) bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); out_bmd: kfree(bmd); return ret; @@ -234,6 +235,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, gfp_t gfp_mask) { unsigned int max_sectors = queue_max_hw_sectors(rq->q); + unsigned int nr_vecs = iov_iter_npages(iter, BIO_MAX_VECS); struct bio *bio; int ret; int j; @@ -241,10 +243,10 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, if (!iov_iter_count(iter)) return -EINVAL; - bio = bio_kmalloc(gfp_mask, iov_iter_npages(iter, BIO_MAX_VECS)); + bio = bio_kmalloc(nr_vecs, gfp_mask); if (!bio) return -ENOMEM; - bio->bi_opf |= req_op(rq); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_vecs, req_op(rq)); while (iov_iter_count(iter)) { struct page **pages; @@ -303,7 +305,8 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, out_unmap: bio_release_pages(bio, false); - bio_put(bio); + bio_uninit(bio); + kfree(bio); return ret; } @@ -323,7 +326,8 @@ static void bio_invalidate_vmalloc_pages(struct bio *bio) static void bio_map_kern_endio(struct bio *bio) { bio_invalidate_vmalloc_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); } /** @@ -348,9 +352,10 @@ static struct bio *bio_map_kern(struct request_queue *q, void *data, int offset, i; struct bio *bio; - bio = bio_kmalloc(gfp_mask, nr_pages); + bio = bio_kmalloc(nr_pages, gfp_mask); if (!bio) return ERR_PTR(-ENOMEM); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_pages, 0); if (is_vmalloc) { flush_kernel_vmap_range(data, len); @@ -374,7 +379,8 @@ static struct bio *bio_map_kern(struct request_queue *q, void *data, if (bio_add_pc_page(q, bio, page, bytes, offset) < bytes) { /* we don't support partial mappings */ - bio_put(bio); + bio_uninit(bio); + kfree(bio); return ERR_PTR(-EINVAL); } @@ -390,7 +396,8 @@ static struct bio *bio_map_kern(struct request_queue *q, void *data, static void bio_copy_kern_endio(struct bio *bio) { bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); } static void bio_copy_kern_endio_read(struct bio *bio) @@ -435,9 +442,10 @@ static struct bio *bio_copy_kern(struct request_queue *q, void *data, return ERR_PTR(-EINVAL); nr_pages = end - start; - bio = bio_kmalloc(gfp_mask, nr_pages); + bio = bio_kmalloc(nr_pages, gfp_mask); if (!bio) return ERR_PTR(-ENOMEM); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_pages, 0); while (len) { struct page *page; @@ -471,7 +479,8 @@ static struct bio *bio_copy_kern(struct request_queue *q, void *data, cleanup: bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); return ERR_PTR(-ENOMEM); } @@ -602,7 +611,8 @@ int blk_rq_unmap_user(struct bio *bio) next_bio = bio; bio = bio->bi_next; - bio_put(next_bio); + bio_uninit(next_bio); + kfree(next_bio); } return ret; @@ -648,8 +658,10 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf, bio->bi_opf |= req_op(rq); ret = blk_rq_append_bio(rq, bio); - if (unlikely(ret)) - bio_put(bio); + if (unlikely(ret)) { + bio_uninit(bio); + kfree(bio); + } return ret; } EXPORT_SYMBOL(blk_rq_map_kern); diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index e745fc29e55d8..5ab43f9027631 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -522,9 +522,10 @@ static struct packet_data *pkt_alloc_packet_data(int frames) goto no_pkt; pkt->frames = frames; - pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames); + pkt->w_bio = bio_kmalloc(frames, GFP_KERNEL); if (!pkt->w_bio) goto no_bio; + bio_init(pkt->w_bio, NULL, pkt->w_bio->bi_inline_vecs, frames, 0); for (i = 0; i < frames / FRAMES_PER_PAGE; i++) { pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO); @@ -536,10 +537,10 @@ static struct packet_data *pkt_alloc_packet_data(int frames) bio_list_init(&pkt->orig_bios); for (i = 0; i < frames; i++) { - struct bio *bio = bio_kmalloc(GFP_KERNEL, 1); + struct bio *bio = bio_kmalloc(1, GFP_KERNEL); if (!bio) goto no_rd_bio; - + bio_init(bio, NULL, bio->bi_inline_vecs, 1, 0); pkt->r_bios[i] = bio; } @@ -547,16 +548,16 @@ static struct packet_data *pkt_alloc_packet_data(int frames) no_rd_bio: for (i = 0; i < frames; i++) { - struct bio *bio = pkt->r_bios[i]; - if (bio) - bio_put(bio); + if (pkt->r_bios[i]) + bio_uninit(pkt->r_bios[i]); + kfree(pkt->r_bios[i]); } - no_page: for (i = 0; i < frames / FRAMES_PER_PAGE; i++) if (pkt->pages[i]) __free_page(pkt->pages[i]); - bio_put(pkt->w_bio); + bio_uninit(pkt->w_bio); + kfree(pkt->w_bio); no_bio: kfree(pkt); no_pkt: @@ -571,13 +572,13 @@ static void pkt_free_packet_data(struct packet_data *pkt) int i; for (i = 0; i < pkt->frames; i++) { - struct bio *bio = pkt->r_bios[i]; - if (bio) - bio_put(bio); + bio_uninit(pkt->r_bios[i]); + kfree(pkt->r_bios[i]); } for (i = 0; i < pkt->frames / FRAMES_PER_PAGE; i++) __free_page(pkt->pages[i]); - bio_put(pkt->w_bio); + bio_uninit(pkt->w_bio); + kfree(pkt->w_bio); kfree(pkt); } diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c index 6230dfdd9286e..7510d1c983a5e 100644 --- a/drivers/md/bcache/debug.c +++ b/drivers/md/bcache/debug.c @@ -107,15 +107,16 @@ void bch_btree_verify(struct btree *b) void bch_data_verify(struct cached_dev *dc, struct bio *bio) { + unsigned int nr_segs = bio_segments(bio); struct bio *check; struct bio_vec bv, cbv; struct bvec_iter iter, citer = { 0 }; - check = bio_kmalloc(GFP_NOIO, bio_segments(bio)); + check = bio_kmalloc(nr_segs, GFP_NOIO); if (!check) return; - bio_set_dev(check, bio->bi_bdev); - check->bi_opf = REQ_OP_READ; + bio_init(check, bio->bi_bdev, check->bi_inline_vecs, nr_segs, + REQ_OP_READ); check->bi_iter.bi_sector = bio->bi_iter.bi_sector; check->bi_iter.bi_size = bio->bi_iter.bi_size; @@ -146,7 +147,8 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) bio_free_pages(check); out_put: - bio_put(check); + bio_uninit(check); + kfree(check); } #endif diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index e9cbc70d5a0ee..5ffa1dcf84cfc 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -611,7 +611,8 @@ static void bio_complete(struct bio *bio) { struct dm_buffer *b = bio->bi_private; blk_status_t status = bio->bi_status; - bio_put(bio); + bio_uninit(bio); + kfree(bio); b->end_io(b, status); } @@ -626,16 +627,14 @@ static void use_bio(struct dm_buffer *b, int rw, sector_t sector, if (unlikely(b->c->sectors_per_block_bits < PAGE_SHIFT - SECTOR_SHIFT)) vec_size += 2; - bio = bio_kmalloc(GFP_NOWAIT | __GFP_NORETRY | __GFP_NOWARN, vec_size); + bio = bio_kmalloc(vec_size, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOWARN); if (!bio) { dmio: use_dmio(b, rw, sector, n_sectors, offset); return; } - + bio_init(bio, b->c->bdev, bio->bi_inline_vecs, vec_size, rw); bio->bi_iter.bi_sector = sector; - bio_set_dev(bio, b->c->bdev); - bio_set_op_attrs(bio, rw, 0); bio->bi_end_io = bio_complete; bio->bi_private = b; diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index 03477e971699b..60cf3ef6457c4 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -165,9 +165,10 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data) * Allocate bios : 1 for reading, n-1 for writing */ for (j = pi->raid_disks ; j-- ; ) { - bio = bio_kmalloc(gfp_flags, RESYNC_PAGES); + bio = bio_kmalloc(RESYNC_PAGES, gfp_flags); if (!bio) goto out_free_bio; + bio_init(bio, NULL, bio->bi_inline_vecs, RESYNC_PAGES, 0); r1_bio->bios[j] = bio; } /* @@ -206,8 +207,10 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data) resync_free_pages(&rps[j]); out_free_bio: - while (++j < pi->raid_disks) - bio_put(r1_bio->bios[j]); + while (++j < pi->raid_disks) { + bio_uninit(r1_bio->bios[j]); + kfree(r1_bio->bios[j]); + } kfree(rps); out_free_r1bio: @@ -225,7 +228,8 @@ static void r1buf_pool_free(void *__r1_bio, void *data) for (i = pi->raid_disks; i--; ) { rp = get_resync_pages(r1bio->bios[i]); resync_free_pages(rp); - bio_put(r1bio->bios[i]); + bio_uninit(r1bio->bios[i]); + kfree(r1bio->bios[i]); } /* resync pages array stored in the 1st bio's .bi_private */ diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 5dd2e17e1d0ea..781b7b8a4b558 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -145,15 +145,17 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data) * Allocate bios. */ for (j = nalloc ; j-- ; ) { - bio = bio_kmalloc(gfp_flags, RESYNC_PAGES); + bio = bio_kmalloc(RESYNC_PAGES, gfp_flags); if (!bio) goto out_free_bio; + bio_init(bio, NULL, bio->bi_inline_vecs, RESYNC_PAGES, 0); r10_bio->devs[j].bio = bio; if (!conf->have_replacement) continue; - bio = bio_kmalloc(gfp_flags, RESYNC_PAGES); + bio = bio_kmalloc(RESYNC_PAGES, gfp_flags); if (!bio) goto out_free_bio; + bio_init(bio, NULL, bio->bi_inline_vecs, RESYNC_PAGES, 0); r10_bio->devs[j].repl_bio = bio; } /* @@ -197,9 +199,11 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data) out_free_bio: for ( ; j < nalloc; j++) { if (r10_bio->devs[j].bio) - bio_put(r10_bio->devs[j].bio); + bio_uninit(r10_bio->devs[j].bio); + kfree(r10_bio->devs[j].bio); if (r10_bio->devs[j].repl_bio) - bio_put(r10_bio->devs[j].repl_bio); + bio_uninit(r10_bio->devs[j].repl_bio); + kfree(r10_bio->devs[j].repl_bio); } kfree(rps); out_free_r10bio: @@ -220,12 +224,15 @@ static void r10buf_pool_free(void *__r10_bio, void *data) if (bio) { rp = get_resync_pages(bio); resync_free_pages(rp); - bio_put(bio); + bio_uninit(bio); + kfree(bio); } bio = r10bio->devs[j].repl_bio; - if (bio) - bio_put(bio); + if (bio) { + bio_uninit(bio); + kfree(bio); + } } /* resync pages array stored in the 1st bio's .bi_private */ diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index bd8ae07273f14..edda1a89d9357 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -820,7 +820,8 @@ static ssize_t pscsi_show_configfs_dev_params(struct se_device *dev, char *b) static void pscsi_bi_endio(struct bio *bio) { - bio_put(bio); + bio_uninit(bio); + kfree(bio); } static sense_reason_t @@ -863,14 +864,13 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, if (!bio) { new_bio: nr_vecs = bio_max_segs(nr_pages); - bio = bio_kmalloc(GFP_KERNEL, nr_vecs); + bio = bio_kmalloc(nr_vecs, GFP_KERNEL); if (!bio) goto fail; + bio_init(bio, NULL, bio->bi_inline_vecs, nr_vecs, + rw ? REQ_OP_WRITE : REQ_OP_READ); bio->bi_end_io = pscsi_bi_endio; - if (rw) - bio_set_op_attrs(bio, REQ_OP_WRITE, 0); - pr_debug("PSCSI: Allocated bio: %p," " dir: %s nr_vecs: %d\n", bio, (rw) ? "rw" : "r", nr_vecs); diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 4311a32218928..930eb530fa622 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -86,12 +86,10 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, int error, i; struct bio *bio; - bio = bio_kmalloc(GFP_NOIO, page_count); + bio = bio_kmalloc(page_count, GFP_NOIO); if (!bio) return -ENOMEM; - bio_set_dev(bio, sb->s_bdev); - bio->bi_opf = REQ_OP_READ; - + bio_init(bio, sb->s_bdev, bio->bi_inline_vecs, page_count, REQ_OP_READ); bio->bi_iter.bi_sector = block * (msblk->devblksize >> SECTOR_SHIFT); for (i = 0; i < page_count; ++i) { @@ -121,7 +119,8 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, out_free_bio: bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); return error; } diff --git a/include/linux/bio.h b/include/linux/bio.h index 4c21f6e69e182..e855bcf52fa7e 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -410,7 +410,7 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs, struct bio_set *bs); struct bio *bio_alloc_kiocb(struct kiocb *kiocb, struct block_device *bdev, unsigned short nr_vecs, unsigned int opf, struct bio_set *bs); -struct bio *bio_kmalloc(gfp_t gfp_mask, unsigned short nr_iovecs); +struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask); extern void bio_put(struct bio *); struct bio *bio_alloc_clone(struct block_device *bdev, struct bio *bio_src, -- 2.30.2 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [dm-devel] [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs Remove the magic autofree semantics and require the callers to explicitly call bio_init to initialize the bio. This allows bio_free to catch accidental bio_put calls on bio_init()ed bios as well. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bio.c | 47 ++++++++++++------------------ block/blk-crypto-fallback.c | 14 +++++---- block/blk-map.c | 42 ++++++++++++++++---------- drivers/block/pktcdvd.c | 25 ++++++++-------- drivers/md/bcache/debug.c | 10 ++++--- drivers/md/dm-bufio.c | 9 +++--- drivers/md/raid1.c | 12 +++++--- drivers/md/raid10.c | 21 ++++++++----- drivers/target/target_core_pscsi.c | 10 +++---- fs/squashfs/block.c | 9 +++--- include/linux/bio.h | 2 +- 11 files changed, 108 insertions(+), 93 deletions(-) diff --git a/block/bio.c b/block/bio.c index 151cace2dbe16..1fe243e61e55d 100644 --- a/block/bio.c +++ b/block/bio.c @@ -224,24 +224,13 @@ EXPORT_SYMBOL(bio_uninit); static void bio_free(struct bio *bio) { struct bio_set *bs = bio->bi_pool; - void *p; - - bio_uninit(bio); + void *p = bio; - if (bs) { - bvec_free(&bs->bvec_pool, bio->bi_io_vec, bio->bi_max_vecs); + WARN_ON_ONCE(!bs); - /* - * If we have front padding, adjust the bio pointer before freeing - */ - p = bio; - p -= bs->front_pad; - - mempool_free(p, &bs->bio_pool); - } else { - /* Bio was allocated by bio_kmalloc() */ - kfree(bio); - } + bio_uninit(bio); + bvec_free(&bs->bvec_pool, bio->bi_io_vec, bio->bi_max_vecs); + mempool_free(p - bs->front_pad, &bs->bio_pool); } /* @@ -529,28 +518,28 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs, EXPORT_SYMBOL(bio_alloc_bioset); /** - * bio_kmalloc - kmalloc a bio for I/O + * bio_kmalloc - kmalloc a bio + * @nr_vecs: number of bio_vecs to allocate * @gfp_mask: the GFP_* mask given to the slab allocator - * @nr_iovecs: number of iovecs to pre-allocate * - * Use kmalloc to allocate and initialize a bio. + * Use kmalloc to allocate a bio (including bvecs). The bio must be initialized + * using bio_init() before use. To free a bio returned from this function use + * kfree() after calling bio_uninit(). A bio returned from this function can + * be reused by calling bio_uninit() before calling bio_init() again. + * + * Note that unlike bio_alloc() or bio_alloc_bioset() allocations from this + * function are not backed by a mempool can can fail. Do not use this function + * for allocations in the file system I/O path. * * Returns: Pointer to new bio on success, NULL on failure. */ -struct bio *bio_kmalloc(gfp_t gfp_mask, unsigned short nr_iovecs) +struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask) { struct bio *bio; - if (nr_iovecs > UIO_MAXIOV) + if (nr_vecs > UIO_MAXIOV) return NULL; - - bio = kmalloc(struct_size(bio, bi_inline_vecs, nr_iovecs), gfp_mask); - if (unlikely(!bio)) - return NULL; - bio_init(bio, NULL, nr_iovecs ? bio->bi_inline_vecs : NULL, nr_iovecs, - 0); - bio->bi_pool = NULL; - return bio; + return kmalloc(struct_size(bio, bi_inline_vecs, nr_vecs), gfp_mask); } EXPORT_SYMBOL(bio_kmalloc); diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c index 18c8eafe20b94..1a624b75da94f 100644 --- a/block/blk-crypto-fallback.c +++ b/block/blk-crypto-fallback.c @@ -152,23 +152,25 @@ static void blk_crypto_fallback_encrypt_endio(struct bio *enc_bio) src_bio->bi_status = enc_bio->bi_status; - bio_put(enc_bio); + bio_uninit(enc_bio); + kfree(enc_bio); bio_endio(src_bio); } static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src) { + unsigned int nr_segs = bio_segments(bio_src); struct bvec_iter iter; struct bio_vec bv; struct bio *bio; - bio = bio_kmalloc(GFP_NOIO, bio_segments(bio_src)); + bio = bio_kmalloc(nr_segs, GFP_NOIO); if (!bio) return NULL; - bio->bi_bdev = bio_src->bi_bdev; + bio_init(bio, bio_src->bi_bdev, bio->bi_inline_vecs, nr_segs, + bio_src->bi_opf); if (bio_flagged(bio_src, BIO_REMAPPED)) bio_set_flag(bio, BIO_REMAPPED); - bio->bi_opf = bio_src->bi_opf; bio->bi_ioprio = bio_src->bi_ioprio; bio->bi_write_hint = bio_src->bi_write_hint; bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; @@ -364,8 +366,8 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr) blk_crypto_put_keyslot(slot); out_put_enc_bio: if (enc_bio) - bio_put(enc_bio); - + bio_uninit(enc_bio); + kfree(enc_bio); return ret; } diff --git a/block/blk-map.c b/block/blk-map.c index 4526adde01564..59be60e69e7af 100644 --- a/block/blk-map.c +++ b/block/blk-map.c @@ -152,10 +152,10 @@ static int bio_copy_user_iov(struct request *rq, struct rq_map_data *map_data, nr_pages = bio_max_segs(DIV_ROUND_UP(offset + len, PAGE_SIZE)); ret = -ENOMEM; - bio = bio_kmalloc(gfp_mask, nr_pages); + bio = bio_kmalloc(nr_pages, gfp_mask); if (!bio) goto out_bmd; - bio->bi_opf |= req_op(rq); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_pages, req_op(rq)); if (map_data) { nr_pages = 1 << map_data->page_order; @@ -224,7 +224,8 @@ static int bio_copy_user_iov(struct request *rq, struct rq_map_data *map_data, cleanup: if (!map_data) bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); out_bmd: kfree(bmd); return ret; @@ -234,6 +235,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, gfp_t gfp_mask) { unsigned int max_sectors = queue_max_hw_sectors(rq->q); + unsigned int nr_vecs = iov_iter_npages(iter, BIO_MAX_VECS); struct bio *bio; int ret; int j; @@ -241,10 +243,10 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, if (!iov_iter_count(iter)) return -EINVAL; - bio = bio_kmalloc(gfp_mask, iov_iter_npages(iter, BIO_MAX_VECS)); + bio = bio_kmalloc(nr_vecs, gfp_mask); if (!bio) return -ENOMEM; - bio->bi_opf |= req_op(rq); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_vecs, req_op(rq)); while (iov_iter_count(iter)) { struct page **pages; @@ -303,7 +305,8 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, out_unmap: bio_release_pages(bio, false); - bio_put(bio); + bio_uninit(bio); + kfree(bio); return ret; } @@ -323,7 +326,8 @@ static void bio_invalidate_vmalloc_pages(struct bio *bio) static void bio_map_kern_endio(struct bio *bio) { bio_invalidate_vmalloc_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); } /** @@ -348,9 +352,10 @@ static struct bio *bio_map_kern(struct request_queue *q, void *data, int offset, i; struct bio *bio; - bio = bio_kmalloc(gfp_mask, nr_pages); + bio = bio_kmalloc(nr_pages, gfp_mask); if (!bio) return ERR_PTR(-ENOMEM); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_pages, 0); if (is_vmalloc) { flush_kernel_vmap_range(data, len); @@ -374,7 +379,8 @@ static struct bio *bio_map_kern(struct request_queue *q, void *data, if (bio_add_pc_page(q, bio, page, bytes, offset) < bytes) { /* we don't support partial mappings */ - bio_put(bio); + bio_uninit(bio); + kfree(bio); return ERR_PTR(-EINVAL); } @@ -390,7 +396,8 @@ static struct bio *bio_map_kern(struct request_queue *q, void *data, static void bio_copy_kern_endio(struct bio *bio) { bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); } static void bio_copy_kern_endio_read(struct bio *bio) @@ -435,9 +442,10 @@ static struct bio *bio_copy_kern(struct request_queue *q, void *data, return ERR_PTR(-EINVAL); nr_pages = end - start; - bio = bio_kmalloc(gfp_mask, nr_pages); + bio = bio_kmalloc(nr_pages, gfp_mask); if (!bio) return ERR_PTR(-ENOMEM); + bio_init(bio, NULL, bio->bi_inline_vecs, nr_pages, 0); while (len) { struct page *page; @@ -471,7 +479,8 @@ static struct bio *bio_copy_kern(struct request_queue *q, void *data, cleanup: bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); return ERR_PTR(-ENOMEM); } @@ -602,7 +611,8 @@ int blk_rq_unmap_user(struct bio *bio) next_bio = bio; bio = bio->bi_next; - bio_put(next_bio); + bio_uninit(next_bio); + kfree(next_bio); } return ret; @@ -648,8 +658,10 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf, bio->bi_opf |= req_op(rq); ret = blk_rq_append_bio(rq, bio); - if (unlikely(ret)) - bio_put(bio); + if (unlikely(ret)) { + bio_uninit(bio); + kfree(bio); + } return ret; } EXPORT_SYMBOL(blk_rq_map_kern); diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index e745fc29e55d8..5ab43f9027631 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -522,9 +522,10 @@ static struct packet_data *pkt_alloc_packet_data(int frames) goto no_pkt; pkt->frames = frames; - pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames); + pkt->w_bio = bio_kmalloc(frames, GFP_KERNEL); if (!pkt->w_bio) goto no_bio; + bio_init(pkt->w_bio, NULL, pkt->w_bio->bi_inline_vecs, frames, 0); for (i = 0; i < frames / FRAMES_PER_PAGE; i++) { pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO); @@ -536,10 +537,10 @@ static struct packet_data *pkt_alloc_packet_data(int frames) bio_list_init(&pkt->orig_bios); for (i = 0; i < frames; i++) { - struct bio *bio = bio_kmalloc(GFP_KERNEL, 1); + struct bio *bio = bio_kmalloc(1, GFP_KERNEL); if (!bio) goto no_rd_bio; - + bio_init(bio, NULL, bio->bi_inline_vecs, 1, 0); pkt->r_bios[i] = bio; } @@ -547,16 +548,16 @@ static struct packet_data *pkt_alloc_packet_data(int frames) no_rd_bio: for (i = 0; i < frames; i++) { - struct bio *bio = pkt->r_bios[i]; - if (bio) - bio_put(bio); + if (pkt->r_bios[i]) + bio_uninit(pkt->r_bios[i]); + kfree(pkt->r_bios[i]); } - no_page: for (i = 0; i < frames / FRAMES_PER_PAGE; i++) if (pkt->pages[i]) __free_page(pkt->pages[i]); - bio_put(pkt->w_bio); + bio_uninit(pkt->w_bio); + kfree(pkt->w_bio); no_bio: kfree(pkt); no_pkt: @@ -571,13 +572,13 @@ static void pkt_free_packet_data(struct packet_data *pkt) int i; for (i = 0; i < pkt->frames; i++) { - struct bio *bio = pkt->r_bios[i]; - if (bio) - bio_put(bio); + bio_uninit(pkt->r_bios[i]); + kfree(pkt->r_bios[i]); } for (i = 0; i < pkt->frames / FRAMES_PER_PAGE; i++) __free_page(pkt->pages[i]); - bio_put(pkt->w_bio); + bio_uninit(pkt->w_bio); + kfree(pkt->w_bio); kfree(pkt); } diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c index 6230dfdd9286e..7510d1c983a5e 100644 --- a/drivers/md/bcache/debug.c +++ b/drivers/md/bcache/debug.c @@ -107,15 +107,16 @@ void bch_btree_verify(struct btree *b) void bch_data_verify(struct cached_dev *dc, struct bio *bio) { + unsigned int nr_segs = bio_segments(bio); struct bio *check; struct bio_vec bv, cbv; struct bvec_iter iter, citer = { 0 }; - check = bio_kmalloc(GFP_NOIO, bio_segments(bio)); + check = bio_kmalloc(nr_segs, GFP_NOIO); if (!check) return; - bio_set_dev(check, bio->bi_bdev); - check->bi_opf = REQ_OP_READ; + bio_init(check, bio->bi_bdev, check->bi_inline_vecs, nr_segs, + REQ_OP_READ); check->bi_iter.bi_sector = bio->bi_iter.bi_sector; check->bi_iter.bi_size = bio->bi_iter.bi_size; @@ -146,7 +147,8 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) bio_free_pages(check); out_put: - bio_put(check); + bio_uninit(check); + kfree(check); } #endif diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index e9cbc70d5a0ee..5ffa1dcf84cfc 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -611,7 +611,8 @@ static void bio_complete(struct bio *bio) { struct dm_buffer *b = bio->bi_private; blk_status_t status = bio->bi_status; - bio_put(bio); + bio_uninit(bio); + kfree(bio); b->end_io(b, status); } @@ -626,16 +627,14 @@ static void use_bio(struct dm_buffer *b, int rw, sector_t sector, if (unlikely(b->c->sectors_per_block_bits < PAGE_SHIFT - SECTOR_SHIFT)) vec_size += 2; - bio = bio_kmalloc(GFP_NOWAIT | __GFP_NORETRY | __GFP_NOWARN, vec_size); + bio = bio_kmalloc(vec_size, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOWARN); if (!bio) { dmio: use_dmio(b, rw, sector, n_sectors, offset); return; } - + bio_init(bio, b->c->bdev, bio->bi_inline_vecs, vec_size, rw); bio->bi_iter.bi_sector = sector; - bio_set_dev(bio, b->c->bdev); - bio_set_op_attrs(bio, rw, 0); bio->bi_end_io = bio_complete; bio->bi_private = b; diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index 03477e971699b..60cf3ef6457c4 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -165,9 +165,10 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data) * Allocate bios : 1 for reading, n-1 for writing */ for (j = pi->raid_disks ; j-- ; ) { - bio = bio_kmalloc(gfp_flags, RESYNC_PAGES); + bio = bio_kmalloc(RESYNC_PAGES, gfp_flags); if (!bio) goto out_free_bio; + bio_init(bio, NULL, bio->bi_inline_vecs, RESYNC_PAGES, 0); r1_bio->bios[j] = bio; } /* @@ -206,8 +207,10 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data) resync_free_pages(&rps[j]); out_free_bio: - while (++j < pi->raid_disks) - bio_put(r1_bio->bios[j]); + while (++j < pi->raid_disks) { + bio_uninit(r1_bio->bios[j]); + kfree(r1_bio->bios[j]); + } kfree(rps); out_free_r1bio: @@ -225,7 +228,8 @@ static void r1buf_pool_free(void *__r1_bio, void *data) for (i = pi->raid_disks; i--; ) { rp = get_resync_pages(r1bio->bios[i]); resync_free_pages(rp); - bio_put(r1bio->bios[i]); + bio_uninit(r1bio->bios[i]); + kfree(r1bio->bios[i]); } /* resync pages array stored in the 1st bio's .bi_private */ diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 5dd2e17e1d0ea..781b7b8a4b558 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -145,15 +145,17 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data) * Allocate bios. */ for (j = nalloc ; j-- ; ) { - bio = bio_kmalloc(gfp_flags, RESYNC_PAGES); + bio = bio_kmalloc(RESYNC_PAGES, gfp_flags); if (!bio) goto out_free_bio; + bio_init(bio, NULL, bio->bi_inline_vecs, RESYNC_PAGES, 0); r10_bio->devs[j].bio = bio; if (!conf->have_replacement) continue; - bio = bio_kmalloc(gfp_flags, RESYNC_PAGES); + bio = bio_kmalloc(RESYNC_PAGES, gfp_flags); if (!bio) goto out_free_bio; + bio_init(bio, NULL, bio->bi_inline_vecs, RESYNC_PAGES, 0); r10_bio->devs[j].repl_bio = bio; } /* @@ -197,9 +199,11 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data) out_free_bio: for ( ; j < nalloc; j++) { if (r10_bio->devs[j].bio) - bio_put(r10_bio->devs[j].bio); + bio_uninit(r10_bio->devs[j].bio); + kfree(r10_bio->devs[j].bio); if (r10_bio->devs[j].repl_bio) - bio_put(r10_bio->devs[j].repl_bio); + bio_uninit(r10_bio->devs[j].repl_bio); + kfree(r10_bio->devs[j].repl_bio); } kfree(rps); out_free_r10bio: @@ -220,12 +224,15 @@ static void r10buf_pool_free(void *__r10_bio, void *data) if (bio) { rp = get_resync_pages(bio); resync_free_pages(rp); - bio_put(bio); + bio_uninit(bio); + kfree(bio); } bio = r10bio->devs[j].repl_bio; - if (bio) - bio_put(bio); + if (bio) { + bio_uninit(bio); + kfree(bio); + } } /* resync pages array stored in the 1st bio's .bi_private */ diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index bd8ae07273f14..edda1a89d9357 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -820,7 +820,8 @@ static ssize_t pscsi_show_configfs_dev_params(struct se_device *dev, char *b) static void pscsi_bi_endio(struct bio *bio) { - bio_put(bio); + bio_uninit(bio); + kfree(bio); } static sense_reason_t @@ -863,14 +864,13 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, if (!bio) { new_bio: nr_vecs = bio_max_segs(nr_pages); - bio = bio_kmalloc(GFP_KERNEL, nr_vecs); + bio = bio_kmalloc(nr_vecs, GFP_KERNEL); if (!bio) goto fail; + bio_init(bio, NULL, bio->bi_inline_vecs, nr_vecs, + rw ? REQ_OP_WRITE : REQ_OP_READ); bio->bi_end_io = pscsi_bi_endio; - if (rw) - bio_set_op_attrs(bio, REQ_OP_WRITE, 0); - pr_debug("PSCSI: Allocated bio: %p," " dir: %s nr_vecs: %d\n", bio, (rw) ? "rw" : "r", nr_vecs); diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 4311a32218928..930eb530fa622 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -86,12 +86,10 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, int error, i; struct bio *bio; - bio = bio_kmalloc(GFP_NOIO, page_count); + bio = bio_kmalloc(page_count, GFP_NOIO); if (!bio) return -ENOMEM; - bio_set_dev(bio, sb->s_bdev); - bio->bi_opf = REQ_OP_READ; - + bio_init(bio, sb->s_bdev, bio->bi_inline_vecs, page_count, REQ_OP_READ); bio->bi_iter.bi_sector = block * (msblk->devblksize >> SECTOR_SHIFT); for (i = 0; i < page_count; ++i) { @@ -121,7 +119,8 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, out_free_bio: bio_free_pages(bio); - bio_put(bio); + bio_uninit(bio); + kfree(bio); return error; } diff --git a/include/linux/bio.h b/include/linux/bio.h index 4c21f6e69e182..e855bcf52fa7e 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -410,7 +410,7 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs, struct bio_set *bs); struct bio *bio_alloc_kiocb(struct kiocb *kiocb, struct block_device *bdev, unsigned short nr_vecs, unsigned int opf, struct bio_set *bs); -struct bio *bio_kmalloc(gfp_t gfp_mask, unsigned short nr_iovecs); +struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask); extern void bio_put(struct bio *); struct bio *bio_alloc_clone(struct block_device *bdev, struct bio *bio_src, -- 2.30.2 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-09 3:06 ` Martin K. Petersen -1 siblings, 0 replies; 38+ messages in thread From: Martin K. Petersen @ 2022-03-09 3:06 UTC (permalink / raw) To: Christoph Hellwig Cc: Jens Axboe, Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Christoph, > Remove the magic autofree semantics and require the callers to explicitly > call bio_init to initialize the bio. > > This allows bio_free to catch accidental bio_put calls on bio_init()ed > bios as well. > -struct bio *bio_kmalloc(gfp_t gfp_mask, unsigned short nr_iovecs); > +struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask); I understand why you did it but this parameter reversal is a bit scary. Hopefully gfp_t will cause any mistakes to be flagged. Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper @ 2022-03-09 3:06 ` Martin K. Petersen 0 siblings, 0 replies; 38+ messages in thread From: Martin K. Petersen @ 2022-03-09 3:06 UTC (permalink / raw) To: Christoph Hellwig Cc: Jens Axboe, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-bcache, linux-btrfs Christoph, > Remove the magic autofree semantics and require the callers to explicitly > call bio_init to initialize the bio. > > This allows bio_free to catch accidental bio_put calls on bio_init()ed > bios as well. > -struct bio *bio_kmalloc(gfp_t gfp_mask, unsigned short nr_iovecs); > +struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask); I understand why you did it but this parameter reversal is a bit scary. Hopefully gfp_t will cause any mistakes to be flagged. Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> -- Martin K. Petersen Oracle Linux Engineering -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
[parent not found: <CGME20220331211804eucas1p28da21f2dfd57aa490abffb8f87417f42@eucas1p2.samsung.com>]
* Re: [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper [not found] ` <CGME20220331211804eucas1p28da21f2dfd57aa490abffb8f87417f42@eucas1p2.samsung.com> @ 2022-03-31 21:18 ` Marek Szyprowski 0 siblings, 0 replies; 38+ messages in thread From: Marek Szyprowski @ 2022-03-31 21:18 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Hi Christoph, On 08.03.2022 07:15, Christoph Hellwig wrote: > Remove the magic autofree semantics and require the callers to explicitly > call bio_init to initialize the bio. > > This allows bio_free to catch accidental bio_put calls on bio_init()ed > bios as well. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This patch, which landed in today's next-20220331 as commit 57c47b42f454 ("block: turn bio_kmalloc into a simple kmalloc wrapper"), breaks badly all my test systems, which use squashfs initrd: RAMDISK: squashfs filesystem found at block 0 RAMDISK: Loading 2489KiB [1 disk] into ram disk... done. using deprecated initrd support, will be removed in 2021. ------------[ cut here ]------------ WARNING: CPU: 4 PID: 1 at block/bio.c:229 bio_free+0x6c/0x70 Modules linked in: CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.17.0-next-20220331 #4767 Hardware name: Samsung Exynos (Flattened Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x58/0x70 dump_stack_lvl from __warn+0xc8/0x218 __warn from warn_slowpath_fmt+0x5c/0xb4 warn_slowpath_fmt from bio_free+0x6c/0x70 bio_free from squashfs_read_data+0x118/0x748 squashfs_read_data from squashfs_read_table+0xdc/0x144 squashfs_read_table from squashfs_fill_super+0x100/0x9ec squashfs_fill_super from get_tree_bdev+0x154/0x248 get_tree_bdev from vfs_get_tree+0x24/0xe4 vfs_get_tree from path_mount+0x3d0/0xb14 path_mount from init_mount+0x54/0x80 init_mount from do_mount_root+0x78/0x104 do_mount_root from mount_block_root+0xf0/0x1fc mount_block_root from initrd_load+0xec/0x294 initrd_load from prepare_namespace+0xdc/0x18c prepare_namespace from kernel_init+0x18/0x12c kernel_init from ret_from_fork+0x14/0x2c Exception stack(0xf0835fb0 to 0xf0835ff8) ... irq event stamp: 398271 hardirqs last enabled at (398279): [<c019c984>] __up_console_sem+0x50/0x60 hardirqs last disabled at (398338): [<c019c970>] __up_console_sem+0x3c/0x60 softirqs last enabled at (398352): [<c0101680>] __do_softirq+0x348/0x610 softirqs last disabled at (398347): [<c012f048>] __irq_exit_rcu+0x144/0x1ec ---[ end trace 0000000000000000 ]--- 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000004 [00000004] *pgd=00000000 Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 4 PID: 1 Comm: swapper/0 Tainted: G W 5.17.0-next-20220331 #4767 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at bio_free+0x24/0x70 LR is at bio_free+0x24/0x70 pc : [<c0502d28>] lr : [<c0502d28>] psr: 80000113 sp : f0835cf0 ip : 00000000 fp : c28cae80 r10: ef0a95c0 r9 : c2805cc0 r8 : 00000060 r7 : 00000060 r6 : 00000060 r5 : 00000000 r4 : c2804a80 r3 : c2804ac8 r2 : 00000001 r1 : c2804ac8 r0 : 00000074 Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000406a DAC: 00000051 Register r0 information: non-paged memory Register r1 information: slab kmalloc-128 start c2804a80 pointer offset 72 size 128 Register r2 information: non-paged memory Register r3 information: slab kmalloc-128 start c2804a80 pointer offset 72 size 128 Register r4 information: slab kmalloc-128 start c2804a80 pointer offset 0 size 128 Register r5 information: NULL pointer Register r6 information: non-paged memory Register r7 information: non-paged memory Register r8 information: non-paged memory Register r9 information: slab kmalloc-192 start c2805cc0 pointer offset 0 size 192 Register r10 information: non-slab/vmalloc memory Register r11 information: slab kmalloc-64 start c28cae80 pointer offset 0 size 64 Register r12 information: NULL pointer Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xf0835cf0 to 0xf0836000) ... bio_free from squashfs_read_data+0x118/0x748 squashfs_read_data from squashfs_read_table+0xdc/0x144 squashfs_read_table from squashfs_fill_super+0x100/0x9ec squashfs_fill_super from get_tree_bdev+0x154/0x248 get_tree_bdev from vfs_get_tree+0x24/0xe4 vfs_get_tree from path_mount+0x3d0/0xb14 path_mount from init_mount+0x54/0x80 init_mount from do_mount_root+0x78/0x104 do_mount_root from mount_block_root+0xf0/0x1fc mount_block_root from initrd_load+0xec/0x294 initrd_load from prepare_namespace+0xdc/0x18c prepare_namespace from kernel_init+0x18/0x12c kernel_init from ret_from_fork+0x14/0x2c Exception stack(0xf0835fb0 to 0xf0835ff8) ... ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b Reverting it on top of linux next-20220331 (together with commit 1292fb59f283 ("pktcdvd: stop using bio_reset")) fixes (or hides?) the issue. > --- > block/bio.c | 47 ++++++++++++------------------ > block/blk-crypto-fallback.c | 14 +++++---- > block/blk-map.c | 42 ++++++++++++++++---------- > drivers/block/pktcdvd.c | 25 ++++++++-------- > drivers/md/bcache/debug.c | 10 ++++--- > drivers/md/dm-bufio.c | 9 +++--- > drivers/md/raid1.c | 12 +++++--- > drivers/md/raid10.c | 21 ++++++++----- > drivers/target/target_core_pscsi.c | 10 +++---- > fs/squashfs/block.c | 9 +++--- > include/linux/bio.h | 2 +- > 11 files changed, 108 insertions(+), 93 deletions(-) > [...] Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper @ 2022-03-31 21:18 ` Marek Szyprowski 0 siblings, 0 replies; 38+ messages in thread From: Marek Szyprowski @ 2022-03-31 21:18 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs Hi Christoph, On 08.03.2022 07:15, Christoph Hellwig wrote: > Remove the magic autofree semantics and require the callers to explicitly > call bio_init to initialize the bio. > > This allows bio_free to catch accidental bio_put calls on bio_init()ed > bios as well. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This patch, which landed in today's next-20220331 as commit 57c47b42f454 ("block: turn bio_kmalloc into a simple kmalloc wrapper"), breaks badly all my test systems, which use squashfs initrd: RAMDISK: squashfs filesystem found at block 0 RAMDISK: Loading 2489KiB [1 disk] into ram disk... done. using deprecated initrd support, will be removed in 2021. ------------[ cut here ]------------ WARNING: CPU: 4 PID: 1 at block/bio.c:229 bio_free+0x6c/0x70 Modules linked in: CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.17.0-next-20220331 #4767 Hardware name: Samsung Exynos (Flattened Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x58/0x70 dump_stack_lvl from __warn+0xc8/0x218 __warn from warn_slowpath_fmt+0x5c/0xb4 warn_slowpath_fmt from bio_free+0x6c/0x70 bio_free from squashfs_read_data+0x118/0x748 squashfs_read_data from squashfs_read_table+0xdc/0x144 squashfs_read_table from squashfs_fill_super+0x100/0x9ec squashfs_fill_super from get_tree_bdev+0x154/0x248 get_tree_bdev from vfs_get_tree+0x24/0xe4 vfs_get_tree from path_mount+0x3d0/0xb14 path_mount from init_mount+0x54/0x80 init_mount from do_mount_root+0x78/0x104 do_mount_root from mount_block_root+0xf0/0x1fc mount_block_root from initrd_load+0xec/0x294 initrd_load from prepare_namespace+0xdc/0x18c prepare_namespace from kernel_init+0x18/0x12c kernel_init from ret_from_fork+0x14/0x2c Exception stack(0xf0835fb0 to 0xf0835ff8) ... irq event stamp: 398271 hardirqs last enabled at (398279): [<c019c984>] __up_console_sem+0x50/0x60 hardirqs last disabled at (398338): [<c019c970>] __up_console_sem+0x3c/0x60 softirqs last enabled at (398352): [<c0101680>] __do_softirq+0x348/0x610 softirqs last disabled at (398347): [<c012f048>] __irq_exit_rcu+0x144/0x1ec ---[ end trace 0000000000000000 ]--- 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000004 [00000004] *pgd=00000000 Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 4 PID: 1 Comm: swapper/0 Tainted: G W 5.17.0-next-20220331 #4767 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at bio_free+0x24/0x70 LR is at bio_free+0x24/0x70 pc : [<c0502d28>] lr : [<c0502d28>] psr: 80000113 sp : f0835cf0 ip : 00000000 fp : c28cae80 r10: ef0a95c0 r9 : c2805cc0 r8 : 00000060 r7 : 00000060 r6 : 00000060 r5 : 00000000 r4 : c2804a80 r3 : c2804ac8 r2 : 00000001 r1 : c2804ac8 r0 : 00000074 Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000406a DAC: 00000051 Register r0 information: non-paged memory Register r1 information: slab kmalloc-128 start c2804a80 pointer offset 72 size 128 Register r2 information: non-paged memory Register r3 information: slab kmalloc-128 start c2804a80 pointer offset 72 size 128 Register r4 information: slab kmalloc-128 start c2804a80 pointer offset 0 size 128 Register r5 information: NULL pointer Register r6 information: non-paged memory Register r7 information: non-paged memory Register r8 information: non-paged memory Register r9 information: slab kmalloc-192 start c2805cc0 pointer offset 0 size 192 Register r10 information: non-slab/vmalloc memory Register r11 information: slab kmalloc-64 start c28cae80 pointer offset 0 size 64 Register r12 information: NULL pointer Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xf0835cf0 to 0xf0836000) ... bio_free from squashfs_read_data+0x118/0x748 squashfs_read_data from squashfs_read_table+0xdc/0x144 squashfs_read_table from squashfs_fill_super+0x100/0x9ec squashfs_fill_super from get_tree_bdev+0x154/0x248 get_tree_bdev from vfs_get_tree+0x24/0xe4 vfs_get_tree from path_mount+0x3d0/0xb14 path_mount from init_mount+0x54/0x80 init_mount from do_mount_root+0x78/0x104 do_mount_root from mount_block_root+0xf0/0x1fc mount_block_root from initrd_load+0xec/0x294 initrd_load from prepare_namespace+0xdc/0x18c prepare_namespace from kernel_init+0x18/0x12c kernel_init from ret_from_fork+0x14/0x2c Exception stack(0xf0835fb0 to 0xf0835ff8) ... ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b Reverting it on top of linux next-20220331 (together with commit 1292fb59f283 ("pktcdvd: stop using bio_reset")) fixes (or hides?) the issue. > --- > block/bio.c | 47 ++++++++++++------------------ > block/blk-crypto-fallback.c | 14 +++++---- > block/blk-map.c | 42 ++++++++++++++++---------- > drivers/block/pktcdvd.c | 25 ++++++++-------- > drivers/md/bcache/debug.c | 10 ++++--- > drivers/md/dm-bufio.c | 9 +++--- > drivers/md/raid1.c | 12 +++++--- > drivers/md/raid10.c | 21 ++++++++----- > drivers/target/target_core_pscsi.c | 10 +++---- > fs/squashfs/block.c | 9 +++--- > include/linux/bio.h | 2 +- > 11 files changed, 108 insertions(+), 93 deletions(-) > [...] Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper 2022-03-31 21:18 ` [dm-devel] " Marek Szyprowski @ 2022-03-31 21:22 ` Jens Axboe -1 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-31 21:22 UTC (permalink / raw) To: Marek Szyprowski, Christoph Hellwig Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On 3/31/22 3:18 PM, Marek Szyprowski wrote: > Hi Christoph, > > On 08.03.2022 07:15, Christoph Hellwig wrote: >> Remove the magic autofree semantics and require the callers to explicitly >> call bio_init to initialize the bio. >> >> This allows bio_free to catch accidental bio_put calls on bio_init()ed >> bios as well. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > This patch, which landed in today's next-20220331 as commit 57c47b42f454 > ("block: turn bio_kmalloc into a simple kmalloc wrapper"), breaks badly > all my test systems, which use squashfs initrd: The series has been reverted on the block side, so next linux-next should be fine again. We'll try again for 5.19. -- Jens Axboe ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper @ 2022-03-31 21:22 ` Jens Axboe 0 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-31 21:22 UTC (permalink / raw) To: Marek Szyprowski, Christoph Hellwig Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs On 3/31/22 3:18 PM, Marek Szyprowski wrote: > Hi Christoph, > > On 08.03.2022 07:15, Christoph Hellwig wrote: >> Remove the magic autofree semantics and require the callers to explicitly >> call bio_init to initialize the bio. >> >> This allows bio_free to catch accidental bio_put calls on bio_init()ed >> bios as well. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > This patch, which landed in today's next-20220331 as commit 57c47b42f454 > ("block: turn bio_kmalloc into a simple kmalloc wrapper"), breaks badly > all my test systems, which use squashfs initrd: The series has been reverted on the block side, so next linux-next should be fine again. We'll try again for 5.19. -- Jens Axboe -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper 2022-03-31 21:18 ` [dm-devel] " Marek Szyprowski @ 2022-04-01 4:57 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-04-01 4:57 UTC (permalink / raw) To: Marek Szyprowski Cc: Jens Axboe, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-bcache, Christoph Hellwig, linux-btrfs On Thu, Mar 31, 2022 at 11:18:03PM +0200, Marek Szyprowski wrote: > Hi Christoph, > > On 08.03.2022 07:15, Christoph Hellwig wrote: > > Remove the magic autofree semantics and require the callers to explicitly > > call bio_init to initialize the bio. > > > > This allows bio_free to catch accidental bio_put calls on bio_init()ed > > bios as well. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > This patch, which landed in today's next-20220331 as commit 57c47b42f454 > ("block: turn bio_kmalloc into a simple kmalloc wrapper"), breaks badly > all my test systems, which use squashfs initrd: In addition to the revert, this is the patch I had already queued up: diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 930eb530fa622..fed99bb3df3be 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -72,6 +72,13 @@ static int copy_bio_to_actor(struct bio *bio, return copied_bytes; } +static void squashfs_bio_free(struct bio *bio) +{ + bio_free_pages(bio); + bio_uninit(bio); + kfree(bio); +} + static int squashfs_bio_read(struct super_block *sb, u64 index, int length, struct bio **biop, int *block_offset) { @@ -118,9 +125,7 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, return 0; out_free_bio: - bio_free_pages(bio); - bio_uninit(bio); - kfree(bio); + squashfs_bio_free(bio); return error; } @@ -183,8 +188,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, data = bvec_virt(bvec); length |= data[0] << 8; } - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); compressed = SQUASHFS_COMPRESSED(length); length = SQUASHFS_COMPRESSED_SIZE(length); @@ -217,8 +221,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, } out_free_bio: - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); out: if (res < 0) { ERROR("Failed to read block 0x%llx: %d\n", index, res); -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper @ 2022-04-01 4:57 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-04-01 4:57 UTC (permalink / raw) To: Marek Szyprowski Cc: Christoph Hellwig, Jens Axboe, Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On Thu, Mar 31, 2022 at 11:18:03PM +0200, Marek Szyprowski wrote: > Hi Christoph, > > On 08.03.2022 07:15, Christoph Hellwig wrote: > > Remove the magic autofree semantics and require the callers to explicitly > > call bio_init to initialize the bio. > > > > This allows bio_free to catch accidental bio_put calls on bio_init()ed > > bios as well. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > This patch, which landed in today's next-20220331 as commit 57c47b42f454 > ("block: turn bio_kmalloc into a simple kmalloc wrapper"), breaks badly > all my test systems, which use squashfs initrd: In addition to the revert, this is the patch I had already queued up: diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 930eb530fa622..fed99bb3df3be 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -72,6 +72,13 @@ static int copy_bio_to_actor(struct bio *bio, return copied_bytes; } +static void squashfs_bio_free(struct bio *bio) +{ + bio_free_pages(bio); + bio_uninit(bio); + kfree(bio); +} + static int squashfs_bio_read(struct super_block *sb, u64 index, int length, struct bio **biop, int *block_offset) { @@ -118,9 +125,7 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, return 0; out_free_bio: - bio_free_pages(bio); - bio_uninit(bio); - kfree(bio); + squashfs_bio_free(bio); return error; } @@ -183,8 +188,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, data = bvec_virt(bvec); length |= data[0] << 8; } - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); compressed = SQUASHFS_COMPRESSED(length); length = SQUASHFS_COMPRESSED_SIZE(length); @@ -217,8 +221,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, } out_free_bio: - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); out: if (res < 0) { ERROR("Failed to read block 0x%llx: %d\n", index, res); ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 5/5] pktcdvd: stop using bio_reset 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 6:15 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs Just initialize the bios on-demand. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/block/pktcdvd.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 5ab43f9027631..335099c4076ee 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -525,7 +525,6 @@ static struct packet_data *pkt_alloc_packet_data(int frames) pkt->w_bio = bio_kmalloc(frames, GFP_KERNEL); if (!pkt->w_bio) goto no_bio; - bio_init(pkt->w_bio, NULL, pkt->w_bio->bi_inline_vecs, frames, 0); for (i = 0; i < frames / FRAMES_PER_PAGE; i++) { pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO); @@ -537,26 +536,20 @@ static struct packet_data *pkt_alloc_packet_data(int frames) bio_list_init(&pkt->orig_bios); for (i = 0; i < frames; i++) { - struct bio *bio = bio_kmalloc(1, GFP_KERNEL); - if (!bio) + pkt->r_bios[i] = bio_kmalloc(1, GFP_KERNEL); + if (!pkt->r_bios[i]) goto no_rd_bio; - bio_init(bio, NULL, bio->bi_inline_vecs, 1, 0); - pkt->r_bios[i] = bio; } return pkt; no_rd_bio: - for (i = 0; i < frames; i++) { - if (pkt->r_bios[i]) - bio_uninit(pkt->r_bios[i]); + for (i = 0; i < frames; i++) kfree(pkt->r_bios[i]); - } no_page: for (i = 0; i < frames / FRAMES_PER_PAGE; i++) if (pkt->pages[i]) __free_page(pkt->pages[i]); - bio_uninit(pkt->w_bio); kfree(pkt->w_bio); no_bio: kfree(pkt); @@ -571,13 +564,10 @@ static void pkt_free_packet_data(struct packet_data *pkt) { int i; - for (i = 0; i < pkt->frames; i++) { - bio_uninit(pkt->r_bios[i]); + for (i = 0; i < pkt->frames; i++) kfree(pkt->r_bios[i]); - } for (i = 0; i < pkt->frames / FRAMES_PER_PAGE; i++) __free_page(pkt->pages[i]); - bio_uninit(pkt->w_bio); kfree(pkt->w_bio); kfree(pkt); } @@ -950,6 +940,7 @@ static void pkt_end_io_read(struct bio *bio) if (bio->bi_status) atomic_inc(&pkt->io_errors); + bio_uninit(bio); if (atomic_dec_and_test(&pkt->io_wait)) { atomic_inc(&pkt->run_sm); wake_up(&pd->wqueue); @@ -967,6 +958,7 @@ static void pkt_end_io_packet_write(struct bio *bio) pd->stats.pkt_ended++; + bio_uninit(bio); pkt_bio_finished(pd); atomic_dec(&pkt->io_wait); atomic_inc(&pkt->run_sm); @@ -1021,7 +1013,7 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt) continue; bio = pkt->r_bios[f]; - bio_reset(bio, pd->bdev, REQ_OP_READ); + bio_init(bio, pd->bdev, bio->bi_inline_vecs, 1, REQ_OP_READ); bio->bi_iter.bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9); bio->bi_end_io = pkt_end_io_read; bio->bi_private = pkt; @@ -1234,7 +1226,8 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt) { int f; - bio_reset(pkt->w_bio, pd->bdev, REQ_OP_WRITE); + bio_init(pkt->w_bio, pd->bdev, pkt->w_bio->bi_inline_vecs, pkt->frames, + REQ_OP_WRITE); pkt->w_bio->bi_iter.bi_sector = pkt->sector; pkt->w_bio->bi_end_io = pkt_end_io_packet_write; pkt->w_bio->bi_private = pkt; -- 2.30.2 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [dm-devel] [PATCH 5/5] pktcdvd: stop using bio_reset @ 2022-03-08 6:15 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-08 6:15 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs Just initialize the bios on-demand. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/block/pktcdvd.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 5ab43f9027631..335099c4076ee 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -525,7 +525,6 @@ static struct packet_data *pkt_alloc_packet_data(int frames) pkt->w_bio = bio_kmalloc(frames, GFP_KERNEL); if (!pkt->w_bio) goto no_bio; - bio_init(pkt->w_bio, NULL, pkt->w_bio->bi_inline_vecs, frames, 0); for (i = 0; i < frames / FRAMES_PER_PAGE; i++) { pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO); @@ -537,26 +536,20 @@ static struct packet_data *pkt_alloc_packet_data(int frames) bio_list_init(&pkt->orig_bios); for (i = 0; i < frames; i++) { - struct bio *bio = bio_kmalloc(1, GFP_KERNEL); - if (!bio) + pkt->r_bios[i] = bio_kmalloc(1, GFP_KERNEL); + if (!pkt->r_bios[i]) goto no_rd_bio; - bio_init(bio, NULL, bio->bi_inline_vecs, 1, 0); - pkt->r_bios[i] = bio; } return pkt; no_rd_bio: - for (i = 0; i < frames; i++) { - if (pkt->r_bios[i]) - bio_uninit(pkt->r_bios[i]); + for (i = 0; i < frames; i++) kfree(pkt->r_bios[i]); - } no_page: for (i = 0; i < frames / FRAMES_PER_PAGE; i++) if (pkt->pages[i]) __free_page(pkt->pages[i]); - bio_uninit(pkt->w_bio); kfree(pkt->w_bio); no_bio: kfree(pkt); @@ -571,13 +564,10 @@ static void pkt_free_packet_data(struct packet_data *pkt) { int i; - for (i = 0; i < pkt->frames; i++) { - bio_uninit(pkt->r_bios[i]); + for (i = 0; i < pkt->frames; i++) kfree(pkt->r_bios[i]); - } for (i = 0; i < pkt->frames / FRAMES_PER_PAGE; i++) __free_page(pkt->pages[i]); - bio_uninit(pkt->w_bio); kfree(pkt->w_bio); kfree(pkt); } @@ -950,6 +940,7 @@ static void pkt_end_io_read(struct bio *bio) if (bio->bi_status) atomic_inc(&pkt->io_errors); + bio_uninit(bio); if (atomic_dec_and_test(&pkt->io_wait)) { atomic_inc(&pkt->run_sm); wake_up(&pd->wqueue); @@ -967,6 +958,7 @@ static void pkt_end_io_packet_write(struct bio *bio) pd->stats.pkt_ended++; + bio_uninit(bio); pkt_bio_finished(pd); atomic_dec(&pkt->io_wait); atomic_inc(&pkt->run_sm); @@ -1021,7 +1013,7 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt) continue; bio = pkt->r_bios[f]; - bio_reset(bio, pd->bdev, REQ_OP_READ); + bio_init(bio, pd->bdev, bio->bi_inline_vecs, 1, REQ_OP_READ); bio->bi_iter.bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9); bio->bi_end_io = pkt_end_io_read; bio->bi_private = pkt; @@ -1234,7 +1226,8 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt) { int f; - bio_reset(pkt->w_bio, pd->bdev, REQ_OP_WRITE); + bio_init(pkt->w_bio, pd->bdev, pkt->w_bio->bi_inline_vecs, pkt->frames, + REQ_OP_WRITE); pkt->w_bio->bi_iter.bi_sector = pkt->sector; pkt->w_bio->bi_end_io = pkt_end_io_packet_write; pkt->w_bio->bi_private = pkt; -- 2.30.2 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 5/5] pktcdvd: stop using bio_reset 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-08 22:10 ` Chaitanya Kulkarni -1 siblings, 0 replies; 38+ messages in thread From: Chaitanya Kulkarni @ 2022-03-08 22:10 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On 3/7/22 22:15, Christoph Hellwig wrote: > Just initialize the bios on-demand. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] [PATCH 5/5] pktcdvd: stop using bio_reset @ 2022-03-08 22:10 ` Chaitanya Kulkarni 0 siblings, 0 replies; 38+ messages in thread From: Chaitanya Kulkarni @ 2022-03-08 22:10 UTC (permalink / raw) To: Christoph Hellwig, Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs On 3/7/22 22:15, Christoph Hellwig wrote: > Just initialize the bios on-demand. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: cleanup bio_kmalloc v2 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-30 14:29 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-30 14:29 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs I just noticed this didn't make it into the 5.18 queue. Which is a bit sad as it leaves us with a rather inconsistent bio API in 5.18. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-30 14:29 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-30 14:29 UTC (permalink / raw) To: Jens Axboe Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs I just noticed this didn't make it into the 5.18 queue. Which is a bit sad as it leaves us with a rather inconsistent bio API in 5.18. -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: cleanup bio_kmalloc v2 2022-03-30 14:29 ` [dm-devel] " Christoph Hellwig @ 2022-03-30 14:37 ` Jens Axboe -1 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-30 14:37 UTC (permalink / raw) To: Christoph Hellwig Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On 3/30/22 8:29 AM, Christoph Hellwig wrote: > I just noticed this didn't make it into the 5.18 queue. Which is a > bit sad as it leaves us with a rather inconsistent bio API in 5.18. Let me take a look, we might still be able to make it... -- Jens Axboe ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-30 14:37 ` Jens Axboe 0 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-30 14:37 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs On 3/30/22 8:29 AM, Christoph Hellwig wrote: > I just noticed this didn't make it into the 5.18 queue. Which is a > bit sad as it leaves us with a rather inconsistent bio API in 5.18. Let me take a look, we might still be able to make it... -- Jens Axboe -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: cleanup bio_kmalloc v2 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-30 14:49 ` Jens Axboe -1 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-30 14:49 UTC (permalink / raw) To: Christoph Hellwig Cc: David Sterba, linux-btrfs, linux-bcache, dm-devel, linux-kernel, Song Liu, Coly Li, Phillip Lougher, Martin K. Petersen, Mike Snitzer, target-devel, Josef Bacik, linux-raid, linux-block On Tue, 8 Mar 2022 07:15:46 +0100, Christoph Hellwig wrote: > this series finishes off the bio allocation interface cleanups by dealing > with the weirdest member of the famility. bio_kmalloc combines a kmalloc > for the bio and bio_vecs with a hidden bio_init call and magic cleanup > semantics. > > This series moves a few callers away from bio_kmalloc and then turns > bio_kmalloc into a simple wrapper for a slab allocation of a bio and the > inline biovecs. The callers need to manually call bio_init instead with > all that entails and the magic that turns bio_put into a kfree goes away > as well, allowing for a proper debug check in bio_put that catches > accidental use on a bio_init()ed bio. > > [...] Applied, thanks! [1/5] btrfs: simplify ->flush_bio handling commit: 6978ffddd5bba44e6b7614d52868cf4954e0529b [2/5] squashfs: always use bio_kmalloc in squashfs_bio_read commit: 88a39feabf25efbaec775ffb48ea240af198994e [3/5] target/pscsi: remove pscsi_get_bio commit: bbccc65bd7c1b22f050b65d8171fbdd8d72cf39c [4/5] block: turn bio_kmalloc into a simple kmalloc wrapper commit: 57c47b42f4545b5f8fa288f190c0d68f96bc477f [5/5] pktcdvd: stop using bio_reset commit: 1292fb59f283e76f55843d94f066c2f0b91dfb7e Best regards, -- Jens Axboe ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-30 14:49 ` Jens Axboe 0 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-30 14:49 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-block, Mike Snitzer, Martin K. Petersen, dm-devel, linux-kernel, linux-raid, Song Liu, linux-bcache, target-devel, David Sterba, Phillip Lougher, Josef Bacik, Coly Li, linux-btrfs On Tue, 8 Mar 2022 07:15:46 +0100, Christoph Hellwig wrote: > this series finishes off the bio allocation interface cleanups by dealing > with the weirdest member of the famility. bio_kmalloc combines a kmalloc > for the bio and bio_vecs with a hidden bio_init call and magic cleanup > semantics. > > This series moves a few callers away from bio_kmalloc and then turns > bio_kmalloc into a simple wrapper for a slab allocation of a bio and the > inline biovecs. The callers need to manually call bio_init instead with > all that entails and the magic that turns bio_put into a kfree goes away > as well, allowing for a proper debug check in bio_put that catches > accidental use on a bio_init()ed bio. > > [...] Applied, thanks! [1/5] btrfs: simplify ->flush_bio handling commit: 6978ffddd5bba44e6b7614d52868cf4954e0529b [2/5] squashfs: always use bio_kmalloc in squashfs_bio_read commit: 88a39feabf25efbaec775ffb48ea240af198994e [3/5] target/pscsi: remove pscsi_get_bio commit: bbccc65bd7c1b22f050b65d8171fbdd8d72cf39c [4/5] block: turn bio_kmalloc into a simple kmalloc wrapper commit: 57c47b42f4545b5f8fa288f190c0d68f96bc477f [5/5] pktcdvd: stop using bio_reset commit: 1292fb59f283e76f55843d94f066c2f0b91dfb7e Best regards, -- Jens Axboe -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: cleanup bio_kmalloc v2 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig @ 2022-03-31 16:34 ` Qian Cai -1 siblings, 0 replies; 38+ messages in thread From: Qian Cai @ 2022-03-31 16:34 UTC (permalink / raw) To: Christoph Hellwig Cc: Jens Axboe, Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On Tue, Mar 08, 2022 at 07:15:46AM +0100, Christoph Hellwig wrote: > Hi Jens, > > this series finishes off the bio allocation interface cleanups by dealing > with the weirdest member of the famility. bio_kmalloc combines a kmalloc > for the bio and bio_vecs with a hidden bio_init call and magic cleanup > semantics. > > This series moves a few callers away from bio_kmalloc and then turns > bio_kmalloc into a simple wrapper for a slab allocation of a bio and the > inline biovecs. The callers need to manually call bio_init instead with > all that entails and the magic that turns bio_put into a kfree goes away > as well, allowing for a proper debug check in bio_put that catches > accidental use on a bio_init()ed bio. Reverting this series fixed boot crashes. WARNING: CPU: 1 PID: 2622 at block/bio.c:229 bio_free Modules linked in: cdc_ether usbnet ipmi_devintf ipmi_msghandler cppc_cpufreq fuse ip_tables x_tables ipv6 btrfs blake2b_generic libcrc32c xor xor_neon raid6_pq zstd_compress dm_mod nouveau crct10dif_ce drm_ttm_helper mlx5_core ttm drm_dp_helper drm_kms_helper nvme mpt3sas xhci_pci nvme_core raid_class drm xhci_pci_renesas CPU: 1 PID: 2622 Comm: mount Not tainted 5.17.0-next-20220331 #50 pstate: 10400009 (nzcV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : bio_free lr : bio_put sp : ffff8000371b7760 x29: ffff8000371b7760 x28: 0000000000000000 x27: dfff800000000000 x26: ffff08028f93a600 x25: 0000000000000000 x24: ffff08028f92f600 x23: 1ffff00006e36f10 x22: ffff08028fa18510 x21: 1fffe10051f430a2 x20: 0000000000000000 x19: ffff08028fa18500 x18: ffffde03db3e7d2c x17: ffffde03d55f8bc4 x16: 1fffe10051e75129 x15: 1fffe106cfcfbb46 x14: 1fffe10051e7511c x13: 0000000000000004 x12: ffff700006e36eab x11: 1ffff00006e36eaa x10: ffff700006e36eaa x9 : ffffde03d5cb9fec x8 : ffff8000371b7557 x7 : 0000000000000001 x6 : ffff700006e36eaa x5 : 1ffff00006e36ea9 x4 : 1ffff00006e36ebe x3 : 1fffe10051f430a2 x2 : 1fffe10051f430ae x1 : 0000000000000000 x0 : ffff08028fa18570 Call trace: bio_free bio_put squashfs_read_data squashfs_read_table squashfs_fill_super get_tree_bdev squashfs_get_tree vfs_get_tree do_new_mount path_mount __arm64_sys_mount invoke_syscall el0_svc_common.constprop.0 do_el0_svc el0_svc el0t_64_sync_handler el0t_64_sync irq event stamp: 33146 hardirqs last enabled at (33145): free_unref_page hardirqs last disabled at (33146): el1_dbg softirqs last enabled at (33122): __do_softirq softirqs last disabled at (33111): __irq_exit_rcu ---[ end trace 0000000000000000 ]--- Unable to handle kernel paging request at virtual address dfff800000000001 KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f] Mem abort info: ESR = 0x96000004 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x04: level 0 translation fault Data abort info: ISV = 0, ISS = 0x00000004 CM = 0, WnR = 0 [dfff800000000001] address between user and kernel address ranges Internal error: Oops: 96000004 [#1] PREEMPT SMP Modules linked in: cdc_ether usbnet ipmi_devintf ipmi_msghandler cppc_cpufreq fuse ip_tables x_tables ipv6 btrfs blake2b_generic libcrc32c xor xor_neon raid6_pq zstd_compress dm_mod nouveau crct10dif_ce drm_ttm_helper mlx5_core ttm drm_dp_helper drm_kms_helper nvme mpt3sas xhci_pci nvme_core raid_class drm xhci_pci_renesas CPU: 1 PID: 2622 Comm: mount Tainted: G W 5.17.0-next-20220331 #50 pc : bio_free lr : bio_free sp : ffff8000371b7760 x29: ffff8000371b7760 x28: 0000000000000000 x27: dfff800000000000 x26: ffff08028f93a600 x25: 0000000000000000 x24: ffff08028f92f600 x23: 1ffff00006e36f10 x22: ffff08028fa18548 x21: 00000000000000d0 x20: 0000000000000000 x19: ffff08028fa18500 x18: ffffde03db3e7d2c x17: ffffde03d55f8bc4 x16: 1fffe10051e75129 x15: 1fffe106cfcfbb46 x14: 1fffe10051e7511c x13: 0000000000000004 x12: ffff700006e36eab x11: 1ffff00006e36eaa x10: ffff700006e36eaa x9 : ffffde03d5cb9c78 x8 : ffff8000371b7557 x7 : 0000000000000001 x6 : ffff700006e36eaa x5 : 1ffff00006e36ea9 x4 : 1fffe10051f430ac x3 : 0000000000000001 x2 : 0000000000000003 x1 : dfff800000000000 x0 : 0000000000000008 Call trace: bio_free bio_put squashfs_read_data squashfs_read_table squashfs_fill_super get_tree_bdev squashfs_get_tree vfs_get_tree do_new_mount path_mount __arm64_sys_mount invoke_syscall el0_svc_common.constprop.0 do_el0_svc el0_svc el0t_64_sync_handler el0t_64_sync Code: d2d00001 f2fbffe1 52800062 d343fc03 (38e16861) ---[ end trace 0000000000000000 ]--- SMP: stopping secondary CPUs Kernel Offset: 0x5e03ccd70000 from 0xffff800008000000 PHYS_OFFSET: 0x80000000 CPU features: 0x000,00085c0d,19801c82 Memory Limit: none ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > > Changes since v1: > - update a pre-existing comment per maintainer suggestion > > Diffstat: > block/bio.c | 47 ++++++++++++++----------------------- > block/blk-crypto-fallback.c | 14 ++++++----- > block/blk-map.c | 42 +++++++++++++++++++++------------ > drivers/block/pktcdvd.c | 34 +++++++++++--------------- > drivers/md/bcache/debug.c | 10 ++++--- > drivers/md/dm-bufio.c | 9 +++---- > drivers/md/raid1.c | 12 ++++++--- > drivers/md/raid10.c | 21 +++++++++++----- > drivers/target/target_core_pscsi.c | 36 ++++------------------------ > fs/btrfs/disk-io.c | 8 +++--- > fs/btrfs/volumes.c | 11 -------- > fs/btrfs/volumes.h | 2 - > fs/squashfs/block.c | 14 +++-------- > include/linux/bio.h | 2 - > 14 files changed, 116 insertions(+), 146 deletions(-) ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-31 16:34 ` Qian Cai 0 siblings, 0 replies; 38+ messages in thread From: Qian Cai @ 2022-03-31 16:34 UTC (permalink / raw) To: Christoph Hellwig Cc: Jens Axboe, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-bcache, linux-btrfs On Tue, Mar 08, 2022 at 07:15:46AM +0100, Christoph Hellwig wrote: > Hi Jens, > > this series finishes off the bio allocation interface cleanups by dealing > with the weirdest member of the famility. bio_kmalloc combines a kmalloc > for the bio and bio_vecs with a hidden bio_init call and magic cleanup > semantics. > > This series moves a few callers away from bio_kmalloc and then turns > bio_kmalloc into a simple wrapper for a slab allocation of a bio and the > inline biovecs. The callers need to manually call bio_init instead with > all that entails and the magic that turns bio_put into a kfree goes away > as well, allowing for a proper debug check in bio_put that catches > accidental use on a bio_init()ed bio. Reverting this series fixed boot crashes. WARNING: CPU: 1 PID: 2622 at block/bio.c:229 bio_free Modules linked in: cdc_ether usbnet ipmi_devintf ipmi_msghandler cppc_cpufreq fuse ip_tables x_tables ipv6 btrfs blake2b_generic libcrc32c xor xor_neon raid6_pq zstd_compress dm_mod nouveau crct10dif_ce drm_ttm_helper mlx5_core ttm drm_dp_helper drm_kms_helper nvme mpt3sas xhci_pci nvme_core raid_class drm xhci_pci_renesas CPU: 1 PID: 2622 Comm: mount Not tainted 5.17.0-next-20220331 #50 pstate: 10400009 (nzcV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : bio_free lr : bio_put sp : ffff8000371b7760 x29: ffff8000371b7760 x28: 0000000000000000 x27: dfff800000000000 x26: ffff08028f93a600 x25: 0000000000000000 x24: ffff08028f92f600 x23: 1ffff00006e36f10 x22: ffff08028fa18510 x21: 1fffe10051f430a2 x20: 0000000000000000 x19: ffff08028fa18500 x18: ffffde03db3e7d2c x17: ffffde03d55f8bc4 x16: 1fffe10051e75129 x15: 1fffe106cfcfbb46 x14: 1fffe10051e7511c x13: 0000000000000004 x12: ffff700006e36eab x11: 1ffff00006e36eaa x10: ffff700006e36eaa x9 : ffffde03d5cb9fec x8 : ffff8000371b7557 x7 : 0000000000000001 x6 : ffff700006e36eaa x5 : 1ffff00006e36ea9 x4 : 1ffff00006e36ebe x3 : 1fffe10051f430a2 x2 : 1fffe10051f430ae x1 : 0000000000000000 x0 : ffff08028fa18570 Call trace: bio_free bio_put squashfs_read_data squashfs_read_table squashfs_fill_super get_tree_bdev squashfs_get_tree vfs_get_tree do_new_mount path_mount __arm64_sys_mount invoke_syscall el0_svc_common.constprop.0 do_el0_svc el0_svc el0t_64_sync_handler el0t_64_sync irq event stamp: 33146 hardirqs last enabled at (33145): free_unref_page hardirqs last disabled at (33146): el1_dbg softirqs last enabled at (33122): __do_softirq softirqs last disabled at (33111): __irq_exit_rcu ---[ end trace 0000000000000000 ]--- Unable to handle kernel paging request at virtual address dfff800000000001 KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f] Mem abort info: ESR = 0x96000004 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x04: level 0 translation fault Data abort info: ISV = 0, ISS = 0x00000004 CM = 0, WnR = 0 [dfff800000000001] address between user and kernel address ranges Internal error: Oops: 96000004 [#1] PREEMPT SMP Modules linked in: cdc_ether usbnet ipmi_devintf ipmi_msghandler cppc_cpufreq fuse ip_tables x_tables ipv6 btrfs blake2b_generic libcrc32c xor xor_neon raid6_pq zstd_compress dm_mod nouveau crct10dif_ce drm_ttm_helper mlx5_core ttm drm_dp_helper drm_kms_helper nvme mpt3sas xhci_pci nvme_core raid_class drm xhci_pci_renesas CPU: 1 PID: 2622 Comm: mount Tainted: G W 5.17.0-next-20220331 #50 pc : bio_free lr : bio_free sp : ffff8000371b7760 x29: ffff8000371b7760 x28: 0000000000000000 x27: dfff800000000000 x26: ffff08028f93a600 x25: 0000000000000000 x24: ffff08028f92f600 x23: 1ffff00006e36f10 x22: ffff08028fa18548 x21: 00000000000000d0 x20: 0000000000000000 x19: ffff08028fa18500 x18: ffffde03db3e7d2c x17: ffffde03d55f8bc4 x16: 1fffe10051e75129 x15: 1fffe106cfcfbb46 x14: 1fffe10051e7511c x13: 0000000000000004 x12: ffff700006e36eab x11: 1ffff00006e36eaa x10: ffff700006e36eaa x9 : ffffde03d5cb9c78 x8 : ffff8000371b7557 x7 : 0000000000000001 x6 : ffff700006e36eaa x5 : 1ffff00006e36ea9 x4 : 1fffe10051f430ac x3 : 0000000000000001 x2 : 0000000000000003 x1 : dfff800000000000 x0 : 0000000000000008 Call trace: bio_free bio_put squashfs_read_data squashfs_read_table squashfs_fill_super get_tree_bdev squashfs_get_tree vfs_get_tree do_new_mount path_mount __arm64_sys_mount invoke_syscall el0_svc_common.constprop.0 do_el0_svc el0_svc el0t_64_sync_handler el0t_64_sync Code: d2d00001 f2fbffe1 52800062 d343fc03 (38e16861) ---[ end trace 0000000000000000 ]--- SMP: stopping secondary CPUs Kernel Offset: 0x5e03ccd70000 from 0xffff800008000000 PHYS_OFFSET: 0x80000000 CPU features: 0x000,00085c0d,19801c82 Memory Limit: none ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > > Changes since v1: > - update a pre-existing comment per maintainer suggestion > > Diffstat: > block/bio.c | 47 ++++++++++++++----------------------- > block/blk-crypto-fallback.c | 14 ++++++----- > block/blk-map.c | 42 +++++++++++++++++++++------------ > drivers/block/pktcdvd.c | 34 +++++++++++--------------- > drivers/md/bcache/debug.c | 10 ++++--- > drivers/md/dm-bufio.c | 9 +++---- > drivers/md/raid1.c | 12 ++++++--- > drivers/md/raid10.c | 21 +++++++++++----- > drivers/target/target_core_pscsi.c | 36 ++++------------------------ > fs/btrfs/disk-io.c | 8 +++--- > fs/btrfs/volumes.c | 11 -------- > fs/btrfs/volumes.h | 2 - > fs/squashfs/block.c | 14 +++-------- > include/linux/bio.h | 2 - > 14 files changed, 116 insertions(+), 146 deletions(-) -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: cleanup bio_kmalloc v2 2022-03-31 16:34 ` [dm-devel] " Qian Cai @ 2022-03-31 16:40 ` Christoph Hellwig -1 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-31 16:40 UTC (permalink / raw) To: Qian Cai Cc: Christoph Hellwig, Jens Axboe, Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs This should fix it: diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 930eb530fa622..fed99bb3df3be 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -72,6 +72,13 @@ static int copy_bio_to_actor(struct bio *bio, return copied_bytes; } +static void squashfs_bio_free(struct bio *bio) +{ + bio_free_pages(bio); + bio_uninit(bio); + kfree(bio); +} + static int squashfs_bio_read(struct super_block *sb, u64 index, int length, struct bio **biop, int *block_offset) { @@ -118,9 +125,7 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, return 0; out_free_bio: - bio_free_pages(bio); - bio_uninit(bio); - kfree(bio); + squashfs_bio_free(bio); return error; } @@ -183,8 +188,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, data = bvec_virt(bvec); length |= data[0] << 8; } - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); compressed = SQUASHFS_COMPRESSED(length); length = SQUASHFS_COMPRESSED_SIZE(length); @@ -217,8 +221,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, } out_free_bio: - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); out: if (res < 0) { ERROR("Failed to read block 0x%llx: %d\n", index, res); ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-31 16:40 ` Christoph Hellwig 0 siblings, 0 replies; 38+ messages in thread From: Christoph Hellwig @ 2022-03-31 16:40 UTC (permalink / raw) To: Qian Cai Cc: Jens Axboe, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-bcache, Christoph Hellwig, linux-btrfs This should fix it: diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index 930eb530fa622..fed99bb3df3be 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -72,6 +72,13 @@ static int copy_bio_to_actor(struct bio *bio, return copied_bytes; } +static void squashfs_bio_free(struct bio *bio) +{ + bio_free_pages(bio); + bio_uninit(bio); + kfree(bio); +} + static int squashfs_bio_read(struct super_block *sb, u64 index, int length, struct bio **biop, int *block_offset) { @@ -118,9 +125,7 @@ static int squashfs_bio_read(struct super_block *sb, u64 index, int length, return 0; out_free_bio: - bio_free_pages(bio); - bio_uninit(bio); - kfree(bio); + squashfs_bio_free(bio); return error; } @@ -183,8 +188,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, data = bvec_virt(bvec); length |= data[0] << 8; } - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); compressed = SQUASHFS_COMPRESSED(length); length = SQUASHFS_COMPRESSED_SIZE(length); @@ -217,8 +221,7 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length, } out_free_bio: - bio_free_pages(bio); - bio_put(bio); + squashfs_bio_free(bio); out: if (res < 0) { ERROR("Failed to read block 0x%llx: %d\n", index, res); -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: cleanup bio_kmalloc v2 2022-03-31 16:40 ` [dm-devel] " Christoph Hellwig @ 2022-03-31 16:48 ` Jens Axboe -1 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-31 16:48 UTC (permalink / raw) To: Christoph Hellwig, Qian Cai Cc: Coly Li, Mike Snitzer, Song Liu, Martin K. Petersen, Josef Bacik, David Sterba, Phillip Lougher, linux-block, dm-devel, linux-kernel, linux-bcache, linux-raid, target-devel, linux-btrfs On 3/31/22 10:40 AM, Christoph Hellwig wrote: > This should fix it: Let's drop this one for 5.18, it's also causing a few conflicts and would probably be more suited for 5.19 at this point. -- Jens Axboe ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [dm-devel] cleanup bio_kmalloc v2 @ 2022-03-31 16:48 ` Jens Axboe 0 siblings, 0 replies; 38+ messages in thread From: Jens Axboe @ 2022-03-31 16:48 UTC (permalink / raw) To: Christoph Hellwig, Qian Cai Cc: linux-bcache, linux-raid, Mike Snitzer, Martin K. Petersen, linux-kernel, Josef Bacik, Coly Li, linux-block, Song Liu, dm-devel, target-devel, David Sterba, Phillip Lougher, linux-btrfs On 3/31/22 10:40 AM, Christoph Hellwig wrote: > This should fix it: Let's drop this one for 5.18, it's also causing a few conflicts and would probably be more suited for 5.19 at this point. -- Jens Axboe -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2022-04-04 6:48 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-08 6:15 cleanup bio_kmalloc v2 Christoph Hellwig 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig 2022-03-08 6:15 ` [PATCH 1/5] btrfs: simplify ->flush_bio handling Christoph Hellwig 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig 2022-03-08 22:07 ` Chaitanya Kulkarni 2022-03-08 22:07 ` [dm-devel] " Chaitanya Kulkarni 2022-03-08 6:15 ` [PATCH 2/5] squashfs: always use bio_kmalloc in squashfs_bio_read Christoph Hellwig 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig 2022-03-08 6:15 ` [PATCH 3/5] target/pscsi: remove pscsi_get_bio Christoph Hellwig 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig 2022-03-08 22:08 ` Chaitanya Kulkarni 2022-03-08 22:08 ` [dm-devel] " Chaitanya Kulkarni 2022-03-08 6:15 ` [PATCH 4/5] block: turn bio_kmalloc into a simple kmalloc wrapper Christoph Hellwig 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig 2022-03-09 3:06 ` Martin K. Petersen 2022-03-09 3:06 ` [dm-devel] " Martin K. Petersen [not found] ` <CGME20220331211804eucas1p28da21f2dfd57aa490abffb8f87417f42@eucas1p2.samsung.com> 2022-03-31 21:18 ` Marek Szyprowski 2022-03-31 21:18 ` [dm-devel] " Marek Szyprowski 2022-03-31 21:22 ` Jens Axboe 2022-03-31 21:22 ` [dm-devel] " Jens Axboe 2022-04-01 4:57 ` Christoph Hellwig 2022-04-01 4:57 ` Christoph Hellwig 2022-03-08 6:15 ` [PATCH 5/5] pktcdvd: stop using bio_reset Christoph Hellwig 2022-03-08 6:15 ` [dm-devel] " Christoph Hellwig 2022-03-08 22:10 ` Chaitanya Kulkarni 2022-03-08 22:10 ` [dm-devel] " Chaitanya Kulkarni 2022-03-30 14:29 ` cleanup bio_kmalloc v2 Christoph Hellwig 2022-03-30 14:29 ` [dm-devel] " Christoph Hellwig 2022-03-30 14:37 ` Jens Axboe 2022-03-30 14:37 ` [dm-devel] " Jens Axboe 2022-03-30 14:49 ` Jens Axboe 2022-03-30 14:49 ` [dm-devel] " Jens Axboe 2022-03-31 16:34 ` Qian Cai 2022-03-31 16:34 ` [dm-devel] " Qian Cai 2022-03-31 16:40 ` Christoph Hellwig 2022-03-31 16:40 ` [dm-devel] " Christoph Hellwig 2022-03-31 16:48 ` Jens Axboe 2022-03-31 16:48 ` [dm-devel] " Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.