| Message ID | 20200423020713.332743-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [V2] scsi: avoid to run synchronize_rcu for each device in scsi_host_block | expand |
On 4/23/20 4:07 AM, Ming Lei wrote: > scsi_host_block() calls scsi_internal_device_block() for each > scsi_device, and scsi_internal_device_block() calls > blk_mq_quiesce_queue() for each LUN. However synchronize_rcu is > run from blk_mq_quiesce_queue(). > > This way may become unnecessary slow in case of lots of LUNs. > > Use scsi_internal_device_block_nowait() to implement scsi_host_block(), > so it is enough to run synchronize_rcu() once since scsi never > supports blk-mq's BLK_MQ_F_BLOCKING. > > Cc: Steffen Maier <maier@linux.ibm.com> > Cc: Bart Van Assche <bvanassche@acm.org> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Dexuan Cui <decui@microsoft.com> > Cc: Hannes Reinecke <hare@suse.de> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > V2: > - fix commit log as pointed by Steffen > - add comment on BLK_MQ_F_BLOCKING as suggested by Bart > > drivers/scsi/scsi_lib.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 47835c4b4ee0..86007a523145 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -2841,11 +2841,26 @@ scsi_host_block(struct Scsi_Host *shost) > struct scsi_device *sdev; > int ret = 0; > > + /* > + * Call scsi_internal_device_block_nowait then we can avoid to > + * run synchronize_rcu() for each LUN > + */ > shost_for_each_device(sdev, shost) { > - ret = scsi_internal_device_block(sdev); > + mutex_lock(&sdev->state_mutex); > + ret = scsi_internal_device_block_nowait(sdev); > + mutex_unlock(&sdev->state_mutex); > if (ret) > break; > } > + > + /* > + * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING, so run > + * synchronize_rcu() once is enough > + */ > + WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING); > + > + if (!ret) > + synchronize_rcu(); > return ret; > } > EXPORT_SYMBOL_GPL(scsi_host_block); > Looks good. Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
Ming, > scsi_host_block() calls scsi_internal_device_block() for each > scsi_device, and scsi_internal_device_block() calls > blk_mq_quiesce_queue() for each LUN. However synchronize_rcu is > run from blk_mq_quiesce_queue(). Applied to 5.8/scsi-queue, thank you!
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 47835c4b4ee0..86007a523145 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2841,11 +2841,26 @@ scsi_host_block(struct Scsi_Host *shost) struct scsi_device *sdev; int ret = 0; + /* + * Call scsi_internal_device_block_nowait then we can avoid to + * run synchronize_rcu() for each LUN + */ shost_for_each_device(sdev, shost) { - ret = scsi_internal_device_block(sdev); + mutex_lock(&sdev->state_mutex); + ret = scsi_internal_device_block_nowait(sdev); + mutex_unlock(&sdev->state_mutex); if (ret) break; } + + /* + * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING, so run + * synchronize_rcu() once is enough + */ + WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING); + + if (!ret) + synchronize_rcu(); return ret; } EXPORT_SYMBOL_GPL(scsi_host_block);
scsi_host_block() calls scsi_internal_device_block() for each scsi_device, and scsi_internal_device_block() calls blk_mq_quiesce_queue() for each LUN. However synchronize_rcu is run from blk_mq_quiesce_queue(). This way may become unnecessary slow in case of lots of LUNs. Use scsi_internal_device_block_nowait() to implement scsi_host_block(), so it is enough to run synchronize_rcu() once since scsi never supports blk-mq's BLK_MQ_F_BLOCKING. Cc: Steffen Maier <maier@linux.ibm.com> Cc: Bart Van Assche <bvanassche@acm.org> Cc: Christoph Hellwig <hch@lst.de> Cc: Dexuan Cui <decui@microsoft.com> Cc: Hannes Reinecke <hare@suse.de> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- V2: - fix commit log as pointed by Steffen - add comment on BLK_MQ_F_BLOCKING as suggested by Bart drivers/scsi/scsi_lib.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)