diff mbox series

[v6,2/5] blk-mq: Introduce atomic variants of blk_mq_(all_tag|tagset_busy)_iter

Message ID 20210406214905.21622-3-bvanassche@acm.org (mailing list archive)
State New, archived
Headers show
Series blk-mq: Fix a race between iterating over requests and freeing requests | expand

Commit Message

Bart Van Assche April 6, 2021, 9:49 p.m. UTC
Since in the next patch knowledge is required of whether or not it is
allowed to sleep inside the tag iteration functions, pass this context
information to the tag iteration functions. I have reviewed all callers of
tag iteration functions to verify these annotations by starting from the
output of the following grep command:

    git grep -nHE 'blk_mq_(all_tag|tagset_busy)_iter'

My conclusions from that analysis are as follows:
- Sleeping is allowed in the blk-mq-debugfs code that iterates over tags.
- Since the blk_mq_tagset_busy_iter() calls in the mtip32xx driver are
  preceded by a function that sleeps (blk_mq_quiesce_queue()), sleeping is
  safe in the context of the blk_mq_tagset_busy_iter() calls.
- The same reasoning also applies to the nbd driver.
- All blk_mq_tagset_busy_iter() calls in the NVMe drivers are followed by a
  call to a function that sleeps so sleeping inside blk_mq_tagset_busy_iter()
  when called from the NVMe driver is fine.
- scsi_host_busy(), scsi_host_complete_all_commands() and
  scsi_host_busy_iter() are used by multiple SCSI LLDs so analyzing whether
  or not these functions may sleep is hard. Instead of performing that
  analysis, make it safe to call these functions from atomic context.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Khazhy Kumykov <khazhy@google.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq-tag.c        | 38 +++++++++++++++++++++++++++++++++-----
 block/blk-mq-tag.h        |  2 +-
 block/blk-mq.c            |  2 +-
 drivers/scsi/hosts.c      | 16 ++++++++--------
 drivers/scsi/ufs/ufshcd.c |  4 ++--
 include/linux/blk-mq.h    |  2 ++
 6 files changed, 47 insertions(+), 17 deletions(-)

Comments

John Garry April 7, 2021, 4:57 p.m. UTC | #1
On 06/04/2021 22:49, Bart Van Assche wrote:
> Since in the next patch knowledge is required of whether or not it is
> allowed to sleep inside the tag iteration functions, pass this context
> information to the tag iteration functions. I have reviewed all callers of
> tag iteration functions to verify these annotations by starting from the
> output of the following grep command:
> 
>      git grep -nHE 'blk_mq_(all_tag|tagset_busy)_iter'
> 
> My conclusions from that analysis are as follows:
> - Sleeping is allowed in the blk-mq-debugfs code that iterates over tags.
> - Since the blk_mq_tagset_busy_iter() calls in the mtip32xx driver are
>    preceded by a function that sleeps (blk_mq_quiesce_queue()), sleeping is
>    safe in the context of the blk_mq_tagset_busy_iter() calls.
> - The same reasoning also applies to the nbd driver.
> - All blk_mq_tagset_busy_iter() calls in the NVMe drivers are followed by a
>    call to a function that sleeps so sleeping inside blk_mq_tagset_busy_iter()
>    when called from the NVMe driver is fine.

Hi Bart,

> - scsi_host_busy(), scsi_host_complete_all_commands() and
>    scsi_host_busy_iter() are used by multiple SCSI LLDs so analyzing whether
>    or not these functions may sleep is hard. Instead of performing that
>    analysis, make it safe to call these functions from atomic context.

Please help me understand this solution. The background is that we are 
unsure if the SCSI iters callback functions may sleep. So we use the 
blk_mq_all_tag_iter_atomic() iter, which tells us that we must not 
sleep. And internally, it uses rcu read lock protection mechanism, which 
relies on not sleeping. So it seems that we're making the SCSI iter 
functions being safe in atomic context, and, as such, rely on the iter 
callbacks not to sleep.

But if we call the SCSI iter function from non-atomic context and the 
iter callback may sleep, then that is a problem, right? We're still 
using rcu.

