[PATCHv2] blk-mq: set the nr_integrity_segments from bio

[PATCHv2] blk-mq: set the nr_integrity_segments from bio

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

This value is used for potential merging later.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v1->v2:

Check the bio actually has integrity before counting the segments. I
previously tested v1 with additional experimental patches atop that
addressed the problem differently and didn't notice the obvious API
requirement.

 block/blk-mq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 36abbaefe3874..3ed5181c75610 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2546,6 +2546,10 @@ static void blk_mq_bio_to_request(struct request *rq, struct bio *bio,
 	rq->__sector = bio->bi_iter.bi_sector;
 	rq->write_hint = bio->bi_write_hint;
 	blk_rq_bio_prep(rq, bio, nr_segs);
+#if defined(CONFIG_BLK_DEV_INTEGRITY)
+	if (bio->bi_opf & REQ_INTEGRITY)
+		rq->nr_integrity_segments = blk_rq_count_integrity_sg(rq->q, bio);
+#endif
 
 	/* This can't fail, since GFP_NOIO includes __GFP_DIRECT_RECLAIM. */
 	err = blk_crypto_rq_bio_prep(rq, bio, GFP_NOIO);
-- 
2.43.5

Re: [PATCHv2] blk-mq: set the nr_integrity_segments from bio

[Christoph Hellwig]

On Tue, Sep 03, 2024 at 12:13:25PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> This value is used for potential merging later.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v1->v2:
> 
> Check the bio actually has integrity before counting the segments. I
> previously tested v1 with additional experimental patches atop that
> addressed the problem differently and didn't notice the obvious API
> requirement.
> 
>  block/blk-mq.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 36abbaefe3874..3ed5181c75610 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2546,6 +2546,10 @@ static void blk_mq_bio_to_request(struct request *rq, struct bio *bio,
>  	rq->__sector = bio->bi_iter.bi_sector;
>  	rq->write_hint = bio->bi_write_hint;
>  	blk_rq_bio_prep(rq, bio, nr_segs);
> +#if defined(CONFIG_BLK_DEV_INTEGRITY)
> +	if (bio->bi_opf & REQ_INTEGRITY)
> +		rq->nr_integrity_segments = blk_rq_count_integrity_sg(rq->q, bio);

Hmm.  The current model is that drivers are supposed to
clal this, which they should stop doing now that the block layer
maintains this count.  So I think this needs a bit more work, after
which blk_rq_count_integrity_sg is also unexported, nad preferably
also has a name that drops the incorrect _sg.

Re: [PATCHv2] blk-mq: set the nr_integrity_segments from bio

[Keith Busch]

On Tue, Sep 03, 2024 at 09:34:55PM -0700, Christoph Hellwig wrote:
> On Tue, Sep 03, 2024 at 12:13:25PM -0700, Keith Busch wrote:
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -2546,6 +2546,10 @@ static void blk_mq_bio_to_request(struct request *rq, struct bio *bio,
> >  	rq->__sector = bio->bi_iter.bi_sector;
> >  	rq->write_hint = bio->bi_write_hint;
> >  	blk_rq_bio_prep(rq, bio, nr_segs);
> > +#if defined(CONFIG_BLK_DEV_INTEGRITY)
> > +	if (bio->bi_opf & REQ_INTEGRITY)
> > +		rq->nr_integrity_segments = blk_rq_count_integrity_sg(rq->q, bio);
> 
> Hmm.  The current model is that drivers are supposed to
> clal this, which they should stop doing now that the block layer
> maintains this count.  So I think this needs a bit more work, after
> which blk_rq_count_integrity_sg is also unexported, nad preferably
> also has a name that drops the incorrect _sg.

Sure, I have a larger patch set in the works doing pretty much that.
I wanted the request's existing field to be accurate first because
inaccurate values break merging. But I can post the rest of series if
you prefer to see drivers rely on the existing field instead of
recounting the segments.