From: Dexuan Cui <decui@microsoft.com>
To: Ming Lei <tom.leiming@gmail.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
Christoph Hellwig <hch@lst.de>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
Long Li <longli@microsoft.com>, vkuznets <vkuznets@redhat.com>,
Michael Kelley <mikelley@microsoft.com>,
KY Srinivasan <kys@microsoft.com>, Olaf Hering <olaf@aepfle.de>,
Stephen Hemminger <sthemmin@microsoft.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: SCSI low level driver: how to prevent I/O upon hibernation?
Date: Sat, 11 Apr 2020 06:01:39 +0000 [thread overview]
Message-ID: <HK0P153MB027320771C7A000B85BF3B97BFDF0@HK0P153MB0273.APCP153.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <CACVXFVO5Ni531JO+62CW4pV2y6gT98_8G=jiCJCZoqjkUBmo9Q@mail.gmail.com>
> From: Ming Lei <tom.leiming@gmail.com>
> Sent: Friday, April 10, 2020 12:43 AM
> To: Dexuan Cui <decui@microsoft.com>
>
> Hello Dexuan,
>
> On Fri, Apr 10, 2020 at 1:44 PM Dexuan Cui <decui@microsoft.com> wrote:
> >
> > Hi all,
> > Can you please recommend the standard way to prevent the upper layer SCSI
> > driver from submitting new I/O requests when the system is doing
> > hibernation?
> >
> > Actually I already asked the question on 5/30 last year ...
> > and I thought all the sdevs are suspended and resumed automatically in
> > drivers/scsi/scsi_pm.c, and the low level SCSI adapter driver (i.e. hv_storvsc)
> > only needs to suspend/resume the state of the adapter itself. However, it
> > looks
> > this is not true, because today I got such a panic in a v5.6 Linux VM running
> > on Hyper-V: the 'suspend' part of the hibernation process finished without
> > any issue, but when the VM was trying to resume back from the 'new'
> > kernel to the 'old' kernel, these events happened:
> >
> > 1. the new kernel loaded the saved state from disk to memory.
> >
> > 2. the new kernel quiesced the devices, including the SCSI DVD device
> > controlled by the hv_storvsc low level SCSI driver, i.e.
> > drivers/scsi/storvsc_drv.c: storvsc_suspend() was called and the related
> > vmbus ringbuffer was freed.
> >
> > 3. However, disk_events_workfn() -> ... -> cdrom_check_events() -> ...
> > -> scsi_queue_rq() -> ... -> storvsc_queuecommand() was still trying to
> > submit I/O commands to the freed vmbus ringbuffer, and as a result, a NULL
> > pointer dereference panic happened.
>
> Last time I replied to you in above link:
>
> "scsi_device_quiesce() has been called by scsi_dev_type_suspend() to prevent
> any non-pm request from entering queue."
>
> That meant no any normal FS request can enter scsi queue after suspend,
> however request with BLK_MQ_REQ_PREEMPT is still allowed to be queued
> to LLD after suspend.
>
> So you can't free related vmbus ringbuffer cause BLK_MQ_REQ_PREEMPT
> request is still to be handled.
>
> Thanks,
> Ming Lei
Actually I think I found the APIs with the help of Long Li:
scsi_host_block and scsi_host_unblock(). The new APIs were just added
on 2/28. :-)
Unluckily scsi_host_block() doesn't allow a state transition from
SDEV_QUIESCE to SDEV_BLOCK and returns -EINVAL for that, so I made the
below patch and it looks hibernation can work reliably for me now.
Please let me know if the change to scsi_device_set_state() is OK.
If the patch looks good, I plan to split and post it sometime next week.
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 47835c4b4ee0..06c260f6cdae 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2284,6 +2284,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
switch (oldstate) {
case SDEV_RUNNING:
case SDEV_CREATED_BLOCK:
+ case SDEV_QUIESCE:
case SDEV_OFFLINE:
break;
default:
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index fb41636519ee..fd51d2f03778 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1948,6 +1948,11 @@ static int storvsc_suspend(struct hv_device *hv_dev)
struct storvsc_device *stor_device = hv_get_drvdata(hv_dev);
struct Scsi_Host *host = stor_device->host;
struct hv_host_device *host_dev = shost_priv(host);
+ int ret;
+
+ ret = scsi_host_block(host);
+ if (ret)
+ return ret;
storvsc_wait_to_drain(stor_device);
@@ -1968,10 +1973,15 @@ static int storvsc_suspend(struct hv_device *hv_dev)
static int storvsc_resume(struct hv_device *hv_dev)
{
+ struct storvsc_device *stor_device = hv_get_drvdata(hv_dev);
+ struct Scsi_Host *host = stor_device->host;
int ret;
ret = storvsc_connect_to_vsp(hv_dev, storvsc_ringbuffer_size,
hv_dev_is_fc(hv_dev));
+ if (!ret)
+ ret = scsi_host_unblock(host, SDEV_RUNNING);
+
return ret;
}
Thanks,
-- Dexuan
next prev parent reply other threads:[~2020-04-11 6:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-10 5:44 SCSI low level driver: how to prevent I/O upon hibernation? Dexuan Cui
2020-04-10 7:42 ` Ming Lei
2020-04-11 6:01 ` Dexuan Cui [this message]
2020-04-11 15:03 ` Bart Van Assche
2020-04-11 17:20 ` Dexuan Cui
2020-04-14 17:09 ` Dexuan Cui
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=HK0P153MB027320771C7A000B85BF3B97BFDF0@HK0P153MB0273.APCP153.PROD.OUTLOOK.COM \
--to=decui@microsoft.com \
--cc=hch@lst.de \
--cc=kys@microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=longli@microsoft.com \
--cc=martin.petersen@oracle.com \
--cc=mikelley@microsoft.com \
--cc=olaf@aepfle.de \
--cc=sthemmin@microsoft.com \
--cc=tom.leiming@gmail.com \
--cc=vkuznets@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).