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

Commit Message

Jens Axboe Jan. 11, 2024, 4 p.m. UTC

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 <[e-mail]>
---
 block/blk-integrity.c | 24 +-----------------------
 block/blk-mq.c        |  8 +++++---
 2 files changed, 6 insertions(+), 26 deletions(-)

Comments

Christoph Hellwig Jan. 11, 2024, 4:05 p.m. UTC | #1

Looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Keith Busch Jan. 11, 2024, 4:09 p.m. UTC | #2

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'?

Jens Axboe Jan. 11, 2024, 4:19 p.m. UTC | #3

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.

Patch

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)