From: Omar Sandoval <osandov@osandov.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org, linux-ide@vger.kernel.org,
linux-scsi@vger.kernel.org
Subject: Re: [PATCH 09/30] scsi: kill off the legacy IO path
Date: Thu, 1 Nov 2018 14:11:56 -0700 [thread overview]
Message-ID: <20181101211156.GE18005@vader> (raw)
In-Reply-To: <20181031175922.8849-10-axboe@kernel.dk>
On Wed, Oct 31, 2018 at 11:59:01AM -0600, Jens Axboe wrote:
> Cc: linux-scsi@vger.kernel.org
> Acked-by: Himanshu Madhani <himanshu.madhani@cavium.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
A bunch of really trivial nitpicks below, only to prove that I read the
thing ;)
Reviewed-by: Omar Sandoval <osandov@fb.com>
> ---
> Documentation/scsi/scsi-parameters.txt | 5 -
> drivers/scsi/Kconfig | 12 -
> drivers/scsi/cxlflash/main.c | 6 -
> drivers/scsi/hosts.c | 29 +-
> drivers/scsi/lpfc/lpfc_scsi.c | 2 +-
> drivers/scsi/qedi/qedi_main.c | 3 +-
> drivers/scsi/qla2xxx/qla_os.c | 30 +-
> drivers/scsi/scsi.c | 5 +-
> drivers/scsi/scsi_debug.c | 3 +-
> drivers/scsi/scsi_error.c | 2 +-
> drivers/scsi/scsi_lib.c | 603 ++-----------------------
> drivers/scsi/scsi_priv.h | 1 -
> drivers/scsi/scsi_scan.c | 10 +-
> drivers/scsi/scsi_sysfs.c | 8 +-
> drivers/scsi/ufs/ufshcd.c | 6 -
> include/scsi/scsi_host.h | 18 +-
> include/scsi/scsi_tcq.h | 14 +-
> 17 files changed, 77 insertions(+), 680 deletions(-)
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 8794e54f43a9..3e2665c66bc4 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -857,13 +857,9 @@ qla2xxx_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
> }
>
> if (ha->mqenable) {
> - if (shost_use_blk_mq(vha->host)) {
> - tag = blk_mq_unique_tag(cmd->request);
> - hwq = blk_mq_unique_tag_to_hwq(tag);
> - qpair = ha->queue_pair_map[hwq];
> - } else if (vha->vp_idx && vha->qpair) {
> - qpair = vha->qpair;
> - }
> + tag = blk_mq_unique_tag(cmd->request);
> + hwq = blk_mq_unique_tag_to_hwq(tag);
> + qpair = ha->queue_pair_map[hwq];
>
> if (qpair)
> return qla2xxx_mqueuecommand(host, cmd, qpair);
> @@ -3153,7 +3149,7 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
> goto probe_failed;
> }
>
> - if (ha->mqenable && shost_use_blk_mq(host)) {
> + if (ha->mqenable) {
> /* number of hardware queues supported by blk/scsi-mq*/
> host->nr_hw_queues = ha->max_qpairs;
>
> @@ -3265,25 +3261,17 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
> base_vha->mgmt_svr_loop_id, host->sg_tablesize);
>
> if (ha->mqenable) {
> - bool mq = false;
> bool startit = false;
>
> - if (QLA_TGT_MODE_ENABLED()) {
> - mq = true;
> + if (QLA_TGT_MODE_ENABLED())
> startit = false;
> - }
>
> - if ((ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED) &&
> - shost_use_blk_mq(host)) {
> - mq = true;
> + if (ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED)
> startit = true;
> - }
This could just be
startit = (QLA_TGT_MODE_ENABLED() ||
(ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED));
>
> - if (mq) {
> - /* Create start of day qpairs for Block MQ */
> - for (i = 0; i < ha->max_qpairs; i++)
> - qla2xxx_create_qpair(base_vha, 5, 0, startit);
> - }
> + /* Create start of day qpairs for Block MQ */
> + for (i = 0; i < ha->max_qpairs; i++)
> + qla2xxx_create_qpair(base_vha, 5, 0, startit);
> }
>
> if (ha->flags.running_gold_fw)
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index fc1356d101b0..99db3f4316b5 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -780,11 +780,8 @@ MODULE_LICENSE("GPL");
> module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
> MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");
>
> -#ifdef CONFIG_SCSI_MQ_DEFAULT
> +/* Kill module parameter */
Is this a leftover todo comment for yourself, or a note for the future?
If the latter, I think it could be clearer.
> bool scsi_use_blk_mq = true;
> -#else
> -bool scsi_use_blk_mq = false;
> -#endif
> module_param_named(use_blk_mq, scsi_use_blk_mq, bool, S_IWUSR | S_IRUGO);
>
> static int __init init_scsi(void)
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 60bcc6df97a9..4740f1e9dd17 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -5881,8 +5881,7 @@ static int sdebug_driver_probe(struct device *dev)
> }
> /* Decide whether to tell scsi subsystem that we want mq */
> /* Following should give the same answer for each host */
> - if (shost_use_blk_mq(hpnt))
> - hpnt->nr_hw_queues = submit_queues;
> + hpnt->nr_hw_queues = submit_queues;
>
> sdbg_host->shost = hpnt;
> *((struct sdebug_host_info **)hpnt->hostdata) = sdbg_host;
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index c736d61b1648..fff128aa9ec2 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -308,7 +308,7 @@ enum blk_eh_timer_return scsi_times_out(struct request *req)
> * error handler. In that case we can return immediately as no
> * further action is required.
> */
> - if (req->q->mq_ops && !blk_mq_mark_complete(req))
> + if (!blk_mq_mark_complete(req))
> return rtn;
> if (scsi_abort_command(scmd) != SUCCESS) {
> set_host_byte(scmd, DID_TIME_OUT);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 8b0345924a92..651be30ba96a 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -687,37 +601,22 @@ static bool scsi_end_request(struct request *req, blk_status_t error,
> destroy_rcu_head(&cmd->rcu);
> }
>
> - if (req->mq_ctx) {
> - /*
> - * In the MQ case the command gets freed by __blk_mq_end_request,
> - * so we have to do all cleanup that depends on it earlier.
> - *
> - * We also can't kick the queues from irq context, so we
> - * will have to defer it to a workqueue.
> - */
> - scsi_mq_uninit_cmd(cmd);
> -
> - __blk_mq_end_request(req, error);
> -
> - if (scsi_target(sdev)->single_lun ||
> - !list_empty(&sdev->host->starved_list))
> - kblockd_schedule_work(&sdev->requeue_work);
> - else
> - blk_mq_run_hw_queues(q, true);
> - } else {
> - unsigned long flags;
> -
> - if (bidi_bytes)
> - scsi_release_bidi_buffers(cmd);
> - scsi_release_buffers(cmd);
> - scsi_put_command(cmd);
> + /*
> + * In the MQ case the command gets freed by __blk_mq_end_request,
> + * so we have to do all cleanup that depends on it earlier.
> + *
> + * We also can't kick the queues from irq context, so we
> + * will have to defer it to a workqueue.
> + */
This comment is slightly stale, since everything is the MQ case now.
> + scsi_mq_uninit_cmd(cmd);
>
> - spin_lock_irqsave(q->queue_lock, flags);
> - blk_finish_request(req, error);
> - spin_unlock_irqrestore(q->queue_lock, flags);
> + __blk_mq_end_request(req, error);
>
> - scsi_run_queue(q);
> - }
> + if (scsi_target(sdev)->single_lun ||
> + !list_empty(&sdev->host->starved_list))
> + kblockd_schedule_work(&sdev->requeue_work);
> + else
> + blk_mq_run_hw_queues(q, true);
>
> put_device(&sdev->sdev_gendev);
> return false;
> diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
> index 99f1db5e467e..5f21547b2ad2 100644
> --- a/drivers/scsi/scsi_priv.h
> +++ b/drivers/scsi/scsi_priv.h
> @@ -92,7 +92,6 @@ extern void scsi_queue_insert(struct scsi_cmnd *cmd, int reason);
> extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
> extern void scsi_run_host_queues(struct Scsi_Host *shost);
> extern void scsi_requeue_run_queue(struct work_struct *work);
> -extern struct request_queue *scsi_old_alloc_queue(struct scsi_device *sdev);
> extern struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev);
> extern void scsi_start_queue(struct scsi_device *sdev);
> extern int scsi_mq_setup_tags(struct Scsi_Host *shost);
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 78ca63dfba4a..dd0d516f65e2 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -266,10 +266,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
> */
> sdev->borken = 1;
>
> - if (shost_use_blk_mq(shost))
> - sdev->request_queue = scsi_mq_alloc_queue(sdev);
> - else
> - sdev->request_queue = scsi_old_alloc_queue(sdev);
> + sdev->request_queue = scsi_mq_alloc_queue(sdev);
> if (!sdev->request_queue) {
> /* release fn is set up in scsi_sysfs_device_initialise, so
> * have to free and put manually here */
> @@ -280,11 +277,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
> WARN_ON_ONCE(!blk_get_queue(sdev->request_queue));
> sdev->request_queue->queuedata = sdev;
>
> - if (!shost_use_blk_mq(sdev->host)) {
> - blk_queue_init_tags(sdev->request_queue,
> - sdev->host->cmd_per_lun, shost->bqt,
> - shost->hostt->tag_alloc_policy);
> - }
> scsi_change_queue_depth(sdev, sdev->host->cmd_per_lun ?
> sdev->host->cmd_per_lun : 1);
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 3aee9464a7bf..12e2c2829df2 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -367,7 +367,6 @@ store_shost_eh_deadline(struct device *dev, struct device_attribute *attr,
>
> static DEVICE_ATTR(eh_deadline, S_IRUGO | S_IWUSR, show_shost_eh_deadline, store_shost_eh_deadline);
>
> -shost_rd_attr(use_blk_mq, "%d\n");
> shost_rd_attr(unique_id, "%u\n");
> shost_rd_attr(cmd_per_lun, "%hd\n");
> shost_rd_attr(can_queue, "%hd\n");
> @@ -386,6 +385,13 @@ show_host_busy(struct device *dev, struct device_attribute *attr, char *buf)
> }
> static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL);
>
> +static ssize_t
> +show_use_blk_mq(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return snprintf(buf, 20, "1\n");
> +}
Looks like you forgot to change this to sprintf()
next prev parent reply other threads:[~2018-11-01 21:11 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-31 17:58 [PATCHSET v3 0/30] blk-mq driver conversions and legacy path removal Jens Axboe
2018-10-31 17:58 ` [PATCH 01/30] sunvdc: convert to blk-mq Jens Axboe
2018-10-31 17:58 ` [PATCH 02/30] ms_block: " Jens Axboe
2018-10-31 17:58 ` [PATCH 03/30] mspro_block: " Jens Axboe
2018-10-31 17:58 ` [PATCH 04/30] ide: " Jens Axboe
2018-10-31 17:58 ` [PATCH 05/30] blk-mq: remove the request_list usage Jens Axboe
2018-10-31 17:58 ` [PATCH 06/30] blk-mq: remove legacy check in queue blk_freeze_queue() Jens Axboe
2018-10-31 17:58 ` [PATCH 07/30] blk-mq: provide mq_ops->busy() hook Jens Axboe
2018-10-31 17:59 ` [PATCH 08/30] scsi: " Jens Axboe
2018-11-07 2:00 ` Martin K. Petersen
2018-10-31 17:59 ` [PATCH 09/30] scsi: kill off the legacy IO path Jens Axboe
2018-11-01 21:11 ` Omar Sandoval [this message]
2018-11-01 22:31 ` Jens Axboe
2018-11-07 2:07 ` Martin K. Petersen
2018-11-07 4:48 ` Jens Axboe
2018-11-09 2:28 ` Martin K. Petersen
2018-11-09 13:38 ` Jens Axboe
2018-10-31 17:59 ` [PATCH 10/30] block: remove q->lld_busy_fn() Jens Axboe
2018-10-31 17:59 ` [PATCH 11/30] dasd: remove dead code Jens Axboe
2018-10-31 17:59 ` [PATCH 12/30] bsg: pass in desired timeout handler Jens Axboe
2018-10-31 17:59 ` [PATCH 13/30] bsg: provide bsg_remove_queue() helper Jens Axboe
2018-10-31 17:59 ` [PATCH 14/30] bsg: convert to use blk-mq Jens Axboe
2018-10-31 17:59 ` [PATCH 15/30] block: remove blk_complete_request() Jens Axboe
2018-10-31 17:59 ` [PATCH 16/30] blk-wbt: kill check for legacy queue type Jens Axboe
2018-10-31 17:59 ` [PATCH 17/30] blk-cgroup: remove legacy queue bypassing Jens Axboe
2018-10-31 17:59 ` [PATCH 18/30] block: remove legacy rq tagging Jens Axboe
2018-10-31 17:59 ` [PATCH 19/30] block: remove non mq parts from the flush code Jens Axboe
2018-10-31 17:59 ` [PATCH 20/30] block: remove legacy IO schedulers Jens Axboe
2018-10-31 17:59 ` [PATCH 21/30] block: remove dead elevator code Jens Axboe
2018-11-01 21:26 ` Omar Sandoval
2018-11-01 22:32 ` Jens Axboe
2018-10-31 17:59 ` [PATCH 22/30] block: remove __blk_put_request() Jens Axboe
2018-10-31 17:59 ` [PATCH 23/30] block: kill legacy parts of timeout handling Jens Axboe
2018-10-31 17:59 ` [PATCH 24/30] bsg: move bsg-lib parts outside of request queue Jens Axboe
2018-10-31 17:59 ` [PATCH 25/30] block: remove request_list code Jens Axboe
2018-10-31 17:59 ` [PATCH 26/30] block: kill request slab cache Jens Axboe
2018-10-31 17:59 ` [PATCH 27/30] block: remove req_no_special_merge() from merging code Jens Axboe
2018-10-31 17:59 ` [PATCH 28/30] blk-merge: kill dead queue lock held check Jens Axboe
2018-10-31 17:59 ` [PATCH 29/30] block: get rid of blk_queued_rq() Jens Axboe
2018-10-31 17:59 ` [PATCH 30/30] block: get rid of q->softirq_done_fn() Jens Axboe
2018-11-01 2:35 ` [PATCHSET v3 0/30] blk-mq driver conversions and legacy path removal Ming Lei
2018-11-01 12:22 ` Jens Axboe
2018-11-01 21:27 ` Omar Sandoval
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181101211156.GE18005@vader \
--to=osandov@osandov.com \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox