[PATCHSET 0/3] Integrity cleanups and optimization

[PATCHSET 0/3] Integrity cleanups and optimization

[Jens Axboe]

Hi,

1 gets rid of the dummy nop profile, 2 marks the queue as having an
actual profile, and 3 avoids calling into bio_integrity_prep() if we
don't have a profile. This both reduces code (getting rid of the nop
profile) and reduces the overhead of the standard setup of having
integrity enabled in kconfig, yet not using a device with an integrity
profile.

 block/blk-integrity.c  | 38 ++++++++++----------------------------
 block/blk-mq.c         | 11 +++++++----
 include/linux/blkdev.h |  1 +
 3 files changed, 18 insertions(+), 32 deletions(-)

-- 
Jens Axboe

[PATCH 1/3] block/integrity: make profile optional

[Jens Axboe]

If no profile is given, we set a 'nop' profile. But we already check for
a NULL profile in most spots, so let's get rid of this wasted code. It's
also considerably faster to check for a NULL handler rather than have an
indirect call into a dummy function.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-integrity.c | 24 +-----------------------
 block/blk-mq.c        |  8 +++++---
 2 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index d4e9b4556d14..a1ea1794c7c8 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -326,28 +326,6 @@ const struct attribute_group blk_integrity_attr_group = {
 	.attrs = integrity_attrs,
 };
 
-static blk_status_t blk_integrity_nop_fn(struct blk_integrity_iter *iter)
-{
-	return BLK_STS_OK;
-}
-
-static void blk_integrity_nop_prepare(struct request *rq)
-{
-}
-
-static void blk_integrity_nop_complete(struct request *rq,
-		unsigned int nr_bytes)
-{
-}
-
-static const struct blk_integrity_profile nop_profile = {
-	.name = "nop",
-	.generate_fn = blk_integrity_nop_fn,
-	.verify_fn = blk_integrity_nop_fn,
-	.prepare_fn = blk_integrity_nop_prepare,
-	.complete_fn = blk_integrity_nop_complete,
-};
-
 /**
  * blk_integrity_register - Register a gendisk as being integrity-capable
  * @disk:	struct gendisk pointer to make integrity-aware
@@ -367,7 +345,7 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
 		template->flags;
 	bi->interval_exp = template->interval_exp ? :
 		ilog2(queue_logical_block_size(disk->queue));
-	bi->profile = template->profile ? template->profile : &nop_profile;
+	bi->profile = template->profile;
 	bi->tuple_size = template->tuple_size;
 	bi->tag_size = template->tag_size;
 
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 421db29535ba..37268656aae9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -834,7 +834,8 @@ static void blk_complete_request(struct request *req)
 		return;
 
 #ifdef CONFIG_BLK_DEV_INTEGRITY
-	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ)
+	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ &&
+	    req->q->integrity.profile->complete_fn)
 		req->q->integrity.profile->complete_fn(req, total_bytes);
 #endif
 
@@ -905,7 +906,7 @@ bool blk_update_request(struct request *req, blk_status_t error,
 
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ &&
-	    error == BLK_STS_OK)
+	    error == BLK_STS_OK && req->q->integrity.profile->complete_fn)
 		req->q->integrity.profile->complete_fn(req, nr_bytes);
 #endif
 
@@ -1268,7 +1269,8 @@ void blk_mq_start_request(struct request *rq)
 	rq->mq_hctx->tags->rqs[rq->tag] = rq;
 
 #ifdef CONFIG_BLK_DEV_INTEGRITY
-	if (blk_integrity_rq(rq) && req_op(rq) == REQ_OP_WRITE)
+	if (blk_integrity_rq(rq) && req_op(rq) == REQ_OP_WRITE &&
+	    q->integrity.profile->prepare_fn)
 		q->integrity.profile->prepare_fn(rq);
 #endif
 	if (rq->bio && rq->bio->bi_opf & REQ_POLLED)
-- 
2.43.0

[PATCH 2/3] block/integrity: flag the queue if it has an integrity profile

[Jens Axboe]

Now that we don't set a dummy profile, if someone registers and actual
profile, flag the queue as such.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-integrity.c  | 14 +++++++++-----
 include/linux/blkdev.h |  1 +
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index a1ea1794c7c8..974af93de2da 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -339,22 +339,25 @@ const struct attribute_group blk_integrity_attr_group = {
  */
 void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
 {
-	struct blk_integrity *bi = &disk->queue->integrity;
+	struct request_queue *q = disk->queue;
+	struct blk_integrity *bi = &q->integrity;
 
 	bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
 		template->flags;
 	bi->interval_exp = template->interval_exp ? :
-		ilog2(queue_logical_block_size(disk->queue));
+		ilog2(queue_logical_block_size(q));
 	bi->profile = template->profile;
+	if (bi->profile)
+		blk_queue_flag_set(QUEUE_FLAG_INTG_PROFILE, q);
 	bi->tuple_size = template->tuple_size;
 	bi->tag_size = template->tag_size;
 
-	blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, disk->queue);
+	blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, q);
 
 #ifdef CONFIG_BLK_INLINE_ENCRYPTION
