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

[Bart Van Assche <bvanassche@acm.org>]

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.