| Message ID | 20180807174433.8374-11-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SCSI: introduce per-host admin queue & enable runtime PM | expand |
On Wed, 2018-08-08 at 01:44 +0800, Ming Lei wrote: > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -278,16 +278,16 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, > struct request *req; > struct scsi_request *rq; > int ret = DRIVER_ERROR << 24; > + struct request_queue *q = sdev->host->admin_q; > > - req = blk_get_request(sdev->request_queue, > + req = blk_get_request(q, > data_direction == DMA_TO_DEVICE ? > REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT); The above looks weird to me. Why are all RQF_PREEMPT requests sent to the admin queue instead of only RQF_PM requests? > @@ -299,6 +299,8 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, > req->cmd_flags |= flags; > req->rq_flags |= rq_flags | RQF_QUIET; > > + atomic_inc(&sdev->nr_admin_pending); Why has a new counter been introduced to keep track of admin requests instead of using q_usage_counter? Thanks, Bart.
On Tue, Aug 07, 2018 at 11:33:15PM +0000, Bart Van Assche wrote: > On Wed, 2018-08-08 at 01:44 +0800, Ming Lei wrote: > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -278,16 +278,16 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, > > struct request *req; > > struct scsi_request *rq; > > int ret = DRIVER_ERROR << 24; > > + struct request_queue *q = sdev->host->admin_q; > > > > - req = blk_get_request(sdev->request_queue, > > + req = blk_get_request(q, > > data_direction == DMA_TO_DEVICE ? > > REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT); > > The above looks weird to me. Why are all RQF_PREEMPT requests sent to the admin > queue instead of only RQF_PM requests? The motivation is to use the dedicated admin queue for sending any admin request, and that is why it isn't named as pm_queue, :-) Also usually RQF_PREEMPT request is allowed when queue is quiesced, which can be used in both system suspend and sending domain validation. > > > @@ -299,6 +299,8 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, > > req->cmd_flags |= flags; > > req->rq_flags |= rq_flags | RQF_QUIET; > > > > + atomic_inc(&sdev->nr_admin_pending); > > Why has a new counter been introduced to keep track of admin requests instead of > using q_usage_counter? Just for making sure all admin requests sent to this scsi_device can be completed before removing this scsi_device, given this patch switches to use per-host admin queue to send admin requests to all scsi_devices in this host. So we can't use the q_usage_counter of scsi_device's queue or the per-host admin queue. It won't be one big deal since scsi_execute() won't be run in fast path. Thanks, Ming
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index dbb355e23262..79475a679750 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -278,16 +278,16 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, struct request *req; struct scsi_request *rq; int ret = DRIVER_ERROR << 24; + struct request_queue *q = sdev->host->admin_q; - req = blk_get_request(sdev->request_queue, + req = blk_get_request(q, data_direction == DMA_TO_DEVICE ? REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT); if (IS_ERR(req)) return ret; rq = scsi_req(req); - if (bufflen && blk_rq_map_kern(sdev->request_queue, req, - buffer, bufflen, GFP_NOIO)) + if (bufflen && blk_rq_map_kern(q, req, buffer, bufflen, GFP_NOIO)) goto out; rq->cmd_len = COMMAND_SIZE(cmd[0]); @@ -299,6 +299,8 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, req->cmd_flags |= flags; req->rq_flags |= rq_flags | RQF_QUIET; + atomic_inc(&sdev->nr_admin_pending); + /* * head injection *required* here otherwise quiesce won't work */ @@ -323,6 +325,9 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, out: blk_put_request(req); + atomic_dec(&sdev->nr_admin_pending); + wake_up_all(&sdev->admin_wq); + return ret; } EXPORT_SYMBOL(__scsi_execute); diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 0880d975eed3..b10af1692ddd 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -243,6 +243,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, mutex_init(&sdev->inquiry_mutex); INIT_WORK(&sdev->event_work, scsi_evt_thread); INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue); + init_waitqueue_head(&sdev->admin_wq); sdev->sdev_gendev.parent = get_device(&starget->dev); sdev->sdev_target = starget; diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 7943b762c12d..7e03a420dfe7 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1377,6 +1377,7 @@ void __scsi_remove_device(struct scsi_device *sdev) blk_cleanup_queue(sdev->request_queue); cancel_work_sync(&sdev->requeue_work); + wait_event(sdev->admin_wq, !atomic_read(&sdev->nr_admin_pending)); if (sdev->host->hostt->slave_destroy) sdev->host->hostt->slave_destroy(sdev); diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 202f4d6a4342..f6820da1dc37 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -227,6 +227,10 @@ struct scsi_device { struct mutex state_mutex; enum scsi_device_state sdev_state; struct task_struct *quiesced_by; + + atomic_t nr_admin_pending; + wait_queue_head_t admin_wq; + unsigned long sdev_data[0]; } __attribute__((aligned(sizeof(unsigned long))));
Now the per-host dedicated admin queue is ready, so use this queue to send admin commands only. Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Christoph Hellwig <hch@lst.de> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: Jianchao Wang <jianchao.w.wang@oracle.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Johannes Thumshirn <jthumshirn@suse.de> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: linux-scsi@vger.kernel.org Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/scsi_lib.c | 11 ++++++++--- drivers/scsi/scsi_scan.c | 1 + drivers/scsi/scsi_sysfs.c | 1 + include/scsi/scsi_device.h | 4 ++++ 4 files changed, 14 insertions(+), 3 deletions(-)