-	if (disk->queue->crypto_profile) {
+	if (q->crypto_profile) {
 		pr_warn("blk-integrity: Integrity and hardware inline encryption are not supported together. Disabling hardware inline encryption.\n");
-		disk->queue->crypto_profile = NULL;
+		q->crypto_profile = NULL;
 	}
 #endif
 }
@@ -377,6 +380,7 @@ void blk_integrity_unregister(struct gendisk *disk)
 	/* ensure all bios are off the integrity workqueue */
 	blk_flush_integrity();
 	blk_queue_flag_clear(QUEUE_FLAG_STABLE_WRITES, disk->queue);
+	blk_queue_flag_clear(QUEUE_FLAG_INTG_PROFILE, disk->queue);
 	memset(bi, 0, sizeof(*bi));
 }
 EXPORT_SYMBOL(blk_integrity_unregister);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e1e705aef51e..de81642831bb 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -510,6 +510,7 @@ struct request_queue {
 /* Keep blk_queue_flag_name[] in sync with the definitions below */
 #define QUEUE_FLAG_STOPPED	0	/* queue is stopped */
 #define QUEUE_FLAG_DYING	1	/* queue being torn down */
+#define QUEUE_FLAG_INTG_PROFILE	2	/* has integrity profile */
 #define QUEUE_FLAG_NOMERGES     3	/* disable merge attempts */
 #define QUEUE_FLAG_SAME_COMP	4	/* complete on same CPU-group */
 #define QUEUE_FLAG_FAIL_IO	5	/* fake timeout */
-- 
2.43.0

[PATCH 3/3] block: only call bio_integrity_prep() if necessary

[Jens Axboe]

Now that the queue is flag as having an actual profile or not, avoid
calling into the integrity code unless we have one. This removes some
overhead from blk_mq_submit_bio() if BLK_DEV_INTEGRITY is enabled and
we don't have any profiles attached, which is the default and expected
case.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-mq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 37268656aae9..965e42a1bbde 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2961,7 +2961,8 @@ bool blk_mq_submit_bio(struct bio *bio)
 
 	bio_set_ioprio(bio);
 
-	if (!bio_integrity_prep(bio))
+	if (test_bit(QUEUE_FLAG_INTG_PROFILE, &q->queue_flags) &&
+	    !bio_integrity_prep(bio))
 		return false;
 
 	if (plug) {
-- 
2.43.0

Re: [PATCH 1/3] block/integrity: make profile optional

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/3] block/integrity: flag the queue if it has an integrity profile

[Christoph Hellwig]

On Thu, Jan 11, 2024 at 09:00:20AM -0700, Jens Axboe wrote:
> Now that we don't set a dummy profile, if someone registers and actual
> profile, flag the queue as such.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  block/blk-integrity.c  | 14 +++++++++-----
>  include/linux/blkdev.h |  1 +
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index a1ea1794c7c8..974af93de2da 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -339,22 +339,25 @@ const struct attribute_group blk_integrity_attr_group = {
>   */
>  void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
>  {
> -	struct blk_integrity *bi = &disk->queue->integrity;
> +	struct request_queue *q = disk->queue;
> +	struct blk_integrity *bi = &q->integrity;
>  
>  	bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
>  		template->flags;
>  	bi->interval_exp = template->interval_exp ? :
> -		ilog2(queue_logical_block_size(disk->queue));
> +		ilog2(queue_logical_block_size(q));
>  	bi->profile = template->profile;
> +	if (bi->profile)
> +		blk_queue_flag_set(QUEUE_FLAG_INTG_PROFILE, q);

Is this really so much better vs just checking for bi->profile
being non-NULL?

Re: [PATCHSET 0/3] Integrity cleanups and optimization

[Christoph Hellwig]

On Thu, Jan 11, 2024 at 09:00:18AM -0700, Jens Axboe wrote:
> Hi,
> 
> 1 gets rid of the dummy nop profile, 2 marks the queue as having an
> actual profile, and 3 avoids calling into bio_integrity_prep() if we
> don't have a profile. This both reduces code (getting rid of the nop
> profile) and reduces the overhead of the standard setup of having
> integrity enabled in kconfig, yet not using a device with an integrity
> profile.

Bw, can someone help with what dm_integrity_profile is for?
It is basically identical to the no-op one, just with a different
name.  With the no-op removal it is the only one outside of the pi
once, and killing it would really help with some de-virtualization
I've looked at a while ago.

Re: [PATCH 1/3] block/integrity: make profile optional

[Keith Busch]

On Thu, Jan 11, 2024 at 09:00:19AM -0700, Jens Axboe wrote:
>  #ifdef CONFIG_BLK_DEV_INTEGRITY
> -	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ)
> +	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ &&
> +	    req->q->integrity.profile->complete_fn)
>  		req->q->integrity.profile->complete_fn(req, total_bytes);

Since you're going to let profile be NULL, shouldn't this check be
'integrity.profile != NULL' instead of 'profile->complete_fn'?

Re: [PATCH 2/3] block/integrity: flag the queue if it has an integrity profile

[Jens Axboe]

On 1/11/24 9:06 AM, Christoph Hellwig wrote:
> On Thu, Jan 11, 2024 at 09:00:20AM -0700, Jens Axboe wrote:
>> Now that we don't set a dummy profile, if someone registers and actual
>> profile, flag the queue as such.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  block/blk-integrity.c  | 14 +++++++++-----
>>  include/linux/blkdev.h |  1 +
>>  2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
>> index a1ea1794c7c8..974af93de2da 100644
>> --- a/block/blk-integrity.c
>> +++ b/block/blk-integrity.c
>> @@ -339,22 +339,25 @@ const struct attribute_group blk_integrity_attr_group = {
>>   */
>>  void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
>>  {
>> -	struct blk_integrity *bi = &disk->queue->integrity;
>> +	struct request_queue *q = disk->queue;
>> +	struct blk_integrity *bi = &q->integrity;
>>  
>>  	bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
>>  		template->flags;
>>  	bi->interval_exp = template->interval_exp ? :
>> -		ilog2(queue_logical_block_size(disk->queue));
>> +		ilog2(queue_logical_block_size(q));
>>  	bi->profile = template->profile;
>> +	if (bi->profile)
>> +		blk_queue_flag_set(QUEUE_FLAG_INTG_PROFILE, q);
> 
> Is this really so much better vs just checking for bi->profile
> being non-NULL?

Before we can check that, we have to load
bio->bi_bdev->bd_disk->queue->integrity, and we have to call in to
bio_integrity_prep() as well needlessly. We could do that in patch 3 and
then we just need to load q->integrity->profile, and while queue_flags
is certainly hot, it's probably not a huge deal to load that cacheline.
I think if we do that, we stick integrity before limits.

I can make that change, and then we can drop the flag.

-- 
Jens Axboe

Re: [PATCH 2/3] block/integrity: flag the queue if it has an integrity profile

[Christoph Hellwig]

On Thu, Jan 11, 2024 at 09:16:42AM -0700, Jens Axboe wrote:
> Before we can check that, we have to load
> bio->bi_bdev->bd_disk->queue->integrity, and we have to call in to
> bio_integrity_prep() as well needlessly. We could do that in patch 3 and
> then we just need to load q->integrity->profile, and while queue_flags
> is certainly hot, it's probably not a huge deal to load that cacheline.
> I think if we do that, we stick integrity before limits.
> 
> I can make that change, and then we can drop the flag.

If this is good enough in your benchmarks I'd prefer that over adding
a redundant flag.

Re: [PATCH 1/3] block/integrity: make profile optional

[Jens Axboe]

On 1/11/24 9:09 AM, Keith Busch wrote:
> On Thu, Jan 11, 2024 at 09:00:19AM -0700, Jens Axboe wrote:
>>  #ifdef CONFIG_BLK_DEV_INTEGRITY
>> -	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ)
>> +	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ &&
>> +	    req->q->integrity.profile->complete_fn)
>>  		req->q->integrity.profile->complete_fn(req, total_bytes);
> 
> Since you're going to let profile be NULL, shouldn't this check be
> 'integrity.profile != NULL' instead of 'profile->complete_fn'?

Yep, it probably should... I'd need to check if we can have
blk_integrity_rq() be true if we don't have one, but in any case,
seems like the saner check.

-- 
Jens Axboe

Re: [PATCHSET 0/3] Integrity cleanups and optimization

[Martin K. Petersen]

> Bw, can someone help with what dm_integrity_profile is for?
> It is basically identical to the no-op one, just with a different
> name.  With the no-op removal it is the only one outside of the pi
> once, and killing it would really help with some de-virtualization
> I've looked at a while ago.

No particular objections from me wrt. using a flag.

However, I believe the no-op profile and associated plumbing was a
requirement for DM. I forget the details. Mike?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 3/3] block: only call bio_integrity_prep() if necessary

[Keith Busch]

On Thu, Jan 11, 2024 at 09:00:21AM -0700, Jens Axboe wrote:
> Now that the queue is flag as having an actual profile or not, avoid
> calling into the integrity code unless we have one. This removes some
> overhead from blk_mq_submit_bio() if BLK_DEV_INTEGRITY is enabled and
> we don't have any profiles attached, which is the default and expected
> case.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  block/blk-mq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 37268656aae9..965e42a1bbde 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2961,7 +2961,8 @@ bool blk_mq_submit_bio(struct bio *bio)
>  
>  	bio_set_ioprio(bio);
>  
> -	if (!bio_integrity_prep(bio))
> +	if (test_bit(QUEUE_FLAG_INTG_PROFILE, &q->queue_flags) &&
> +	    !bio_integrity_prep(bio))
>  		return false;

I'm confused. The bio_integrity_prep() allocates the metadata buffer.
Drivers that were previoulsy using the 'nop' profile for odd formats
don't get a metadata buffer anymore?

Re: [PATCHSET 0/3] Integrity cleanups and optimization

[Mike Snitzer]

On Thu, Jan 11 2024 at 11:24P -0500,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> 
> > Bw, can someone help with what dm_integrity_profile is for?
> > It is basically identical to the no-op one, just with a different
> > name.  With the no-op removal it is the only one outside of the pi
> > once, and killing it would really help with some de-virtualization
> > I've looked at a while ago.
> 
> No particular objections from me wrt. using a flag.
> 
> However, I believe the no-op profile and associated plumbing was a
> requirement for DM. I forget the details. Mike?

I'll have to take a closer look.. staking device always complicates
things.

But the dummy functions that got wired up with this commit are suspect:
54d4e6ab91eb block: centralize PI remapping logic to the block layer

Effectively the entirety of the dm_integrity_profile is "we don't do
anything special".. so yes it would be nice to not require indirect
calls to accomplish what dm-integrity needs from block core.

Mike

Re: [PATCHSET 0/3] Integrity cleanups and optimization

[Mike Snitzer]

On Thu, Jan 11 2024 at 11:34P -0500,
Mike Snitzer <snitzer@kernel.org> wrote:

> On Thu, Jan 11 2024 at 11:24P -0500,
> Martin K. Petersen <martin.petersen@oracle.com> wrote:
> 
> > 
> > > Bw, can someone help with what dm_integrity_profile is for?
> > > It is basically identical to the no-op one, just with a different
> > > name.  With the no-op removal it is the only one outside of the pi
> > > once, and killing it would really help with some de-virtualization
> > > I've looked at a while ago.
> > 
> > No particular objections from me wrt. using a flag.
> > 
> > However, I believe the no-op profile and associated plumbing was a
> > requirement for DM. I forget the details. Mike?
> 
> I'll have to take a closer look.. stacking device always complicates
> things.

The bulk of the following drivers/md/dm-table.c code snippets are from
these 2 commits (first from me, 2nd from Martin):

a63a5cf84dac dm: improve block integrity support
25520d55cdb6 block: Inline blk_integrity in struct gendisk

static bool integrity_profile_exists(struct gendisk *disk)
{
        return !!blk_get_integrity(disk);
}

/*
 * Get a disk whose integrity profile reflects the table's profile.
 * Returns NULL if integrity support was inconsistent or unavailable.
 */
static struct gendisk *dm_table_get_integrity_disk(struct dm_table *t)
{
        struct list_head *devices = dm_table_get_devices(t);
        struct dm_dev_internal *dd = NULL;
        struct gendisk *prev_disk = NULL, *template_disk = NULL;

        for (unsigned int i = 0; i < t->num_targets; i++) {
                struct dm_target *ti = dm_table_get_target(t, i);

                if (!dm_target_passes_integrity(ti->type))
                        goto no_integrity;
        }

        list_for_each_entry(dd, devices, list) {
                template_disk = dd->dm_dev->bdev->bd_disk;
                if (!integrity_profile_exists(template_disk))
                        goto no_integrity;
                else if (prev_disk &&
                         blk_integrity_compare(prev_disk, template_disk) < 0)
                        goto no_integrity;
                prev_disk = template_disk;
        }

        return template_disk;

no_integrity:
        if (prev_disk)
                DMWARN("%s: integrity not set: %s and %s profile mismatch",
                       dm_device_name(t->md),
	               prev_disk->disk_name,
                       template_disk->disk_name);
        return NULL;
}

/*
 * Register the mapped device for blk_integrity support if the
 * underlying devices have an integrity profile.  But all devices may
 * not have matching profiles (checking all devices isn't reliable
 * during table load because this table may use other DM device(s) which
 * must be resumed before they will have an initialized integity
 * profile).  Consequently, stacked DM devices force a 2 stage integrity
 * profile validation: First pass during table load, final pass during
 * resume.
 */