Thanks,
John
Bart Van Assche April 7, 2021, 6:42 p.m. UTC | #2
On 4/7/21 9:57 AM, John Garry wrote:
> On 06/04/2021 22:49, Bart Van Assche wrote:
>> Since in the next patch knowledge is required of whether or not it is
>> allowed to sleep inside the tag iteration functions, pass this context
>> information to the tag iteration functions. I have reviewed all 
>> callers of
>> tag iteration functions to verify these annotations by starting from the
>> output of the following grep command:
>>
>>      git grep -nHE 'blk_mq_(all_tag|tagset_busy)_iter'
>>
>> My conclusions from that analysis are as follows:
>> - Sleeping is allowed in the blk-mq-debugfs code that iterates over tags.
>> - Since the blk_mq_tagset_busy_iter() calls in the mtip32xx driver are
>>    preceded by a function that sleeps (blk_mq_quiesce_queue()), 
>> sleeping is
>>    safe in the context of the blk_mq_tagset_busy_iter() calls.
>> - The same reasoning also applies to the nbd driver.
>> - All blk_mq_tagset_busy_iter() calls in the NVMe drivers are followed 
>> by a
>>    call to a function that sleeps so sleeping inside 
>> blk_mq_tagset_busy_iter()
>>    when called from the NVMe driver is fine.
> 
> Hi Bart,
> 
>> - scsi_host_busy(), scsi_host_complete_all_commands() and
>>    scsi_host_busy_iter() are used by multiple SCSI LLDs so analyzing 
>> whether
>>    or not these functions may sleep is hard. Instead of performing that
>>    analysis, make it safe to call these functions from atomic context.
> 
> Please help me understand this solution. The background is that we are 
> unsure if the SCSI iters callback functions may sleep. So we use the 
> blk_mq_all_tag_iter_atomic() iter, which tells us that we must not 
> sleep. And internally, it uses rcu read lock protection mechanism, which 
> relies on not sleeping. So it seems that we're making the SCSI iter 
> functions being safe in atomic context, and, as such, rely on the iter 
> callbacks not to sleep.
> 
> But if we call the SCSI iter function from non-atomic context and the 
> iter callback may sleep, then that is a problem, right? We're still 
> using rcu.

Hi John,

Please take a look at the output of the following grep command:

git grep -nHEw 'blk_mq_tagset_busy_iter|scsi_host_busy_iter'\ drivers/scsi

Do you agree with me that it is safe to call all the callback functions 
passed to blk_mq_tagset_busy_iter() and scsi_host_busy_iter() from an 
atomic context?

Thanks,

Bart.
John Garry April 8, 2021, 12:48 p.m. UTC | #3
On 07/04/2021 19:42, Bart Van Assche wrote:
>>
>>> - scsi_host_busy(), scsi_host_complete_all_commands() and
>>>    scsi_host_busy_iter() are used by multiple SCSI LLDs so analyzing 
>>> whether
>>>    or not these functions may sleep is hard. Instead of performing that
>>>    analysis, make it safe to call these functions from atomic context.
>>
>> Please help me understand this solution. The background is that we are 
>> unsure if the SCSI iters callback functions may sleep. So we use the 
>> blk_mq_all_tag_iter_atomic() iter, which tells us that we must not 
>> sleep. And internally, it uses rcu read lock protection mechanism, 
>> which relies on not sleeping. So it seems that we're making the SCSI 
>> iter functions being safe in atomic context, and, as such, rely on the 
>> iter callbacks not to sleep.
>>
>> But if we call the SCSI iter function from non-atomic context and the 
>> iter callback may sleep, then that is a problem, right? We're still 
>> using rcu.
> 

Hi Bart,

> 
> Please take a look at the output of the following grep command:
> 
> git grep -nHEw 'blk_mq_tagset_busy_iter|scsi_host_busy_iter'\ drivers/scsi
> 
> Do you agree with me that it is safe to call all the callback functions 
> passed to blk_mq_tagset_busy_iter() and scsi_host_busy_iter() from an 
> atomic context?
> 

That list output I got is at the bottom (with this patchset, not 
mainline - not sure which you meant).

The following does not look atomic safe with the mutex usage:
drivers/block/nbd.c:819:        blk_mq_tagset_busy_iter(&nbd->tag_set, 
nbd_clear_req, NULL);

