From: "jianchao.wang" <jianchao.w.wang@oracle.com>
To: Bart Van Assche <Bart.VanAssche@wdc.com>,
"jejb@linux.vnet.ibm.com" <jejb@linux.vnet.ibm.com>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"hch@lst.de" <hch@lst.de>
Subject: Re: [PATCH V2] scsi: core: use blk_mq_requeue_request in __scsi_queue_insert
Date: Thu, 1 Mar 2018 09:57:54 +0800 [thread overview]
Message-ID: <624cedc6-9658-7b76-9c63-61c30fdfe6be@oracle.com> (raw)
In-Reply-To: <1519840355.2777.8.camel@wdc.com>
Hi Bart
Thanks for your precious time to review this and kindly detailed response.
On 03/01/2018 01:52 AM, Bart Van Assche wrote:
> On Wed, 2018-02-28 at 16:55 +0800, Jianchao Wang wrote:
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index a86df9c..6fa7b0c 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -191,7 +191,8 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, bool unbusy)
>> */
>> cmd->result = 0;
>> if (q->mq_ops) {
>> - scsi_mq_requeue_cmd(cmd);
>> + blk_mq_requeue_request(cmd->request, true);
>> + put_device(&device->sdev_gendev);
>> return;
>> }
>> spin_lock_irqsave(q->queue_lock, flags);
>
> Anyone who sees the put_device() call that follows the blk_mq_requeue_request()
> call will wonder why that call occurs there. So I think we need a comment above
> that call that explains where the matching get_device() call is.
Yes, I will add this.
> For the legacy code path, there is a get_device() call in scsi_prep_fn() but no
> put_device() call in scsi_unprep_fn() - the matching put_device() calls occur in
> scsi_end_request() and after blk_requeue_request().
>
> For scsi-mq however there is a get_device() call in scsi_mq_get_budget() and a
> put_device() call in scsi_mq_put_budget(). So why do we need the put_device()
> calls after blk_mq_requeue_request() and in the mq path for scsi_end_request()?
>
>From the source code, we know the scsi_mq_get_budget will be invoked every time when we issue a request.
But scsi_mq_put_budget is just in the fail path.
scsi_queue_rq // if any error
-> scsi_mq_put_budget
blk_mq_dispatch_rq_list // if no driver tags
-> blk_mq_put_dispatch_budget
-> scsi_mq_put_budget
blk_mq_do_dispatch_sched/blk_mq_do_dispatch_ctx // if no requests
-> blk_mq_put_dispatch_budget
-> scsi_mq_put_budget
So we have to add put_device after blk_mq_requeue_request() and in scsi_end_request() to match the
scsi_mq_get_budget.
Thanks
Jianchao
next prev parent reply other threads:[~2018-03-01 1:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-28 8:55 [PATCH V2] scsi: core: use blk_mq_requeue_request in __scsi_queue_insert Jianchao Wang
2018-02-28 17:52 ` Bart Van Assche
2018-03-01 1:57 ` jianchao.wang [this message]
2018-03-01 17:43 ` Bart Van Assche
2018-03-02 1:43 ` Martin K. Petersen
2018-03-02 1:43 ` Martin K. Petersen
2018-03-02 2:17 ` jianchao.wang
2018-03-02 2:16 ` jianchao.wang
2018-03-02 1:44 ` Martin K. Petersen
2018-03-02 2:18 ` jianchao.wang
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=624cedc6-9658-7b76-9c63-61c30fdfe6be@oracle.com \
--to=jianchao.w.wang@oracle.com \
--cc=Bart.VanAssche@wdc.com \
--cc=hch@lst.de \
--cc=jejb@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.