| Message ID | 20240903191325.3642403-1-kbusch@meta.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv2] blk-mq: set the nr_integrity_segments from bio | expand |
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.
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.
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);