* [PATCH 0/2] Fix a race condition between SPI domain validation and system suspend
@ 2018-01-05 17:19 Bart Van Assche
2018-01-05 17:19 ` [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules Bart Van Assche
2018-01-05 17:19 ` [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
0 siblings, 2 replies; 8+ messages in thread
From: Bart Van Assche @ 2018-01-05 17:19 UTC (permalink / raw
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-pm, Rafael J . Wysocki, Woody Suwalski,
Alan Stern, Bart Van Assche
Hello Martin,
Recently it was discovered that the v4.15 rework of suspend/resume handling
in the SCSI subsystem introduced a race condition between SPI domain validation
and system suspend. The two patches in this series fix that race. Please
consider these two patches for kernel v4.15.
Thanks,
Bart.
Bart Van Assche (2):
PM / sleep: Make lock/unlock_system_sleep() available to kernel
modules
Fix a race condition between SPI domain validation and system suspend
drivers/scsi/scsi_transport_spi.c | 16 ++++++++++++++--
include/linux/suspend.h | 28 ++--------------------------
kernel/power/main.c | 29 +++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 28 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules
2018-01-05 17:19 [PATCH 0/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
@ 2018-01-05 17:19 ` Bart Van Assche
2018-01-05 23:00 ` Rafael J. Wysocki
2018-01-05 17:19 ` [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
1 sibling, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2018-01-05 17:19 UTC (permalink / raw
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-pm, Rafael J . Wysocki, Woody Suwalski,
Alan Stern, Bart Van Assche
Since pm_mutex is not exported using lock/unlock_system_sleep() from
inside a kernel module causes a "pm_mutex undefined" linker error.
Hence move lock/unlock_system_sleep() into kernel/power/main.c and
export these.
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
---
include/linux/suspend.h | 28 ++--------------------------
kernel/power/main.c | 29 +++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index d60b0f5c38d5..cc22a24516d6 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -443,32 +443,8 @@ extern bool pm_save_wakeup_count(unsigned int count);
extern void pm_wakep_autosleep_enabled(bool set);
extern void pm_print_active_wakeup_sources(void);
-static inline void lock_system_sleep(void)
-{
- current->flags |= PF_FREEZER_SKIP;
- mutex_lock(&pm_mutex);
-}
-
-static inline void unlock_system_sleep(void)
-{
- /*
- * Don't use freezer_count() because we don't want the call to
- * try_to_freeze() here.
- *
- * Reason:
- * Fundamentally, we just don't need it, because freezing condition
- * doesn't come into effect until we release the pm_mutex lock,
- * since the freezer always works with pm_mutex held.
- *
- * More importantly, in the case of hibernation,
- * unlock_system_sleep() gets called in snapshot_read() and
- * snapshot_write() when the freezing condition is still in effect.
- * Which means, if we use try_to_freeze() here, it would make them
- * enter the refrigerator, thus causing hibernation to lockup.
- */
- current->flags &= ~PF_FREEZER_SKIP;
- mutex_unlock(&pm_mutex);
-}
+extern void lock_system_sleep(void);
+extern void unlock_system_sleep(void);
#else /* !CONFIG_PM_SLEEP */
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3a2ca9066583..705c2366dafe 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -22,6 +22,35 @@ DEFINE_MUTEX(pm_mutex);
#ifdef CONFIG_PM_SLEEP
+void lock_system_sleep(void)
+{
+ current->flags |= PF_FREEZER_SKIP;
+ mutex_lock(&pm_mutex);
+}
+EXPORT_SYMBOL_GPL(lock_system_sleep);
+
+void unlock_system_sleep(void)
+{
+ /*
+ * Don't use freezer_count() because we don't want the call to
+ * try_to_freeze() here.
+ *
+ * Reason:
+ * Fundamentally, we just don't need it, because freezing condition
+ * doesn't come into effect until we release the pm_mutex lock,
+ * since the freezer always works with pm_mutex held.
+ *
+ * More importantly, in the case of hibernation,
+ * unlock_system_sleep() gets called in snapshot_read() and
+ * snapshot_write() when the freezing condition is still in effect.
+ * Which means, if we use try_to_freeze() here, it would make them
+ * enter the refrigerator, thus causing hibernation to lockup.
+ */
+ current->flags &= ~PF_FREEZER_SKIP;
+ mutex_unlock(&pm_mutex);
+}
+EXPORT_SYMBOL_GPL(unlock_system_sleep);
+
/* Routines for PM-transition notifications */
static BLOCKING_NOTIFIER_HEAD(pm_chain_head);
--
2.15.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend
2018-01-05 17:19 [PATCH 0/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
2018-01-05 17:19 ` [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules Bart Van Assche
@ 2018-01-05 17:19 ` Bart Van Assche
2018-01-08 16:02 ` Martin K. Petersen
1 sibling, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2018-01-05 17:19 UTC (permalink / raw
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, linux-pm, Rafael J . Wysocki, Woody Suwalski,
Alan Stern, Bart Van Assche
Avoid that the following warning is reported when suspending a system
that is using the mptspi driver:
WARNING: CPU: 0 PID: 4187 at drivers/scsi/scsi_lib.c:2960 scsi_device_quiesce+0x20/0xb0
EIP: scsi_device_quiesce+0x20/0xb0
Call Trace:
spi_dv_device+0x65/0x5f0 [scsi_transport_spi]
mptspi_dv_device+0x4d/0x170 [mptspi]
mptspi_dv_renegotiate_work+0x49/0xc0 [mptspi]
process_one_work+0x190/0x2e0
worker_thread+0x37/0x3f0
kthread+0xcb/0x100
ret_from_fork+0x19/0x24
Fixes: 3a0a529971ec ("block, scsi: Make SCSI quiesce and resume work reliably")
Reported-by: Woody Suwalski <terraluna977@gmail.com>
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
---
drivers/scsi/scsi_transport_spi.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
index 10ebb213ddb3..871ea582029e 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -26,6 +26,7 @@
#include <linux/mutex.h>
#include <linux/sysfs.h>
#include <linux/slab.h>
+#include <linux/suspend.h>
#include <scsi/scsi.h>
#include "scsi_priv.h"
#include <scsi/scsi_device.h>
@@ -1009,11 +1010,20 @@ spi_dv_device(struct scsi_device *sdev)
u8 *buffer;
const int len = SPI_MAX_ECHO_BUFFER_SIZE*2;
+ /*
+ * Because this function and the power management code both call
+ * scsi_device_quiesce(), it is not safe to perform domain validation
+ * while suspend or resume is in progress. Hence the
+ * lock/unlock_system_sleep() calls.
+ */
+ lock_system_sleep();
+
if (unlikely(spi_dv_in_progress(starget)))
- return;
+ goto unlock;
if (unlikely(scsi_device_get(sdev)))
- return;
+ goto unlock;
+
spi_dv_in_progress(starget) = 1;
buffer = kzalloc(len, GFP_KERNEL);
@@ -1049,6 +1059,8 @@ spi_dv_device(struct scsi_device *sdev)
out_put:
spi_dv_in_progress(starget) = 0;
scsi_device_put(sdev);
+unlock:
+ unlock_system_sleep();
}
EXPORT_SYMBOL(spi_dv_device);
--
2.15.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules
2018-01-05 17:19 ` [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules Bart Van Assche
@ 2018-01-05 23:00 ` Rafael J. Wysocki
2018-01-05 23:32 ` Bart Van Assche
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2018-01-05 23:00 UTC (permalink / raw
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley,
open list:TARGET SUBSYSTEM, Linux PM, Rafael J . Wysocki,
Woody Suwalski, Alan Stern
On Fri, Jan 5, 2018 at 6:19 PM, Bart Van Assche <bart.vanassche@wdc.com> wrote:
> Since pm_mutex is not exported using lock/unlock_system_sleep() from
> inside a kernel module causes a "pm_mutex undefined" linker error.
> Hence move lock/unlock_system_sleep() into kernel/power/main.c and
> export these.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> ---
> include/linux/suspend.h | 28 ++--------------------------
> kernel/power/main.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index d60b0f5c38d5..cc22a24516d6 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -443,32 +443,8 @@ extern bool pm_save_wakeup_count(unsigned int count);
> extern void pm_wakep_autosleep_enabled(bool set);
> extern void pm_print_active_wakeup_sources(void);
>
> -static inline void lock_system_sleep(void)
> -{
> - current->flags |= PF_FREEZER_SKIP;
> - mutex_lock(&pm_mutex);
> -}
> -
> -static inline void unlock_system_sleep(void)
> -{
> - /*
> - * Don't use freezer_count() because we don't want the call to
> - * try_to_freeze() here.
> - *
> - * Reason:
> - * Fundamentally, we just don't need it, because freezing condition
> - * doesn't come into effect until we release the pm_mutex lock,
> - * since the freezer always works with pm_mutex held.
> - *
> - * More importantly, in the case of hibernation,
> - * unlock_system_sleep() gets called in snapshot_read() and
> - * snapshot_write() when the freezing condition is still in effect.
> - * Which means, if we use try_to_freeze() here, it would make them
> - * enter the refrigerator, thus causing hibernation to lockup.
> - */
> - current->flags &= ~PF_FREEZER_SKIP;
> - mutex_unlock(&pm_mutex);
> -}
> +extern void lock_system_sleep(void);
> +extern void unlock_system_sleep(void);
>
> #else /* !CONFIG_PM_SLEEP */
Don't you need to add stubs for this case too?
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 3a2ca9066583..705c2366dafe 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -22,6 +22,35 @@ DEFINE_MUTEX(pm_mutex);
>
> #ifdef CONFIG_PM_SLEEP
>
> +void lock_system_sleep(void)
> +{
> + current->flags |= PF_FREEZER_SKIP;
> + mutex_lock(&pm_mutex);
> +}
> +EXPORT_SYMBOL_GPL(lock_system_sleep);
> +
> +void unlock_system_sleep(void)
> +{
> + /*
> + * Don't use freezer_count() because we don't want the call to
> + * try_to_freeze() here.
> + *
> + * Reason:
> + * Fundamentally, we just don't need it, because freezing condition
> + * doesn't come into effect until we release the pm_mutex lock,
> + * since the freezer always works with pm_mutex held.
> + *
> + * More importantly, in the case of hibernation,
> + * unlock_system_sleep() gets called in snapshot_read() and
> + * snapshot_write() when the freezing condition is still in effect.
> + * Which means, if we use try_to_freeze() here, it would make them
> + * enter the refrigerator, thus causing hibernation to lockup.
> + */
> + current->flags &= ~PF_FREEZER_SKIP;
> + mutex_unlock(&pm_mutex);
> +}
> +EXPORT_SYMBOL_GPL(unlock_system_sleep);
> +
> /* Routines for PM-transition notifications */
>
> static BLOCKING_NOTIFIER_HEAD(pm_chain_head);
> --
The changes are fine by me, but I really would prefer them to go in
via the PM tree which I guess means that the second patch would need
to go in this way too.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules
2018-01-05 23:00 ` Rafael J. Wysocki
@ 2018-01-05 23:32 ` Bart Van Assche
2018-01-05 23:38 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2018-01-05 23:32 UTC (permalink / raw
To: rafael@kernel.org
Cc: jejb@linux.vnet.ibm.com, linux-scsi@vger.kernel.org,
terraluna977@gmail.com, stern@rowland.harvard.edu,
linux-pm@vger.kernel.org, martin.petersen@oracle.com,
rjw@rjwysocki.net
On Sat, 2018-01-06 at 00:00 +0100, Rafael J. Wysocki wrote:
> The changes are fine by me, but I really would prefer them to go in
> via the PM tree which I guess means that the second patch would need
> to go in this way too.
Hello Rafael,
If I can get a Tested-by from Woody and an Acked-by from Martin then I can
do that.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules
2018-01-05 23:32 ` Bart Van Assche
@ 2018-01-05 23:38 ` Rafael J. Wysocki
0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2018-01-05 23:38 UTC (permalink / raw
To: Bart Van Assche
Cc: rafael@kernel.org, jejb@linux.vnet.ibm.com,
linux-scsi@vger.kernel.org, terraluna977@gmail.com,
stern@rowland.harvard.edu, linux-pm@vger.kernel.org,
martin.petersen@oracle.com, rjw@rjwysocki.net
On Sat, Jan 6, 2018 at 12:32 AM, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Sat, 2018-01-06 at 00:00 +0100, Rafael J. Wysocki wrote:
>> The changes are fine by me, but I really would prefer them to go in
>> via the PM tree which I guess means that the second patch would need
>> to go in this way too.
>
> Hello Rafael,
>
> If I can get a Tested-by from Woody and an Acked-by from Martin then I can
> do that.
That would be an ACK from Marting for the second patch.
OK
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend
2018-01-05 17:19 ` [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
@ 2018-01-08 16:02 ` Martin K. Petersen
2018-01-09 0:27 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Martin K. Petersen @ 2018-01-08 16:02 UTC (permalink / raw
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
linux-pm, Rafael J . Wysocki, Woody Suwalski, Alan Stern
Bart,
> Avoid that the following warning is reported when suspending a system
> that is using the mptspi driver:
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend
2018-01-08 16:02 ` Martin K. Petersen
@ 2018-01-09 0:27 ` Rafael J. Wysocki
0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2018-01-09 0:27 UTC (permalink / raw
To: Martin K. Petersen, Bart Van Assche
Cc: James E . J . Bottomley, linux-scsi, linux-pm, Woody Suwalski,
Alan Stern
On Monday, January 8, 2018 5:02:53 PM CET Martin K. Petersen wrote:
>
> Bart,
>
> > Avoid that the following warning is reported when suspending a system
> > that is using the mptspi driver:
>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Thanks!
Let me take both patches to the linux-pm tree then.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-01-09 0:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-05 17:19 [PATCH 0/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
2018-01-05 17:19 ` [PATCH 1/2] PM / sleep: Make lock/unlock_system_sleep() available to kernel modules Bart Van Assche
2018-01-05 23:00 ` Rafael J. Wysocki
2018-01-05 23:32 ` Bart Van Assche
2018-01-05 23:38 ` Rafael J. Wysocki
2018-01-05 17:19 ` [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend Bart Van Assche
2018-01-08 16:02 ` Martin K. Petersen
2018-01-09 0:27 ` Rafael J. Wysocki
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.