* Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure [not found] ` <HK0P153MB0273B954294B331E20AACB41BFD20@HK0P153MB0273.APCP153.PROD.OUTLOOK.COM> @ 2020-04-22 2:01 ` Ming Lei 2020-04-22 3:08 ` Paul E. McKenney 0 siblings, 1 reply; 7+ messages in thread From: Ming Lei @ 2020-04-22 2:01 UTC (permalink / raw) To: Dexuan Cui, Paul E. McKenney, Josh Triplett, Rafael J. Wysocki Cc: jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm On Wed, Apr 22, 2020 at 01:48:25AM +0000, Dexuan Cui wrote: > > From: Ming Lei <ming.lei@redhat.com> > > Sent: Tuesday, April 21, 2020 6:28 PM > > To: Dexuan Cui <decui@microsoft.com> > > > > On Tue, Apr 21, 2020 at 05:17:24PM -0700, Dexuan Cui wrote: > > > During hibernation, the sdevs are suspended automatically in > > > drivers/scsi/scsi_pm.c before storvsc_suspend(), so after > > > storvsc_suspend(), there is no disk I/O from the file systems, but there > > > can still be disk I/O from the kernel space, e.g. disk_check_events() -> > > > sr_block_check_events() -> cdrom_check_events() can still submit I/O > > > to the storvsc driver, which causes a paic of NULL pointer dereference, > > > since storvsc has closed the vmbus channel in storvsc_suspend(): refer > > > to the below links for more info: > > > > > > Fix the panic by blocking/unblocking all the I/O queues properly. > > > > > > Note: this patch depends on another patch "scsi: core: Allow the state > > > change from SDEV_QUIESCE to SDEV_BLOCK" (refer to the second link > > above). > > > > > > Fixes: 56fb10585934 ("scsi: storvsc: Add the support of hibernation") > > > Signed-off-by: Dexuan Cui <decui@microsoft.com> > > > --- > > > drivers/scsi/storvsc_drv.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > 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; > > > } > > > > scsi_host_block() is actually too heavy for just avoiding > > scsi internal command, which can be done simply by one atomic > > variable. > > > > Not mention scsi_host_block() is implemented too clumsy because > > nr_luns * synchronize_rcu() are required in scsi_host_block(), > > which should have been optimized to just one. > > > > Also scsi_device_quiesce() is heavy too, still takes 2 > > synchronize_rcu() for one LUN. > > > > That is said SCSI suspend may take (3 * nr_luns) sysnchronize_rcu() in > > case that the HBA's suspend handler needs scsi_host_block(). > > > > Thanks, > > Ming > > When we're in storvsc_suspend(), all the userspace processes have been > frozen and all the file systems have been flushed, and there should not > be too much I/O from the kernel space, so IMO scsi_host_block() should be > pretty fast here. I guess it depends on RCU's implementation, so CC RCU guys. Hello Paul & Josh, Could you clarify that if sysnchronize_rcu becomes quickly during system suspend? Thanks, Ming ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure 2020-04-22 2:01 ` [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure Ming Lei @ 2020-04-22 3:08 ` Paul E. McKenney 2020-04-22 4:16 ` Ming Lei 0 siblings, 1 reply; 7+ messages in thread From: Paul E. McKenney @ 2020-04-22 3:08 UTC (permalink / raw) To: Ming Lei Cc: Dexuan Cui, Josh Triplett, Rafael J. Wysocki, jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm On Wed, Apr 22, 2020 at 10:01:34AM +0800, Ming Lei wrote: > On Wed, Apr 22, 2020 at 01:48:25AM +0000, Dexuan Cui wrote: > > > From: Ming Lei <ming.lei@redhat.com> > > > Sent: Tuesday, April 21, 2020 6:28 PM > > > To: Dexuan Cui <decui@microsoft.com> > > > > > > On Tue, Apr 21, 2020 at 05:17:24PM -0700, Dexuan Cui wrote: > > > > During hibernation, the sdevs are suspended automatically in > > > > drivers/scsi/scsi_pm.c before storvsc_suspend(), so after > > > > storvsc_suspend(), there is no disk I/O from the file systems, but there > > > > can still be disk I/O from the kernel space, e.g. disk_check_events() -> > > > > sr_block_check_events() -> cdrom_check_events() can still submit I/O > > > > to the storvsc driver, which causes a paic of NULL pointer dereference, > > > > since storvsc has closed the vmbus channel in storvsc_suspend(): refer > > > > to the below links for more info: > > > > > > > > Fix the panic by blocking/unblocking all the I/O queues properly. > > > > > > > > Note: this patch depends on another patch "scsi: core: Allow the state > > > > change from SDEV_QUIESCE to SDEV_BLOCK" (refer to the second link > > > above). > > > > > > > > Fixes: 56fb10585934 ("scsi: storvsc: Add the support of hibernation") > > > > Signed-off-by: Dexuan Cui <decui@microsoft.com> > > > > --- > > > > drivers/scsi/storvsc_drv.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > 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; > > > > } > > > > > > scsi_host_block() is actually too heavy for just avoiding > > > scsi internal command, which can be done simply by one atomic > > > variable. > > > > > > Not mention scsi_host_block() is implemented too clumsy because > > > nr_luns * synchronize_rcu() are required in scsi_host_block(), > > > which should have been optimized to just one. > > > > > > Also scsi_device_quiesce() is heavy too, still takes 2 > > > synchronize_rcu() for one LUN. > > > > > > That is said SCSI suspend may take (3 * nr_luns) sysnchronize_rcu() in > > > case that the HBA's suspend handler needs scsi_host_block(). > > > > > > Thanks, > > > Ming > > > > When we're in storvsc_suspend(), all the userspace processes have been > > frozen and all the file systems have been flushed, and there should not > > be too much I/O from the kernel space, so IMO scsi_host_block() should be > > pretty fast here. > > I guess it depends on RCU's implementation, so CC RCU guys. > > Hello Paul & Josh, > > Could you clarify that if sysnchronize_rcu becomes quickly during > system suspend? Once you have all but one CPU offlined, it becomes extremely fast, as in roughly a no-op (which is an idea of Josh's from back in the day). But if there is more than one CPU online, then synchronize_rcu() still takes on the order of several to several tens of jiffies. So, yes, in some portions of system suspend, synchronize_rcu() becomes very fast indeed. Thanx, Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure 2020-04-22 3:08 ` Paul E. McKenney @ 2020-04-22 4:16 ` Ming Lei 2020-04-22 4:58 ` Dexuan Cui 0 siblings, 1 reply; 7+ messages in thread From: Ming Lei @ 2020-04-22 4:16 UTC (permalink / raw) To: Paul E. McKenney Cc: Dexuan Cui, Josh Triplett, Rafael J. Wysocki, jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm On Tue, Apr 21, 2020 at 08:08:07PM -0700, Paul E. McKenney wrote: > On Wed, Apr 22, 2020 at 10:01:34AM +0800, Ming Lei wrote: > > On Wed, Apr 22, 2020 at 01:48:25AM +0000, Dexuan Cui wrote: > > > > From: Ming Lei <ming.lei@redhat.com> > > > > Sent: Tuesday, April 21, 2020 6:28 PM > > > > To: Dexuan Cui <decui@microsoft.com> > > > > > > > > On Tue, Apr 21, 2020 at 05:17:24PM -0700, Dexuan Cui wrote: > > > > > During hibernation, the sdevs are suspended automatically in > > > > > drivers/scsi/scsi_pm.c before storvsc_suspend(), so after > > > > > storvsc_suspend(), there is no disk I/O from the file systems, but there > > > > > can still be disk I/O from the kernel space, e.g. disk_check_events() -> > > > > > sr_block_check_events() -> cdrom_check_events() can still submit I/O > > > > > to the storvsc driver, which causes a paic of NULL pointer dereference, > > > > > since storvsc has closed the vmbus channel in storvsc_suspend(): refer > > > > > to the below links for more info: > > > > > > > > > > Fix the panic by blocking/unblocking all the I/O queues properly. > > > > > > > > > > Note: this patch depends on another patch "scsi: core: Allow the state > > > > > change from SDEV_QUIESCE to SDEV_BLOCK" (refer to the second link > > > > above). > > > > > > > > > > Fixes: 56fb10585934 ("scsi: storvsc: Add the support of hibernation") > > > > > Signed-off-by: Dexuan Cui <decui@microsoft.com> > > > > > --- > > > > > drivers/scsi/storvsc_drv.c | 10 ++++++++++ > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > 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; > > > > > } > > > > > > > > scsi_host_block() is actually too heavy for just avoiding > > > > scsi internal command, which can be done simply by one atomic > > > > variable. > > > > > > > > Not mention scsi_host_block() is implemented too clumsy because > > > > nr_luns * synchronize_rcu() are required in scsi_host_block(), > > > > which should have been optimized to just one. > > > > > > > > Also scsi_device_quiesce() is heavy too, still takes 2 > > > > synchronize_rcu() for one LUN. > > > > > > > > That is said SCSI suspend may take (3 * nr_luns) sysnchronize_rcu() in > > > > case that the HBA's suspend handler needs scsi_host_block(). > > > > > > > > Thanks, > > > > Ming > > > > > > When we're in storvsc_suspend(), all the userspace processes have been > > > frozen and all the file systems have been flushed, and there should not > > > be too much I/O from the kernel space, so IMO scsi_host_block() should be > > > pretty fast here. > > > > I guess it depends on RCU's implementation, so CC RCU guys. > > > > Hello Paul & Josh, > > > > Could you clarify that if sysnchronize_rcu becomes quickly during > > system suspend? > > Once you have all but one CPU offlined, it becomes extremely fast, as > in roughly a no-op (which is an idea of Josh's from back in the day). > But if there is more than one CPU online, then synchronize_rcu() still > takes on the order of several to several tens of jiffies. > > So, yes, in some portions of system suspend, synchronize_rcu() becomes > very fast indeed. Hi Paul, Thanks for your clarification. In system suspend path, device is suspended before suspend_disable_secondary_cpus(), so I guess synchronize_rcu() is not quick enough even though user space processes and some kernel threads are frozen. Thanks, Ming ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure 2020-04-22 4:16 ` Ming Lei @ 2020-04-22 4:58 ` Dexuan Cui 2020-04-22 9:23 ` Ming Lei 0 siblings, 1 reply; 7+ messages in thread From: Dexuan Cui @ 2020-04-22 4:58 UTC (permalink / raw) To: Ming Lei, Paul E. McKenney Cc: Josh Triplett, Rafael J. Wysocki, jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm@vger.kernel.org > From: Ming Lei <ming.lei@redhat.com> > Sent: Tuesday, April 21, 2020 9:16 PM > ... > > > > When we're in storvsc_suspend(), all the userspace processes have been > > > > frozen and all the file systems have been flushed, and there should not > > > > be too much I/O from the kernel space, so IMO scsi_host_block() should > be > > > > pretty fast here. > > > > > > I guess it depends on RCU's implementation, so CC RCU guys. > > > > > > Hello Paul & Josh, > > > > > > Could you clarify that if sysnchronize_rcu becomes quickly during > > > system suspend? > > > > Once you have all but one CPU offlined, it becomes extremely fast, as > > in roughly a no-op (which is an idea of Josh's from back in the day). > > But if there is more than one CPU online, then synchronize_rcu() still > > takes on the order of several to several tens of jiffies. > > > > So, yes, in some portions of system suspend, synchronize_rcu() becomes > > very fast indeed. > > Hi Paul, > > Thanks for your clarification. > > In system suspend path, device is suspended before > suspend_disable_secondary_cpus(), > so I guess synchronize_rcu() is not quick enough even though user space > processes and some kernel threads are frozen. > > Thanks, > Ming storvsc_suspend() -> scsi_host_block() is only called in the hibernation path, which is not a hot path at all, so IMHO we don't really care if it takes 10ms or 100ms or even 1s. :-) BTW, in my test, typically the scsi_host_block() here takes about 3ms in my 40-vCPU VM. storvsc_suspend() is not called from the runtime PM path, because the runtime_suspend/runtime_resume/runtime_idle ops are not defined at all for the devices on the Hyper-V VMBus bus: these are pure software-emulated devices, so runtime PM is unnecessary for them. Thanks, -- Dexuan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure 2020-04-22 4:58 ` Dexuan Cui @ 2020-04-22 9:23 ` Ming Lei 2020-04-22 16:19 ` Paul E. McKenney 2020-04-22 18:01 ` Dexuan Cui 0 siblings, 2 replies; 7+ messages in thread From: Ming Lei @ 2020-04-22 9:23 UTC (permalink / raw) To: Dexuan Cui Cc: Paul E. McKenney, Josh Triplett, Rafael J. Wysocki, jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm@vger.kernel.org On Wed, Apr 22, 2020 at 04:58:14AM +0000, Dexuan Cui wrote: > > From: Ming Lei <ming.lei@redhat.com> > > Sent: Tuesday, April 21, 2020 9:16 PM > > ... > > > > > When we're in storvsc_suspend(), all the userspace processes have been > > > > > frozen and all the file systems have been flushed, and there should not > > > > > be too much I/O from the kernel space, so IMO scsi_host_block() should > > be > > > > > pretty fast here. > > > > > > > > I guess it depends on RCU's implementation, so CC RCU guys. > > > > > > > > Hello Paul & Josh, > > > > > > > > Could you clarify that if sysnchronize_rcu becomes quickly during > > > > system suspend? > > > > > > Once you have all but one CPU offlined, it becomes extremely fast, as > > > in roughly a no-op (which is an idea of Josh's from back in the day). > > > But if there is more than one CPU online, then synchronize_rcu() still > > > takes on the order of several to several tens of jiffies. > > > > > > So, yes, in some portions of system suspend, synchronize_rcu() becomes > > > very fast indeed. > > > > Hi Paul, > > > > Thanks for your clarification. > > > > In system suspend path, device is suspended before > > suspend_disable_secondary_cpus(), > > so I guess synchronize_rcu() is not quick enough even though user space > > processes and some kernel threads are frozen. > > > > Thanks, > > Ming > > storvsc_suspend() -> scsi_host_block() is only called in the hibernation > path, which is not a hot path at all, so IMHO we don't really care if it > takes 10ms or 100ms or even 1s. :-) BTW, in my test, typically the Are you sure the 'we' can cover all users? > scsi_host_block() here takes about 3ms in my 40-vCPU VM. If more LUNs are added, the time should be increased proportionallly, that is why I think scsi_host_block() is bad. Thanks, Ming ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure 2020-04-22 9:23 ` Ming Lei @ 2020-04-22 16:19 ` Paul E. McKenney 2020-04-22 18:01 ` Dexuan Cui 1 sibling, 0 replies; 7+ messages in thread From: Paul E. McKenney @ 2020-04-22 16:19 UTC (permalink / raw) To: Ming Lei Cc: Dexuan Cui, Josh Triplett, Rafael J. Wysocki, jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm@vger.kernel.org On Wed, Apr 22, 2020 at 05:23:51PM +0800, Ming Lei wrote: > On Wed, Apr 22, 2020 at 04:58:14AM +0000, Dexuan Cui wrote: > > > From: Ming Lei <ming.lei@redhat.com> > > > Sent: Tuesday, April 21, 2020 9:16 PM > > > ... > > > > > > When we're in storvsc_suspend(), all the userspace processes have been > > > > > > frozen and all the file systems have been flushed, and there should not > > > > > > be too much I/O from the kernel space, so IMO scsi_host_block() should > > > be > > > > > > pretty fast here. > > > > > > > > > > I guess it depends on RCU's implementation, so CC RCU guys. > > > > > > > > > > Hello Paul & Josh, > > > > > > > > > > Could you clarify that if sysnchronize_rcu becomes quickly during > > > > > system suspend? > > > > > > > > Once you have all but one CPU offlined, it becomes extremely fast, as > > > > in roughly a no-op (which is an idea of Josh's from back in the day). > > > > But if there is more than one CPU online, then synchronize_rcu() still > > > > takes on the order of several to several tens of jiffies. > > > > > > > > So, yes, in some portions of system suspend, synchronize_rcu() becomes > > > > very fast indeed. > > > > > > Hi Paul, > > > > > > Thanks for your clarification. > > > > > > In system suspend path, device is suspended before > > > suspend_disable_secondary_cpus(), > > > so I guess synchronize_rcu() is not quick enough even though user space > > > processes and some kernel threads are frozen. > > > > > > Thanks, > > > Ming > > > > storvsc_suspend() -> scsi_host_block() is only called in the hibernation > > path, which is not a hot path at all, so IMHO we don't really care if it > > takes 10ms or 100ms or even 1s. :-) BTW, in my test, typically the > > Are you sure the 'we' can cover all users? > > > scsi_host_block() here takes about 3ms in my 40-vCPU VM. > > If more LUNs are added, the time should be increased proportionallly, > that is why I think scsi_host_block() is bad. If the caller must wait until the grace period ends, then the traditional approach is to use a single synchronize_rcu() to cover all LUNs. This of course can require some reworking of the code. If the caller does not need to wait, then either call_rcu() or kfree_rcu() can work quite well. Thanx, Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure 2020-04-22 9:23 ` Ming Lei 2020-04-22 16:19 ` Paul E. McKenney @ 2020-04-22 18:01 ` Dexuan Cui 1 sibling, 0 replies; 7+ messages in thread From: Dexuan Cui @ 2020-04-22 18:01 UTC (permalink / raw) To: Ming Lei Cc: Paul E. McKenney, Josh Triplett, Rafael J. Wysocki, jejb@linux.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de, bvanassche@acm.org, hare@suse.de, Michael Kelley, Long Li, linux-hyperv@vger.kernel.org, wei.liu@kernel.org, Stephen Hemminger, Haiyang Zhang, KY Srinivasan, linux-pm@vger.kernel.org > From: Ming Lei <ming.lei@redhat.com> > Sent: Wednesday, April 22, 2020 2:24 AM > > ... > > storvsc_suspend() -> scsi_host_block() is only called in the hibernation > > path, which is not a hot path at all, so IMHO we don't really care if it > > takes 10ms or 100ms or even 1s. :-) BTW, in my test, typically the > > Are you sure the 'we' can cover all users? I actully only meant I don't have any performance concern in this path storvsc_suspend() -> scsi_host_block() :-) Thanks for your effort to improve the scsi_host_block() API in the patch [PATCH] scsi: avoid to run synchronize_rcu for each device in scsi_host_block. > > scsi_host_block() here takes about 3ms in my 40-vCPU VM. > > If more LUNs are added, the time should be increased proportionallly, > that is why I think scsi_host_block() is bad. > > Thanks, > Ming FWIW, if the time increases linearly against the number of the LUNs, it looks typically it's still fast enough for storvsc_suspend(). Thanks, -- Dexuan ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-22 18:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1587514644-47058-1-git-send-email-decui@microsoft.com>
[not found] ` <20200422012814.GB299948@T590>
[not found] ` <HK0P153MB0273B954294B331E20AACB41BFD20@HK0P153MB0273.APCP153.PROD.OUTLOOK.COM>
2020-04-22 2:01 ` [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure Ming Lei
2020-04-22 3:08 ` Paul E. McKenney
2020-04-22 4:16 ` Ming Lei
2020-04-22 4:58 ` Dexuan Cui
2020-04-22 9:23 ` Ming Lei
2020-04-22 16:19 ` Paul E. McKenney
2020-04-22 18:01 ` Dexuan Cui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox