locking/arch: Move qrwlock.h include after qspinlock.h
Subsystems (4)
Waiman Long
reviewerPATCH 1/1 locking/arch: Move qrwlock.h include after qspinlock.h
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including qrwlock.h before qspinlock.h can be problematic when expanding qrwlock functionality.
If both qspinlock.h and qrwlock.h are to be included, the qrwlock.h include should always be after qspinlock.h. Update the current set of asm/spinlock.h files to enforce that.
--- arch/arm64/include/asm/spinlock.h | 2 +- arch/mips/include/asm/spinlock.h | 2 +- arch/xtensa/include/asm/spinlock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/include/asm/spinlock.h +1 -1
@@ -5,8 +5,8 @@#ifndef __ASM_SPINLOCK_H#define __ASM_SPINLOCK_H -#include <asm/qrwlock.h> #include <asm/qspinlock.h> +#include <asm/qrwlock.h> /* See include/linux/spinlock.h */ #define smp_mb__after_spinlock () smp_mb()
arch/mips/include/asm/spinlock.h +1 -1
@@ -10,7 +10,6 @@#define _ASM_SPINLOCK_H#include <asm/processor.h> -#include <asm/qrwlock.h> #include <asm-generic/qspinlock_types.h>
@@ -27,5 +26,6 @@ static inline void queued_spin_unlock(struct qspinlock *lock) }#include <asm/qspinlock.h> +#include <asm/qrwlock.h> #endif /* _ASM_SPINLOCK_H */
arch/xtensa/include/asm/spinlock.h +1 -1
@@ -12,8 +12,8 @@#define _XTENSA_SPINLOCK_H#include <asm/barrier.h> -#include <asm/qrwlock.h> #include <asm/qspinlock.h> +#include <asm/qrwlock.h> #define smp_mb__after_spinlock () smp_mb()
Signature · 7 lines
2.18.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 10 lines · Waiman Long
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including
qrwlock.hbeforeqspinlock.hcan be problematic when expanding qrwlock functionality.If both
qspinlock.handqrwlock.hare to be included, theqrwlock.hinclude should always be afterqspinlock.h. Update the current set ofasm/spinlock.hfiles to enforce that.Signed-off-by Waiman Long <[email protected]>
There should be a Fixes: tag here. If the SHA of the offending commit is not stable, there should be a better reference than "The queued rwlock code".
This patch fixes the build problem I had observed on mips. I also tested xtensa:defconfig and arm64:defconfig with no problems observed.
Guenter
Quoted 54 lines
--- arch/arm64/include/asm/spinlock.h | 2 +- arch/mips/include/asm/spinlock.h | 2 +- arch/xtensa/include/asm/spinlock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)arch/arm64/include/asm/spinlock.h +1
-1@@ -5,8 +5,8 @@#ifndef __ASM_SPINLOCK_H#define __ASM_SPINLOCK_H -#include <asm/qrwlock.h> #include <asm/qspinlock.h> +#include <asm/qrwlock.h> /* See include/linux/spinlock.h */ #define smp_mb__after_spinlock () smp_mb()arch/mips/include/asm/spinlock.h +1
-1@@ -10,7 +10,6 @@#define _ASM_SPINLOCK_H#include <asm/processor.h> -#include <asm/qrwlock.h> #include <asm-generic/qspinlock_types.h> @@ -27,5 +26,6 @@ static inline void queued_spin_unlock(struct qspinlock *lock) }#include <asm/qspinlock.h> +#include <asm/qrwlock.h> #endif /* _ASM_SPINLOCK_H */ arch/xtensa/include/asm/spinlock.h +1
-1@@ -12,8 +12,8 @@#define _XTENSA_SPINLOCK_H#include <asm/barrier.h> -#include <asm/qrwlock.h> #include <asm/qspinlock.h> +#include <asm/qrwlock.h> #define smp_mb__after_spinlock () smp_mb()
Waiman Long
reviewerQuoted 6 lines · Guenter Roeck
Quoted 5 lines · Waiman Long
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including
qrwlock.hbeforeqspinlock.hcan be problematic when expanding qrwlock functionality.
Quoted 7 lines
Quoted 5 lines
If both
qspinlock.handqrwlock.hare to be included, theqrwlock.hinclude should always be afterqspinlock.h. Update the current set ofasm/spinlock.hfiles to enforce that.Signed-off-by Waiman Long <[email protected]>There should be a Fixes: tag here. If the SHA of the offending commit is not stable, there should be a better reference than "The queued rwlock code".
I originally have a Fixes tag when I was modifying the mips' asm/spinlock.h file. After I realize that there are more files to modify, I take that out. Anyway, the problem was exposed by Ben's qrwlock patch. So existing stable releases should still be fine without this patch.
Quoted 5 lines
This patch fixes the build problem I had observed on mips. I also tested xtensa:defconfig and arm64:defconfig with no problems observed.
Tested-by Guenter Roeck <[email protected]>
Thanks for the testing as I don't have a build environment to verify that.
Cheers,
Longman
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ben Gardon
Reviewed-byQuoted 24 lines · Waiman Long
Quoted 6 lines · Guenter Roeck
Quoted 5 lines · Waiman Long
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including
qrwlock.hbeforeqspinlock.hcan be problematic when expanding qrwlock functionality.Quoted 7 lines
Quoted 5 lines
If both
qspinlock.handqrwlock.hare to be included, theqrwlock.hinclude should always be afterqspinlock.h. Update the current set ofasm/spinlock.hfiles to enforce that.Signed-off-by Waiman Long <[email protected]>There should be a Fixes: tag here. If the SHA of the offending commit is not stable, there should be a better reference than "The queued rwlock code".
I originally have a Fixes tag when I was modifying the mips'
asm/spinlock.hfile. After I realize that there are more files to modify, I take that out. Anyway, the problem was exposed by Ben's qrwlock patch. So existing stable releases should still be fine without this patch.Quoted 4 lines
This patch fixes the build problem I had observed on mips. I also tested xtensa:defconfig and arm64:defconfig with no problems observed.
Quoted 7 lines
Quoted 1 line
Tested-by Guenter Roeck <[email protected]>Thanks for the testing as I don't have a build environment to verify that.
Cheers,
Longman
Thanks Longman and Guenter for developing and testing this fix! I don't have the environment to test this either, but the patch looks good to me.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Thomas Bogendoerfer
maintainerQuoted 9 lines · Waiman Long
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including
qrwlock.hbeforeqspinlock.hcan be problematic when expanding qrwlock functionality.If both
qspinlock.handqrwlock.hare to be included, theqrwlock.hinclude should always be afterqspinlock.h. Update the current set ofasm/spinlock.hfiles to enforce that.
Quoted 6 lines
Signed-off-by Waiman Long <[email protected]>--- arch/arm64/include/asm/spinlock.h | 2 +- arch/mips/include/asm/spinlock.h | 2 +- arch/xtensa/include/asm/spinlock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
which tree should this go through ? I can take it via mips-next, if everybody agrees.
Thomas.
Signature · 2 lines
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea. [ RFC1925, 2.3 ]
Quoted 6 lines · Thomas Bogendoerfer
Quoted 4 lines
arch/arm64/include/asm/spinlock.h | 2 +- arch/mips/include/asm/spinlock.h | 2 +- arch/xtensa/include/asm/spinlock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)which tree should this go through ? I can take it via mips-next, if everybody agrees.
The breakage is in the KVM tree, and the existing patch has acked-by from the locking primitives folks. So I'll queue it there in order to limit the range that breaks bisection.
Paolo
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 11 lines · Paolo Bonzini
Quoted 6 lines · Thomas Bogendoerfer
Quoted 4 lines
arch/arm64/include/asm/spinlock.h | 2 +- arch/mips/include/asm/spinlock.h | 2 +- arch/xtensa/include/asm/spinlock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)which tree should this go through ? I can take it via mips-next, if everybody agrees.
The breakage is in the KVM tree, and the existing patch has acked-by from the locking primitives folks. So I'll queue it there in order to limit the range that breaks bisection.
if it's not too late you can add by
Signature · 7 lines
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea. [ RFC1925, 2.3 ] _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 10 lines · Waiman Long
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including
qrwlock.hbeforeqspinlock.hcan be problematic when expanding qrwlock functionality.If both
qspinlock.handqrwlock.hare to be included, theqrwlock.hinclude should always be afterqspinlock.h. Update the current set ofasm/spinlock.hfiles to enforce that.Signed-off-by Waiman Long <[email protected]>
arch/sparc/include/asm/spinlock_64.h is missing. Also, the include in kernel/locking/qrwlock.c is not necessary (it may be there for aesthetic reasons, but it complicates thing in this case).
I'll send a v2 that is based on the kvm/next tree.
Paolo
Quoted 45 lines
--- arch/arm64/include/asm/spinlock.h | 2 +- arch/mips/include/asm/spinlock.h | 2 +- arch/xtensa/include/asm/spinlock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)arch/arm64/include/asm/spinlock.h +1
-1@@ -5,8 +5,8 @@#ifndef __ASM_SPINLOCK_H#define __ASM_SPINLOCK_H -#include <asm/qrwlock.h> #include <asm/qspinlock.h> +#include <asm/qrwlock.h> /* See include/linux/spinlock.h */ #define smp_mb__after_spinlock () smp_mb()arch/mips/include/asm/spinlock.h +1
-1@@ -10,7 +10,6 @@#define _ASM_SPINLOCK_H#include <asm/processor.h> -#include <asm/qrwlock.h> #include <asm-generic/qspinlock_types.h> @@ -27,5 +26,6 @@ static inline void queued_spin_unlock(struct qspinlock *lock) }#include <asm/qspinlock.h> +#include <asm/qrwlock.h> #endif /* _ASM_SPINLOCK_H */ arch/xtensa/include/asm/spinlock.h +0
-0
arch/xtensa/include/asm/spinlock.h line 19
Quoted 9 lines
arch/xtensa/include/asm/spinlock.h +1
-1@@ -12,8 +12,8 @@#define _XTENSA_SPINLOCK_H#include <asm/barrier.h> -#include <asm/qrwlock.h> #include <asm/qspinlock.h> +#include <asm/qrwlock.h> #define smp_mb__after_spinlock () smp_mb()
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Waiman Long
reviewerQuoted 6 lines · Paolo Bonzini
Quoted 5 lines · Waiman Long
The queued rwlock code has a dependency on the current spinlock implementation (likely to be qspinlock), but not vice versa. Including
qrwlock.hbeforeqspinlock.hcan be problematic when expanding qrwlock functionality.
Quoted 7 lines
Quoted 5 lines
If both
qspinlock.handqrwlock.hare to be included, theqrwlock.hinclude should always be afterqspinlock.h. Update the current set ofasm/spinlock.hfiles to enforce that.Signed-off-by Waiman Long <[email protected]>
arch/sparc/include/asm/spinlock_64.his missing. Also, the include inkernel/locking/qrwlock.cis not necessary (it may be there for aesthetic reasons, but it complicates thing in this case).
Sorry for missing arch/sparc/include/asm/spinlock_64.h. I was just focusing on asm/spinlock.h and not aware that there are other variants there.
It is true that the asm/qrwlock.h include in qrwlock.c is not really necessary. I can't recall why it was there.
Quoted 5 lines
I'll send a v2 that is based on the kvm/next tree.
Paolo
Thanks for taking care of that.
Cheers,
Longman
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel