With the current implementation the following race can happen:
* blk_pre_runtime_suspend() calls blk_freeze_queue_start() and
blk_mq_unfreeze_queue().
* blk_queue_enter() calls blk_queue_pm_only() and that function returns
true.
* blk_queue_enter() calls blk_pm_request_resume() and that function does
not call pm_request_resume() because the queue runtime status is
RPM_ACTIVE.
* blk_pre_runtime_suspend() changes the queue status into RPM_SUSPENDING.
Fix this race by changing the queue runtime status into RPM_SUSPENDING
before switching q_usage_counter to atomic mode.
Acked-by: Alan Stern <stern(a)rowland.harvard.edu>
Acked-by: Stanley Chu <stanley.chu(a)mediatek.com>
Reviewed-by: Christoph Hellwig <hch(a)lst.de>
Cc: Ming Lei <ming.lei(a)redhat.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
Cc: stable <stable(a)vger.kernel.org>
Fixes: 986d413b7c15 ("blk-mq: Enable support for runtime power management")
Signed-off-by: Can Guo <cang(a)codeaurora.org>
Signed-off-by: Bart Van Assche <bvanassche(a)acm.org>
---
block/blk-pm.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/block/blk-pm.c b/block/blk-pm.c
index b85234d758f7..17bd020268d4 100644
--- a/block/blk-pm.c
+++ b/block/blk-pm.c
@@ -67,6 +67,10 @@ int blk_pre_runtime_suspend(struct request_queue *q)
WARN_ON_ONCE(q->rpm_status != RPM_ACTIVE);
+ spin_lock_irq(&q->queue_lock);
+ q->rpm_status = RPM_SUSPENDING;
+ spin_unlock_irq(&q->queue_lock);
+
/*
* Increase the pm_only counter before checking whether any
* non-PM blk_queue_enter() calls are in progress to avoid that any
@@ -89,15 +93,14 @@ int blk_pre_runtime_suspend(struct request_queue *q)
/* Switch q_usage_counter back to per-cpu mode. */
blk_mq_unfreeze_queue(q);
- spin_lock_irq(&q->queue_lock);
- if (ret < 0)
+ if (ret < 0) {
+ spin_lock_irq(&q->queue_lock);
+ q->rpm_status = RPM_ACTIVE;
pm_runtime_mark_last_busy(q->dev);
- else
- q->rpm_status = RPM_SUSPENDING;
- spin_unlock_irq(&q->queue_lock);
+ spin_unlock_irq(&q->queue_lock);
- if (ret)
blk_clear_pm_only(q);
+ }
return ret;
}
Guenter reports a build failure on cell_defconfig and maple_defconfg:
In file included from arch/powerpc/include/asm/kup.h:10:0,
from arch/powerpc/include/asm/uaccess.h:12,
from arch/powerpc/lib/checksum_wrappers.c:24:
arch/powerpc/include/asm/book3s/64/kup-radix.h:5:1: error: data definition has no type or storage class [-Werror]
DECLARE_STATIC_KEY_FALSE(uaccess_flush_key);
^~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/book3s/64/kup-radix.h:5:1: error: type defaults to ‘int’ in declaration of ‘DECLARE_STATIC_KEY_FALSE’ [-Werror=implicit-int]
arch/powerpc/include/asm/book3s/64/kup-radix.h:5:1: error: parameter names (without types) in function declaration [-Werror]
arch/powerpc/include/asm/book3s/64/kup-radix.h: In function ‘prevent_user_access’:
arch/powerpc/include/asm/book3s/64/kup-radix.h:18:6: error: implicit declaration of function ‘static_branch_unlikely’ [-Werror=implicit-function-declaration]
if (static_branch_unlikely(&uaccess_flush_key))
^~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/book3s/64/kup-radix.h:18:30: error: ‘uaccess_flush_key’ undeclared (first use in this function); did you mean
‘do_uaccess_flush’?
if (static_branch_unlikely(&uaccess_flush_key))
^~~~~~~~~~~~~~~~~
do_uaccess_flush
arch/powerpc/include/asm/book3s/64/kup-radix.h:18:30: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
This is because I failed to include linux/jump_label.h in kup-radix.h. Include it.
Reported-by: Guenter Roeck <linux(a)roeck-us.net>
Signed-off-by: Daniel Axtens <dja(a)axtens.net>
---
arch/powerpc/include/asm/book3s/64/kup-radix.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index aa54ac2e5659..cce8e7497d72 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
#ifndef _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
#define _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
+#include <linux/jump_label.h>
DECLARE_STATIC_KEY_FALSE(uaccess_flush_key);
--
2.25.1
Guenter reports a build failure on cell_defconfig and maple_defconfg:
In file included from arch/powerpc/include/asm/kup.h:10:0,
from arch/powerpc/include/asm/uaccess.h:12,
from arch/powerpc/lib/checksum_wrappers.c:24:
arch/powerpc/include/asm/book3s/64/kup-radix.h:5:1: error: data definition has no type or storage class [-Werror]
DECLARE_STATIC_KEY_FALSE(uaccess_flush_key);
^~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/book3s/64/kup-radix.h:5:1: error: type defaults to ‘int’ in declaration of ‘DECLARE_STATIC_KEY_FALSE’ [-Werror=implicit-int]
arch/powerpc/include/asm/book3s/64/kup-radix.h:5:1: error: parameter names (without types) in function declaration [-Werror]
arch/powerpc/include/asm/book3s/64/kup-radix.h: In function ‘prevent_user_access’:
arch/powerpc/include/asm/book3s/64/kup-radix.h:18:6: error: implicit declaration of function ‘static_branch_unlikely’ [-Werror=implicit-function-declaration]
if (static_branch_unlikely(&uaccess_flush_key))
^~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/book3s/64/kup-radix.h:18:30: error: ‘uaccess_flush_key’ undeclared (first use in this function); did you mean
‘do_uaccess_flush’?
if (static_branch_unlikely(&uaccess_flush_key))
^~~~~~~~~~~~~~~~~
do_uaccess_flush
arch/powerpc/include/asm/book3s/64/kup-radix.h:18:30: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
This is because I failed to include linux/jump_label.h in kup-radix.h. Include it.
Reported-by: Guenter Roeck <linux(a)roeck-us.net>
Signed-off-by: Daniel Axtens <dja(a)axtens.net>
---
arch/powerpc/include/asm/book3s/64/kup-radix.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index aa54ac2e5659..cce8e7497d72 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
#ifndef _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
#define _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
+#include <linux/jump_label.h>
DECLARE_STATIC_KEY_FALSE(uaccess_flush_key);
--
2.25.1
On Sat, Nov 21, 2020 at 10:29:03AM -0800, Guenter Roeck wrote:
> On Fri, Nov 20, 2020 at 12:02:58PM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.4.245 release.
> > There are 15 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Sun, 22 Nov 2020 10:45:32 +0000.
> > Anything received after that time might be too late.
> >
> Build results:
> total: 165 pass: 164 fail: 1
> Failed builds:
> powerpc:ppc64e_defconfig
> Qemu test results:
> total: 328 pass: 323 fail: 5
> Failed tests:
> ppc64:ppce500:corenet64_smp_defconfig:e5500:initrd
> ppc64:ppce500:corenet64_smp_defconfig:e5500:nvme:rootfs
> ppc64:ppce500:corenet64_smp_defconfig:e5500:sdhci:mmc:rootfs
> ppc64:ppce500:corenet64_smp_defconfig:e5500:scsi[53C895A]:rootfs
> ppc64:ppce500:corenet64_smp_defconfig:e5500:sata-sii3112:rootfs
>
> Failure in all cases is:
>
> In file included from arch/powerpc/kernel/ppc_ksyms.c:10:0:
> arch/powerpc/include/asm/book3s/64/kup-radix.h:11:29: error: redefinition of ‘allow_user_access’
> static __always_inline void allow_user_access(void __user *to, const void __user *from,
> ^~~~~~~~~~~~~~~~~
> In file included from arch/powerpc/include/asm/uaccess.h:12:0,
> from arch/powerpc/kernel/ppc_ksyms.c:8:
> arch/powerpc/include/asm/kup.h:12:20: note: previous definition of ‘allow_user_access’ was here
> static inline void allow_user_access(void __user *to, const void __user *from,
> ^~~~~~~~~~~~~~~~~
> In file included from arch/powerpc/kernel/ppc_ksyms.c:10:0:
> arch/powerpc/include/asm/book3s/64/kup-radix.h:16:20: error: redefinition of ‘prevent_user_access’
> static inline void prevent_user_access(void __user *to, const void __user *from,
> ^~~~~~~~~~~~~~~~~~~
> In file included from arch/powerpc/include/asm/uaccess.h:12:0,
> from arch/powerpc/kernel/ppc_ksyms.c:8:
> arch/powerpc/include/asm/kup.h:14:20: note: previous definition of ‘prevent_user_access’ was here
> static inline void prevent_user_access(void __user *to, const void __user *from,
> ^~~~~~~~~~~~~~~~~~~
>
> Tested-by: Guenter Roeck <linux(a)roeck-us.net>
Thanks for testing these.
Daniel, looks like your patches broke some configurations on powerpc as
shown above. Care to send a fix-up patch for these?
thanks,
greg k-h