static int dm_table_register_integrity(struct dm_table *t)
{
	struct mapped_device *md = t->md;
	struct gendisk *template_disk = NULL;

	/* If target handles integrity itself do not register it here. */
	if (t->integrity_added)
		return 0;

	template_disk = dm_table_get_integrity_disk(t);
	if (!template_disk)
		return 0;

	if (!integrity_profile_exists(dm_disk(md))) {
		t->integrity_supported = true;
		/*
		 * Register integrity profile during table load; we can do
		 * this because the final profile must match during resume.
		 */
		blk_integrity_register(dm_disk(md),
				       blk_get_integrity(template_disk));
		return 0;
	}

	/*
	 * If DM device already has an initialized integrity
	 * profile the new profile should not conflict.
	 */
	if (blk_integrity_compare(dm_disk(md), template_disk) < 0) {
		DMERR("%s: conflict with existing integrity profile: %s profile mismatch",
		      dm_device_name(t->md),
		      template_disk->disk_name);
		return 1;
	}

	/* Preserve existing integrity profile */
	t->integrity_supported = true;
	return 0;
}

/*
 * Verify that all devices have an integrity profile that matches the
 * DM device's registered integrity profile.  If the profiles don't
 * match then unregister the DM device's integrity profile.
 */
static void dm_table_verify_integrity(struct dm_table *t)
{
	struct gendisk *template_disk = NULL;

	if (t->integrity_added)
		return;

	if (t->integrity_supported) {
		/*
		 * Verify that the original integrity profile
		 * matches all the devices in this table.
		 */
		template_disk = dm_table_get_integrity_disk(t);
		if (template_disk &&
		    blk_integrity_compare(dm_disk(t->md), template_disk) >= 0)
			return;
	}

	if (integrity_profile_exists(dm_disk(t->md))) {
		DMWARN("%s: unable to establish an integrity profile",
		       dm_device_name(t->md));
		blk_integrity_unregister(dm_disk(t->md));
	}
}


> But the dummy functions that got wired up with this commit are suspect:
> 54d4e6ab91eb block: centralize PI remapping logic to the block layer
> 
> Effectively the entirety of the dm_integrity_profile is "we don't do
> anything special".. so yes it would be nice to not require indirect
> calls to accomplish what dm-integrity needs from block core.

All said, if blk_get_integrity() can be made to handle the removal of
the nop profile then all should "just work" right?  Here is how it has
been:

static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
{
        struct blk_integrity *bi = &disk->queue->integrity;

        if (!bi->profile)
                return NULL;

        return bi;
}

Not looked closely at Jens' patchset (but will do), however it seems
Jens missed updating blk_get_integrity() to deal with his new
QUEUE_FLAG_INTG_PROFILE flag?

As for complete removal of integrity profiles entirely, DM needs a way
to analyze integrity configurations to try to stack up a sane end
result.

Mike

Re: [PATCH 3/3] block: only call bio_integrity_prep() if necessary

[Jens Axboe]

On 1/11/24 9:28 AM, Keith Busch wrote:
> On Thu, Jan 11, 2024 at 09:00:21AM -0700, Jens Axboe wrote:
>> Now that the queue is flag as having an actual profile or not, avoid
>> calling into the integrity code unless we have one. This removes some
>> overhead from blk_mq_submit_bio() if BLK_DEV_INTEGRITY is enabled and
>> we don't have any profiles attached, which is the default and expected
>> case.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  block/blk-mq.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 37268656aae9..965e42a1bbde 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -2961,7 +2961,8 @@ bool blk_mq_submit_bio(struct bio *bio)
>>  
>>  	bio_set_ioprio(bio);
>>  
>> -	if (!bio_integrity_prep(bio))
>> +	if (test_bit(QUEUE_FLAG_INTG_PROFILE, &q->queue_flags) &&
>> +	    !bio_integrity_prep(bio))
>>  		return false;
> 
> I'm confused. The bio_integrity_prep() allocates the metadata buffer.
> Drivers that were previoulsy using the 'nop' profile for odd formats
> don't get a metadata buffer anymore?

Yeah, I just came to the same conclusion diving a bit deeper into
this. It's a bit of a mess imho, but you are right that previously,
by default, we'd get metadata buffers attached with a dummy profile
and with this we would not.

I'll drop this for now, I think this requires more involved work
to make this sane.

-- 
Jens Axboe