static bool nbd_clear_req(struct request *req, void *data, bool reserved)
{
	struct nbd_cmd *cmd = blk_mq_rq_to_pdu(req);

	mutex_lock(&cmd->lock);
	cmd->status = BLK_STS_IOERR;
	mutex_unlock(&cmd->lock);

	blk_mq_complete_request(req);
	return true;
}

But blk_mq_tagset_busy_iter() uses BT_TAG_ITER_MAY sleep flag in your 
series.

As for the fc, I am not sure. I assume that you would know more about 
this. My concern is

__nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op)
{
...

	ctrl->lport->ops->fcp_abort(&ctrl->lport->localport, ..);
}

Looking at many instances of fcp_abort callback, they look atomic safe 
from general high usage of spinlock, but I am not certain. They are 
quite complex.

Thanks,
John

block/blk-core.c:287: * blk_mq_tagset_busy_iter() and also for their 
atomic variants to finish
block/blk-mq-debugfs.c:418: 
blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy_rq,
block/blk-mq-tag.c:405: * blk_mq_tagset_busy_iter - iterate over all 
started requests in a tag set
block/blk-mq-tag.c:416:void blk_mq_tagset_busy_iter(struct 
blk_mq_tag_set *tagset,
block/blk-mq-tag.c:436:EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
drivers/block/mtip32xx/mtip32xx.c:2685: 
blk_mq_tagset_busy_iter(&dd->tags, mtip_queue_cmd, dd);
drivers/block/mtip32xx/mtip32xx.c:2690: 
blk_mq_tagset_busy_iter(&dd->tags,
drivers/block/mtip32xx/mtip32xx.c:3800: 
blk_mq_tagset_busy_iter(&dd->tags, mtip_no_dev_cleanup, dd);
drivers/block/nbd.c:819:        blk_mq_tagset_busy_iter(&nbd->tag_set, 
nbd_clear_req, NULL);
drivers/nvme/host/core.c:392: 
blk_mq_tagset_busy_iter(ctrl->tagset,
drivers/nvme/host/core.c:402: 
blk_mq_tagset_busy_iter(ctrl->admin_tagset,
drivers/nvme/host/fc.c:2477: 
blk_mq_tagset_busy_iter(&ctrl->tag_set,
drivers/nvme/host/fc.c:2500: 
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
drivers/nvme/host/pci.c:2504:   blk_mq_tagset_busy_iter(&dev->tagset, 
nvme_cancel_request, &dev->ctrl);
drivers/nvme/host/pci.c:2505: 
blk_mq_tagset_busy_iter(&dev->admin_tagset, nvme_cancel_request, 
&dev->ctrl);
drivers/nvme/target/loop.c:420: 
blk_mq_tagset_busy_iter(&ctrl->tag_set,
drivers/nvme/target/loop.c:430: 
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
include/linux/blk-mq.h:527:void blk_mq_tagset_busy_iter(struct 
blk_mq_tag_set *tagset,
lines 1-18/18 (END)
Bart Van Assche April 8, 2021, 4:12 p.m. UTC | #4
On 4/8/21 5:48 AM, John Garry wrote:
> The following does not look atomic safe with the mutex usage:
> drivers/block/nbd.c:819:        blk_mq_tagset_busy_iter(&nbd->tag_set,
> nbd_clear_req, NULL);
> 
> static bool nbd_clear_req(struct request *req, void *data, bool reserved)
> {
>     struct nbd_cmd *cmd = blk_mq_rq_to_pdu(req);
> 
>     mutex_lock(&cmd->lock);
>     cmd->status = BLK_STS_IOERR;
>     mutex_unlock(&cmd->lock);
> 
>     blk_mq_complete_request(req);
>     return true;
> }
> 
> But blk_mq_tagset_busy_iter() uses BT_TAG_ITER_MAY sleep flag in your
> series.

I will mention the nbd driver in the commit message.

> As for the fc, I am not sure. I assume that you would know more about
> this. My concern is
> 
> __nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op)
> {
> ...
> 
>     ctrl->lport->ops->fcp_abort(&ctrl->lport->localport, ..);
> }
> 
> Looking at many instances of fcp_abort callback, they look atomic safe
> from general high usage of spinlock, but I am not certain. They are
> quite complex.
I have not tried to analyze whether or not it is safe to call
__nvme_fc_abort_op() from an atomic context. Instead I analyzed the
contexts from which this function is called, namely the
blk_mq_tagset_busy_iter() calls in __nvme_fc_abort_outstanding_ios() and
__nvme_fc_abort_outstanding_ios(). Both blk_mq_tagset_busy_iter() calls
are followed by a call to a function that may sleep. Hence it is safe to
sleep inside the blk_mq_tagset_busy_iter() calls from the nvme_fc code.
I have not tried to analyze whether it would be safe to change these
blk_mq_tagset_busy_iter() calls into blk_mq_tagset_busy_iter_atomic()
calls. Does this answer your question?

Bart.
John Garry April 8, 2021, 4:35 p.m. UTC | #5
Hi Bart,

>> But blk_mq_tagset_busy_iter() uses BT_TAG_ITER_MAY sleep flag in your
>> series.
> 
> I will mention the nbd driver in the commit message.
> 
>> As for the fc, I am not sure. I assume that you would know more about
>> this. My concern is
>>
>> __nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op)
>> {
>> ...
>>
>>      ctrl->lport->ops->fcp_abort(&ctrl->lport->localport, ..);
>> }
>>
>> Looking at many instances of fcp_abort callback, they look atomic safe
>> from general high usage of spinlock, but I am not certain. They are
>> quite complex.
> I have not tried to analyze whether or not it is safe to call
> __nvme_fc_abort_op() from an atomic context. Instead I analyzed the
> contexts from which this function is called, namely the
> blk_mq_tagset_busy_iter() calls in __nvme_fc_abort_outstanding_ios() and
> __nvme_fc_abort_outstanding_ios(). Both blk_mq_tagset_busy_iter() calls
> are followed by a call to a function that may sleep. Hence it is safe to
> sleep inside the blk_mq_tagset_busy_iter() calls from the nvme_fc code.
> I have not tried to analyze whether it would be safe to change these
> blk_mq_tagset_busy_iter() calls into blk_mq_tagset_busy_iter_atomic()
> calls. Does this answer your question?

Yes, fine.

Thanks,
John
Daniel Wagner April 13, 2021, 7:50 a.m. UTC | #6
On Tue, Apr 06, 2021 at 02:49:02PM -0700, Bart Van Assche wrote:
> Since in the next patch knowledge is required of whether or not it is
> allowed to sleep inside the tag iteration functions, pass this context
> information to the tag iteration functions. I have reviewed all callers of
> tag iteration functions to verify these annotations by starting from the
> output of the following grep command:
> 
>     git grep -nHE 'blk_mq_(all_tag|tagset_busy)_iter'
> 
> My conclusions from that analysis are as follows:
> - Sleeping is allowed in the blk-mq-debugfs code that iterates over tags.
> - Since the blk_mq_tagset_busy_iter() calls in the mtip32xx driver are
>   preceded by a function that sleeps (blk_mq_quiesce_queue()), sleeping is
>   safe in the context of the blk_mq_tagset_busy_iter() calls.
> - The same reasoning also applies to the nbd driver.
> - All blk_mq_tagset_busy_iter() calls in the NVMe drivers are followed by a
>   call to a function that sleeps so sleeping inside blk_mq_tagset_busy_iter()
>   when called from the NVMe driver is fine.
> - scsi_host_busy(), scsi_host_complete_all_commands() and
>   scsi_host_busy_iter() are used by multiple SCSI LLDs so analyzing whether
>   or not these functions may sleep is hard. Instead of performing that
>   analysis, make it safe to call these functions from atomic context.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Cc: John Garry <john.garry@huawei.com>
> Cc: Khazhy Kumykov <khazhy@google.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Maybe you could also annotate blk_mq_all_tag_iter() with a
might_sleep(). This would help to find API abusers more easily.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Bart Van Assche April 20, 2021, 9:35 p.m. UTC | #7
On 4/13/21 12:50 AM, Daniel Wagner wrote:
> Maybe you could also annotate blk_mq_all_tag_iter() with a
> might_sleep(). This would help to find API abusers more easily.

Hmm ... my understanding is that blk_mq_all_tag_iter() does not sleep
since that function does not sleep itself and since the only caller of
blk_mq_all_tag_iter() callers passes a callback function that does not
sleep?

Thanks,

Bart.
Daniel Wagner April 21, 2021, 7:25 a.m. UTC | #8
On Tue, Apr 20, 2021 at 02:35:53PM -0700, Bart Van Assche wrote:
> On 4/13/21 12:50 AM, Daniel Wagner wrote:
> > Maybe you could also annotate blk_mq_all_tag_iter() with a
> > might_sleep(). This would help to find API abusers more easily.
> 
> Hmm ... my understanding is that blk_mq_all_tag_iter() does not sleep
> since that function does not sleep itself and since the only caller of
> blk_mq_all_tag_iter() callers passes a callback function that does not
> sleep?

Yes, you are right. I read the documentation and assumed a might_sleep()
would be missing.
diff mbox series

Patch

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 2a37731e8244..d8eaa38a1bd1 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -322,18 +322,19 @@  static void __blk_mq_all_tag_iter(struct blk_mq_tags *tags,
 }
 
 /**
- * blk_mq_all_tag_iter - iterate over all requests in a tag map
+ * blk_mq_all_tag_iter_atomic - iterate over all requests in a tag map
  * @tags:	Tag map to iterate over.
  * @fn:		Pointer to the function that will be called for each
  *		request. @fn will be called as follows: @fn(rq, @priv,
  *		reserved) where rq is a pointer to a request. 'reserved'
  *		indicates whether or not @rq is a reserved request. Return
- *		true to continue iterating tags, false to stop.
+ *		true to continue iterating tags, false to stop. Must not
+ *		sleep.
  * @priv:	Will be passed as second argument to @fn.
  *
- * Caller has to pass the tag map from which requests are allocated.
+ * Does not sleep.
  */
-void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
+void blk_mq_all_tag_iter_atomic(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
 		void *priv)
 {
 	__blk_mq_all_tag_iter(tags, fn, priv, BT_TAG_ITER_STATIC_RQS);
@@ -348,6 +349,8 @@  void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
  *		indicates whether or not @rq is a reserved request. Return
  *		true to continue iterating tags, false to stop.
  * @priv:	Will be passed as second argument to @fn.
+ *
+ * May sleep.
  */
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		busy_tag_iter_fn *fn, void *priv)
@@ -362,6 +365,31 @@  void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 }
 EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
 
+/**
+ * blk_mq_tagset_busy_iter_atomic - iterate over all started requests in a tag set
+ * @tagset:	Tag set to iterate over.
+ * @fn:		Pointer to the function that will be called for each started
+ *		request. @fn will be called as follows: @fn(rq, @priv,
+ *		reserved) where rq is a pointer to a request. 'reserved'
+ *		indicates whether or not @rq is a reserved request. Return
+ *		true to continue iterating tags, false to stop. Must not sleep.
+ * @priv:	Will be passed as second argument to @fn.
+ *
+ * Does not sleep.
+ */
+void blk_mq_tagset_busy_iter_atomic(struct blk_mq_tag_set *tagset,
+		busy_tag_iter_fn *fn, void *priv)
+{
+	int i;
+
+	for (i = 0; i < tagset->nr_hw_queues; i++) {
+		if (tagset->tags && tagset->tags[i])
+			__blk_mq_all_tag_iter(tagset->tags[i], fn, priv,
+					      BT_TAG_ITER_STARTED);
+	}
+}
+EXPORT_SYMBOL(blk_mq_tagset_busy_iter_atomic);
+
 static bool blk_mq_tagset_count_completed_rqs(struct request *rq,
 		void *data, bool reserved)
 {
@@ -384,7 +412,7 @@  void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset)
 	while (true) {
 		unsigned count = 0;
 
-		blk_mq_tagset_busy_iter(tagset,
+		blk_mq_tagset_busy_iter_atomic(tagset,
 				blk_mq_tagset_count_completed_rqs, &count);
 		if (!count)
 			break;
diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h
index 7d3e6b333a4a..0290c308ece9 100644
--- a/block/blk-mq-tag.h
+++ b/block/blk-mq-tag.h
@@ -43,7 +43,7 @@  extern void blk_mq_tag_resize_shared_sbitmap(struct blk_mq_tag_set *set,
 extern void blk_mq_tag_wakeup_all(struct blk_mq_tags *tags, bool);
 void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn,
 		void *priv);
-void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
+void blk_mq_all_tag_iter_atomic(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
 		void *priv);
 
 static inline struct sbq_wait_state *bt_wait_ptr(struct sbitmap_queue *bt,
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d4d7c1caa439..5b170faa6318 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2483,7 +2483,7 @@  static bool blk_mq_hctx_has_requests(struct blk_mq_hw_ctx *hctx)
 		.hctx	= hctx,
 	};
 
-	blk_mq_all_tag_iter(tags, blk_mq_has_request, &data);
+	blk_mq_all_tag_iter_atomic(tags, blk_mq_has_request, &data);
 	return data.has_rq;
 }
 
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 697c09ef259b..f8863aa88642 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -573,8 +573,8 @@  int scsi_host_busy(struct Scsi_Host *shost)
 {
 	int cnt = 0;
 
-	blk_mq_tagset_busy_iter(&shost->tag_set,
-				scsi_host_check_in_flight, &cnt);
+	blk_mq_tagset_busy_iter_atomic(&shost->tag_set,
+				       scsi_host_check_in_flight, &cnt);
 	return cnt;
 }
 EXPORT_SYMBOL(scsi_host_busy);
@@ -672,8 +672,8 @@  static bool complete_all_cmds_iter(struct request *rq, void *data, bool rsvd)
  */
 void scsi_host_complete_all_commands(struct Scsi_Host *shost, int status)
 {
-	blk_mq_tagset_busy_iter(&shost->tag_set, complete_all_cmds_iter,
-				&status);
+	blk_mq_tagset_busy_iter_atomic(&shost->tag_set, complete_all_cmds_iter,
+				       &status);
 }
 EXPORT_SYMBOL_GPL(scsi_host_complete_all_commands);
 
@@ -694,11 +694,11 @@  static bool __scsi_host_busy_iter_fn(struct request *req, void *priv,
 /**
  * scsi_host_busy_iter - Iterate over all busy commands
  * @shost:	Pointer to Scsi_Host.
- * @fn:		Function to call on each busy command
+ * @fn:		Function to call on each busy command. Must not sleep.
  * @priv:	Data pointer passed to @fn
  *
  * If locking against concurrent command completions is required
- * ithas to be provided by the caller
+ * it has to be provided by the caller.
  **/
 void scsi_host_busy_iter(struct Scsi_Host *shost,
 			 bool (*fn)(struct scsi_cmnd *, void *, bool),
@@ -709,7 +709,7 @@  void scsi_host_busy_iter(struct Scsi_Host *shost,
 		.priv = priv,
 	};
 
-	blk_mq_tagset_busy_iter(&shost->tag_set, __scsi_host_busy_iter_fn,
-				&iter_data);
+	blk_mq_tagset_busy_iter_atomic(&shost->tag_set,
+				       __scsi_host_busy_iter_fn, &iter_data);
 }
 EXPORT_SYMBOL_GPL(scsi_host_busy_iter);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c86760788c72..6d2f8f18e2a3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1380,7 +1380,7 @@  static bool ufshcd_any_tag_in_use(struct ufs_hba *hba)
 	struct request_queue *q = hba->cmd_queue;
 	int busy = 0;
 
-	blk_mq_tagset_busy_iter(q->tag_set, ufshcd_is_busy, &busy);
+	blk_mq_tagset_busy_iter_atomic(q->tag_set, ufshcd_is_busy, &busy);
 	return busy;
 }
 
@@ -6269,7 +6269,7 @@  static irqreturn_t ufshcd_tmc_handler(struct ufs_hba *hba)
 		.pending = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL),
 	};
 
-	blk_mq_tagset_busy_iter(q->tag_set, ufshcd_compl_tm, &ci);
+	blk_mq_tagset_busy_iter_atomic(q->tag_set, ufshcd_compl_tm, &ci);
 	return ci.ncpl ? IRQ_HANDLED : IRQ_NONE;
 }
 
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 2c473c9b8990..dfa0114a49fd 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -526,6 +526,8 @@  void blk_mq_run_hw_queues(struct request_queue *q, bool async);
 void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs);
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		busy_tag_iter_fn *fn, void *priv);
+void blk_mq_tagset_busy_iter_atomic(struct blk_mq_tag_set *tagset,
+		busy_tag_iter_fn *fn, void *priv);
 void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset);
 void blk_mq_freeze_queue(struct request_queue *q);
 void blk_mq_unfreeze_queue(struct request_queue *q);