On 7/19/23 22:54, Christoph Hellwig wrote:
On Wed, Jul 19, 2023 at 11:22:42AM -0700, Bart Van Assche wrote:
+ * for execution. Don't wait for completion. May sleep if BLK_MQ_F_BLOCKING
+ * has been set.
*
* Note:
* This function will invoke @done directly if the queue is dead.
@@ -2213,6 +2214,8 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
*/
WARN_ON_ONCE(!async && in_interrupt());
+ might_sleep_if(!async && hctx->flags & BLK_MQ_F_BLOCKING);
This is some odd an very complex calling conventions. I suspect most
!BLK_MQ_F_BLOCKING could also deal with the may sleep if not async,
and that would give us a much easier to audit change as we could
remove the WARN_ON_ONCE above and just do a:
might_sleep_if(!async);
In fact this might be a good time to split up blk_mq_run_hw_queue
into blk_mq_run_hw_queue and blk_mq_run_hw_queue_async and do
away with the bool and have cristal clear calling conventions.
If we really need !async calles than can sleep we can add a specific
blk_mq_run_hw_queue_atomic.
Hi Christoph,
blk_mq_run_hw_queue(hctx, false) is called from inside the block layer
with an RCU lock held if BLK_MQ_F_BLOCKING and with an SRCU lock held if
BLK_MQ_F_BLOCKING is not set. So I'm not sure whether it is possible to
simplify the above might_sleep_if() statement. From block/blk-mq.h:
/* run the code block in @dispatch_ops with rcu/srcu read lock held */
#define __blk_mq_run_dispatch_ops(q, check_sleep, dispatch_ops) \
do { \
if ((q)->tag_set->flags & BLK_MQ_F_BLOCKING) { \
struct blk_mq_tag_set *__tag_set = (q)->tag_set; \
int srcu_idx; \
\
might_sleep_if(check_sleep); \
srcu_idx = srcu_read_lock(__tag_set->srcu); \
(dispatch_ops); \
srcu_read_unlock(__tag_set->srcu, srcu_idx); \
} else { \
rcu_read_lock(); \
(dispatch_ops); \
rcu_read_unlock(); \
} \
} while (0)
Thanks,
Bart.