On 3/8/22 1:00 AM, Daejun Park wrote: > Hi Mike, > >> @@ -2952,8 +2954,8 @@ scsi_host_block(struct Scsi_Host *shost) >> } >> >> /* >> - * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag so >> - * calling synchronize_rcu() once is enough. >> + * Drivers that use this helper enable blk-mq's BLK_MQ_F_BLOCKING flag >> + * so calling synchronize_rcu() once is enough. >> */ >> WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING); > Should we keep this line? > > And I think scsi_host_block() should call synchronize_srcu() rather than > synchronize_rcu(), if the BLK_MQ_F_BLOCKING flag is used. > Sorry, I messed up the comment. The comment should have been: Drivers that set the blk-mq BLK_MQ_F_BLOCKING do not use this function so calling synchronize_rcu() once is enough. -------------------------- The iscsi drivers that use BLK_MQ_F_BLOCKING use scsi_target_block which ends up calling blk_mq_wait_quiesce_done. That function will do the right thing wrt rcu/srcu based on if the BLK_MQ_F_BLOCKING flag was set when the scsi host was added. So in the above comment I meant to express drivers that use BLK_MQ_F_BLOCKING shouldn't call scsi_host_block and right now the WARN call is valid. However, I could do something like you are suggesting and do a: if (shost->hostt->queuecommand_may_sleep_blocks) synchronize_srcu() else synchronize_rcu() However, I don't have any way to test it, so I was a little reluctant to add that.
