Re: [PATCH 3/6] block: move ->bio_split to the gendisk

[Ming Lei <ming.lei@redhat.com>]

On Thu, Jul 28, 2022 at 10:07:14AM +0200, Hannes Reinecke wrote:
> On 7/27/22 18:22, Christoph Hellwig wrote:
> > Only non-passthrough requests are split by the block layer and use the
> > ->bio_split bio_set.  Move it from the request_queue to the gendisk.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> > ---
> >   block/blk-core.c       | 9 +--------
> >   block/blk-merge.c      | 7 ++++---
> >   block/blk-sysfs.c      | 2 --
> >   block/genhd.c          | 8 +++++++-
> >   drivers/md/dm.c        | 2 +-
> >   include/linux/blkdev.h | 3 ++-
> >   6 files changed, 15 insertions(+), 16 deletions(-)
> > 
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index 123468b9d2e43..59f13d011949d 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -377,7 +377,6 @@ static void blk_timeout_work(struct work_struct *work)
> >   struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
> >   {
> >   	struct request_queue *q;
> > -	int ret;
> >   	q = kmem_cache_alloc_node(blk_get_queue_kmem_cache(alloc_srcu),
> >   			GFP_KERNEL | __GFP_ZERO, node_id);
> > @@ -396,13 +395,9 @@ struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
> >   	if (q->id < 0)
> >   		goto fail_srcu;
> > -	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, 0);
> > -	if (ret)
> > -		goto fail_id;
> > -
> >   	q->stats = blk_alloc_queue_stats();
> >   	if (!q->stats)
> > -		goto fail_split;
> > +		goto fail_id;
> >   	q->node = node_id;
> > @@ -439,8 +434,6 @@ struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
> >   fail_stats:
> >   	blk_free_queue_stats(q->stats);
> > -fail_split:
> > -	bioset_exit(&q->bio_split);
> >   fail_id:
> >   	ida_free(&blk_queue_ida, q->id);
> >   fail_srcu:
> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index 6e29fb28584ef..30872a3537648 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -331,18 +331,19 @@ static struct bio *bio_split_rw(struct bio *bio, struct request_queue *q,
> >   struct bio *__bio_split_to_limits(struct bio *bio, struct request_queue *q,
> >   		       unsigned int *nr_segs)
> >   {
> > +	struct bio_set *bs = &bio->bi_bdev->bd_disk->bio_split;
> >   	struct bio *split;
> 
> What happens for nvme-multipath?
> While I know that we shouldn't split on a path, experience shows that we
> _will_ do it eventually.

You mean contiguous bios should be mapped to one same path? If yes, at
least the current block layer can't do that, here dm-mapth is same with
nvme-mpath, since bio is always split first, and the split small bio
can be mapped to other path.



Thanks,
Ming