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 v2:
- rebased to 5.18-rc1
- fix bio freeing in squashfs
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(-)
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 <[email protected]>
Acked-by: Phillip Lougher <[email protected]>
---
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
Just initialize the bios on-demand.
Signed-off-by: Christoph Hellwig <[email protected]>
---
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 4a5b8730133c5..f270080f14786 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);
}
@@ -952,6 +942,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);
@@ -969,6 +960,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);
@@ -1023,7 +1015,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;
@@ -1236,7 +1228,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
Remove pscsi_get_bio and simplify the code flow in the only caller.
Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Martin K. Petersen <[email protected]>
---
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 ff292b75e23f9..77bd3af4b2bf8 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -821,23 +821,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)
@@ -878,12 +861,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);
@@ -912,11 +893,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
Use and embedded bios that is initialized when used instead of
bio_kmalloc plus bio_reset.
Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: David Sterba <[email protected]>
---
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 b30309f187cf0..51f4cdf6bcaea 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4225,6 +4225,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);
}
@@ -4234,7 +4235,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
/*
@@ -4252,7 +4253,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;
@@ -4266,7 +4268,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 1be7cb2f955fc..00517239b5c01 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);
}
@@ -6956,16 +6955,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 bd297f23d19e7..628c9af894749 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -121,8 +121,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
On 4/5/22 23:12, Christoph Hellwig wrote:
> Remove pscsi_get_bio and simplify the code flow in the only caller.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Acked-by: Martin K. Petersen <[email protected]>
> ---
>
Looks good.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
-ck
On 4/5/22 23:12, Christoph Hellwig wrote:
> Use and embedded bios that is initialized when used instead of
> bio_kmalloc plus bio_reset.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Reviewed-by: David Sterba <[email protected]>
Looks good.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
-ck
On 4/5/22 23:12, Christoph Hellwig wrote:
> 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 <[email protected]>
> Acked-by: Phillip Lougher <[email protected]>
> ---
Looks good.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
-ck
Hi Jens,
can you pull this into 5.19/block? I'd like to send some bio cloning
optimizations for Mike on top of this series.
On Wed, 6 Apr 2022 08:12:23 +0200, 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: f9e69aa9ccd7e51c47b147e45e03987ea0ef9aa3
[2/5] squashfs: always use bio_kmalloc in squashfs_bio_read
commit: 46a2d4ccc49903923506685a8368ca88312bbdc9
[3/5] target/pscsi: remove pscsi_get_bio
commit: 7655db80932d95f501a0811544d9520ec720e38d
[4/5] block: turn bio_kmalloc into a simple kmalloc wrapper
commit: 066ff571011d8416e903d3d4f1f41e0b5eb91e1d
[5/5] pktcdvd: stop using bio_reset
commit: 852ad96cb03621f7995764b4b31cbff9801d8bcd
Best regards,
--
Jens Axboe