Messages in this thread | | | Subject | Re: [PATCH v2 06/28] locking/rwlocks: Add contention detection for rwlocks | From | Waiman Long <> | Date | Tue, 9 Feb 2021 19:41:41 -0500 |
| |
On 2/9/21 7:27 PM, Waiman Long wrote: > On 2/9/21 5:25 PM, Guenter Roeck wrote: >> On Tue, Feb 09, 2021 at 04:46:02PM -0500, Waiman Long wrote: >>> On 2/9/21 3:39 PM, Guenter Roeck wrote: >>>> On Tue, Feb 02, 2021 at 10:57:12AM -0800, Ben Gardon wrote: >>>>> rwlocks do not currently have any facility to detect contention >>>>> like spinlocks do. In order to allow users of rwlocks to better >>>>> manage >>>>> latency, add contention detection for queued rwlocks. >>>>> >>>>> CC: Ingo Molnar <mingo@redhat.com> >>>>> CC: Will Deacon <will@kernel.org> >>>>> Acked-by: Peter Zijlstra <peterz@infradead.org> >>>>> Acked-by: Davidlohr Bueso <dbueso@suse.de> >>>>> Acked-by: Waiman Long <longman@redhat.com> >>>>> Acked-by: Paolo Bonzini <pbonzini@redhat.com> >>>>> Signed-off-by: Ben Gardon <bgardon@google.com> >>>> When building mips:defconfig, this patch results in: >>>> >>>> Error log: >>>> In file included from include/linux/spinlock.h:90, >>>> from include/linux/ipc.h:5, >>>> from include/uapi/linux/sem.h:5, >>>> from include/linux/sem.h:5, >>>> from include/linux/compat.h:14, >>>> from arch/mips/kernel/asm-offsets.c:12: >>>> arch/mips/include/asm/spinlock.h:17:28: error: redefinition of >>>> 'queued_spin_unlock' >>>> 17 | #define queued_spin_unlock queued_spin_unlock >>>> | ^~~~~~~~~~~~~~~~~~ >>>> arch/mips/include/asm/spinlock.h:22:20: note: in expansion of macro >>>> 'queued_spin_unlock' >>>> 22 | static inline void queued_spin_unlock(struct qspinlock >>>> *lock) >>>> | ^~~~~~~~~~~~~~~~~~ >>>> In file included from include/asm-generic/qrwlock.h:17, >>>> from ./arch/mips/include/generated/asm/qrwlock.h:1, >>>> from arch/mips/include/asm/spinlock.h:13, >>>> from include/linux/spinlock.h:90, >>>> from include/linux/ipc.h:5, >>>> from include/uapi/linux/sem.h:5, >>>> from include/linux/sem.h:5, >>>> from include/linux/compat.h:14, >>>> from arch/mips/kernel/asm-offsets.c:12: >>>> include/asm-generic/qspinlock.h:94:29: note: previous definition of >>>> 'queued_spin_unlock' was here >>>> 94 | static __always_inline void queued_spin_unlock(struct >>>> qspinlock *lock) >>>> | ^~~~~~~~~~~~~~~~~~ >>> I think the compile error is caused by the improper header file >>> inclusion >>> ordering. Can you try the following change to see if it can fix the >>> compile >>> error? >>> >> That results in: >> >> In file included from ./arch/mips/include/generated/asm/qrwlock.h:1, >> from ./arch/mips/include/asm/spinlock.h:13, >> from ./include/linux/spinlock.h:90, >> from ./include/linux/ipc.h:5, >> from ./include/uapi/linux/sem.h:5, >> from ./include/linux/sem.h:5, >> from ./include/linux/compat.h:14, >> from arch/mips/kernel/asm-offsets.c:12: >> ./include/asm-generic/qrwlock.h: In function >> 'queued_rwlock_is_contended': >> ./include/asm-generic/qrwlock.h:127:9: error: implicit declaration of >> function 'arch_spin_is_locked' >> >> Guenter > > It is because in arch/mips/include/asm/spinlock.h, asm/qrwlock.h is > included before asm/qspinlock.h. The compilation error should be gone > if the asm/qrwlock.h is removed or moved after asm/qspinlock.h.
After thinking a bit more, I think we should remove asm/qrwlock.h in arch/mips/include/asm/spinlock.h. qrwlock and qspinlocks are independent. An architecture can include one but not the other. Also there is no point in including qrwlock.h in a asm/spinlock.h.
Regards, Longman
| |