From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "jianchao.w.wang@oracle.com" <jianchao.w.wang@oracle.com>,
"axboe@kernel.dk" <axboe@kernel.dk>
Cc: "hch@lst.de" <hch@lst.de>,
"stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
"jthumshirn@suse.de" <jthumshirn@suse.de>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"hare@suse.com" <hare@suse.com>,
"ming.lei@redhat.com" <ming.lei@redhat.com>
Subject: Re: [PATCH v6 08/12] block, scsi: Introduce blk_pm_runtime_exit()
Date: Fri, 10 Aug 2018 16:17:52 +0000 [thread overview]
Message-ID: <01569014d645b6561a7c1da0f20d08b41a8a213a.camel@wdc.com> (raw)
In-Reply-To: <e993bdc480ccffc4922463fef7102b0cb2e542fd.camel@wdc.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-7", Size: 3309 bytes --]
On Fri, 2018-08-10 at 15:27 +-0000, Bart Van Assche wrote:
+AD4- On Fri, 2018-08-10 at 10:39 +-0800, jianchao.wang wrote:
+AD4- +AD4- On 08/10/2018 03:41 AM, Bart Van Assche wrote:
+AD4- +AD4- +AD4- +-void blk+AF8-pm+AF8-runtime+AF8-exit(struct request+AF8=
-queue +ACo-q)
+AD4- +AD4- +AD4- +-+AHs-
+AD4- +AD4- +AD4- +- if (+ACE-q-+AD4-dev)
+AD4- +AD4- +AD4- +- return+ADs-
+AD4- +AD4- +AD4- +-
+AD4- +AD4- +AD4- +- pm+AF8-runtime+AF8-get+AF8-sync(q-+AD4-dev)+ADs-
+AD4- +AD4- +AD4- +- q-+AD4-dev +AD0- NULL+ADs-
+AD4- +AD4- +AD4- +-+AH0-
+AD4- +AD4- +AD4- +-EXPORT+AF8-SYMBOL(blk+AF8-pm+AF8-runtime+AF8-exit)+ADs-
+AD4- +AD4- +AD4- +-
+AD4- +AD4- +AD4- /+ACoAKg-
+AD4- +AD4- +AD4- +ACo- blk+AF8-pre+AF8-runtime+AF8-suspend - Pre runtime=
suspend check
+AD4- +AD4- +AD4- +ACo- +AEA-q: the queue of the device
+AD4- +AD4- +AD4- diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
+AD4- +AD4- +AD4- index 69ab459abb98..5537762dfcfd 100644
+AD4- +AD4- +AD4- --- a/drivers/scsi/sd.c
+AD4- +AD4- +AD4- +-+-+- b/drivers/scsi/sd.c
+AD4- +AD4- +AD4- +AEAAQA- -3420,12 +-3420,11 +AEAAQA- static int sd+AF8-pr=
obe(struct device +ACo-dev)
+AD4- +AD4- +AD4- +ACoAKg-/
+AD4- +AD4- +AD4- static int sd+AF8-remove(struct device +ACo-dev)
+AD4- +AD4- +AD4- +AHs-
+AD4- +AD4- +AD4- - struct scsi+AF8-disk +ACo-sdkp+ADs-
+AD4- +AD4- +AD4- - dev+AF8-t devt+ADs-
+AD4- +AD4- +AD4- +- struct scsi+AF8-disk +ACo-sdkp +AD0- dev+AF8-get+AF8-d=
rvdata(dev)+ADs-
+AD4- +AD4- +AD4- +- struct scsi+AF8-device +ACo-sdp +AD0- sdkp-+AD4-device=
+ADs-
+AD4- +AD4- +AD4- +- dev+AF8-t devt +AD0- disk+AF8-devt(sdkp-+AD4-disk)+ADs=
-
+AD4- +AD4- +AD4- =20
+AD4- +AD4- +AD4- - sdkp +AD0- dev+AF8-get+AF8-drvdata(dev)+ADs-
+AD4- +AD4- +AD4- - devt +AD0- disk+AF8-devt(sdkp-+AD4-disk)+ADs-
+AD4- +AD4- +AD4- - scsi+AF8-autopm+AF8-get+AF8-device(sdkp-+AD4-device)+AD=
s-
+AD4- +AD4- +AD4- +- blk+AF8-pm+AF8-runtime+AF8-exit(sdp-+AD4-request+AF8-q=
ueue)
+AD4- +AD4-=20
+AD4- +AD4-=20
+AD4- +AD4- Based on +AF8AXw-scsi+AF8-device+AF8-remove, sd+AF8-remove will=
be invoked before blk+AF8-cleanup+AF8-queue.
+AD4- +AD4- So it should be not safe that we set the q-+AD4-dev to NULL her=
e.
+AD4- +AD4-=20
+AD4- +AD4- We have following operations in followed patch:
+AD4- +AD4- +-static inline void blk+AF8-pm+AF8-mark+AF8-last+AF8-busy(stru=
ct request +ACo-rq)
+AD4- +AD4- +-+AHs-
+AD4- +AD4- +- if (rq-+AD4-q-+AD4-dev +ACYAJg- +ACE-(rq-+AD4-rq+AF8-flags +=
ACY- RQF+AF8-PM))
+AD4- +AD4- +- pm+AF8-runtime+AF8-mark+AF8-last+AF8-busy(rq-+AD4-q-+AD4-de=
v)+ADs-
+AD4- +AD4- +-+AH0-
+AD4-=20
+AD4- How about moving the blk+AF8-pm+AF8-runtime+AF8-exit() call into blk+=
AF8-cleanup+AF8-queue() at
+AD4- a point where it is sure that there are no requests in progress? That=
should
+AD4- avoid that blk+AF8-pm+AF8-mark+AF8-last+AF8-busy() races against blk+=
AF8-pm+AF8-runtime+AF8-exit().
That wouldn't work because unbinding the SCSI ULD doesn't remove the reques=
t
queue. How about changing blk+AF8-pm+AF8-runtime+AF8-exit() into something =
like the following?
void blk+AF8-pm+AF8-runtime+AF8-exit(struct request+AF8-queue +ACo-q)
+AHs-
if (+ACE-q-+AD4-dev)
return+ADs-
pm+AF8-runtime+AF8-get+AF8-sync(q-+AD4-dev)+ADs-
blk+AF8-freeze+AF8-queue(q)+ADs-
q-+AD4-dev +AD0- NULL+ADs-
blk+AF8-unfreeze+AF8-queue(q)+ADs-
+AH0-
Bart.
next prev parent reply other threads:[~2018-08-10 16:17 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-09 19:41 [PATCH v6 00/12] blk-mq: Implement runtime power management Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 01/12] block, scsi: Introduce request flag RQF_DV Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 02/12] scsi: Alter handling of RQF_DV requests Bart Van Assche
2018-08-10 1:20 ` Ming Lei
2018-08-10 15:07 ` Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 03/12] scsi: Only set RQF_DV for requests used for domain validation Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 04/12] scsi: Introduce the SDEV_SUSPENDED device status Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 05/12] block, scsi: Rename QUEUE_FLAG_PREEMPT_ONLY into DV_ONLY and introduce PM_ONLY Bart Van Assche
2018-08-10 1:39 ` jianchao.wang
2018-08-10 15:18 ` Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 06/12] scsi: Reallow SPI domain validation during system suspend Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 07/12] block: Move power management code into a new source file Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 08/12] block, scsi: Introduce blk_pm_runtime_exit() Bart Van Assche
2018-08-10 2:39 ` jianchao.wang
2018-08-10 15:27 ` Bart Van Assche
2018-08-10 16:17 ` Bart Van Assche [this message]
2018-08-13 9:24 ` jianchao.wang
2018-08-13 16:09 ` Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 09/12] block: Split blk_pm_add_request() and blk_pm_put_request() Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 10/12] block: Change the runtime power management approach (1/2) Bart Van Assche
2018-08-10 1:59 ` jianchao.wang
2018-08-10 15:20 ` Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 11/12] block: Change the runtime power management approach (2/2) Bart Van Assche
2018-08-10 1:51 ` jianchao.wang
2018-08-10 15:22 ` Bart Van Assche
2018-08-09 19:41 ` [PATCH v6 12/12] blk-mq: Enable support for runtime power management Bart Van Assche
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=01569014d645b6561a7c1da0f20d08b41a8a213a.camel@wdc.com \
--to=bart.vanassche@wdc.com \
--cc=axboe@kernel.dk \
--cc=hare@suse.com \
--cc=hch@lst.de \
--cc=jianchao.w.wang@oracle.com \
--cc=jthumshirn@suse.de \
--cc=linux-block@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=ming.lei@redhat.com \
--cc=stern@rowland.harvard.edu \
/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