| Message ID | 20230731023308.3748432-1-guoren@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] asm-generic: ticket-lock: Optimize arch_spin_value_unlocked | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 2808 this patch: 2808 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 15873 this patch: 15873 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Sun, Jul 30, 2023 at 10:33:08PM -0400, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > The arch_spin_value_unlocked would cause an unnecessary memory > access to the contended value. Although it won't cause a significant > performance gap in most architectures, the arch_spin_value_unlocked > argument contains enough information. Thus, remove unnecessary > atomic_read in arch_spin_value_unlocked(). > > The caller of arch_spin_value_unlocked() could benefit from this > change. Currently, the only caller is lockref. > > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Waiman Long <longman@redhat.com> > Cc: David Laight <David.Laight@ACULAB.COM> > Cc: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > Changelog > V2: > - Fixup commit log with Waiman advice. > - Add Waiman comment in the commit msg. > --- > include/asm-generic/spinlock.h | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) Acked-by: Will Deacon <will@kernel.org> It would be nice to see some numbers showing this actually helps, though. Will
在 2023/7/31 10:33, guoren@kernel.org 写道: > From: Guo Ren <guoren@linux.alibaba.com> > > The arch_spin_value_unlocked would cause an unnecessary memory > access to the contended value. Although it won't cause a significant > performance gap in most architectures, the arch_spin_value_unlocked > argument contains enough information. Thus, remove unnecessary > atomic_read in arch_spin_value_unlocked(). > > The caller of arch_spin_value_unlocked() could benefit from this > change. Currently, the only caller is lockref. > > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Waiman Long <longman@redhat.com> > Cc: David Laight <David.Laight@ACULAB.COM> > Cc: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > Changelog > V2: > - Fixup commit log with Waiman advice. > - Add Waiman comment in the commit msg. > --- > include/asm-generic/spinlock.h | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h > index fdfebcb050f4..90803a826ba0 100644 > --- a/include/asm-generic/spinlock.h > +++ b/include/asm-generic/spinlock.h > @@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) > smp_store_release(ptr, (u16)val + 1); > } > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) > +{ > + u32 val = lock.counter; > + > + return ((val >> 16) == (val & 0xffff)); > +} I do not know much about lock, will it be cached in register without memory access again like READ_ONCE or atomic_read? Regards Bibo Mao > + > static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) > { > - u32 val = atomic_read(lock); > + arch_spinlock_t val = READ_ONCE(*lock); > > - return ((val >> 16) != (val & 0xffff)); > + return !arch_spin_value_unlocked(val); > } > > static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) > @@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) > return (s16)((val >> 16) - (val & 0xffff)) > 1; > } > > -static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) > -{ > - return !arch_spin_is_locked(&lock); > -} > - > #include <asm/qrwlock.h> > > #endif /* __ASM_GENERIC_SPINLOCK_H */
On 7/30/23 22:33, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > The arch_spin_value_unlocked would cause an unnecessary memory > access to the contended value. Although it won't cause a significant > performance gap in most architectures, the arch_spin_value_unlocked > argument contains enough information. Thus, remove unnecessary > atomic_read in arch_spin_value_unlocked(). > > The caller of arch_spin_value_unlocked() could benefit from this > change. Currently, the only caller is lockref. > > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Waiman Long <longman@redhat.com> > Cc: David Laight <David.Laight@ACULAB.COM> > Cc: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > Changelog > V2: > - Fixup commit log with Waiman advice. > - Add Waiman comment in the commit msg. > --- > include/asm-generic/spinlock.h | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h > index fdfebcb050f4..90803a826ba0 100644 > --- a/include/asm-generic/spinlock.h > +++ b/include/asm-generic/spinlock.h > @@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) > smp_store_release(ptr, (u16)val + 1); > } > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) > +{ > + u32 val = lock.counter; > + > + return ((val >> 16) == (val & 0xffff)); > +} > + > static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) > { > - u32 val = atomic_read(lock); > + arch_spinlock_t val = READ_ONCE(*lock); > > - return ((val >> 16) != (val & 0xffff)); > + return !arch_spin_value_unlocked(val); > } > > static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) > @@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) > return (s16)((val >> 16) - (val & 0xffff)) > 1; > } > > -static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) > -{ > - return !arch_spin_is_locked(&lock); > -} > - > #include <asm/qrwlock.h> > > #endif /* __ASM_GENERIC_SPINLOCK_H */ I am fine with the current change. However, modern optimizing compiler should be able to avoid the redundant memory read anyway. So this patch may not have an impact from the performance point of view. Acked-by: Waiman Long <longman@redhat.com>
在 2023/7/31 23:16, Waiman Long 写道: > On 7/30/23 22:33, guoren@kernel.org wrote: >> From: Guo Ren <guoren@linux.alibaba.com> >> >> The arch_spin_value_unlocked would cause an unnecessary memory >> access to the contended value. Although it won't cause a significant >> performance gap in most architectures, the arch_spin_value_unlocked >> argument contains enough information. Thus, remove unnecessary >> atomic_read in arch_spin_value_unlocked(). >> >> The caller of arch_spin_value_unlocked() could benefit from this >> change. Currently, the only caller is lockref. >> >> Signed-off-by: Guo Ren <guoren@kernel.org> >> Cc: Waiman Long <longman@redhat.com> >> Cc: David Laight <David.Laight@ACULAB.COM> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Signed-off-by: Guo Ren <guoren@linux.alibaba.com> >> --- >> Changelog >> V2: >> - Fixup commit log with Waiman advice. >> - Add Waiman comment in the commit msg. >> --- >> include/asm-generic/spinlock.h | 16 +++++++++------- >> 1 file changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h >> index fdfebcb050f4..90803a826ba0 100644 >> --- a/include/asm-generic/spinlock.h >> +++ b/include/asm-generic/spinlock.h >> @@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) >> smp_store_release(ptr, (u16)val + 1); >> } >> +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >> +{ >> + u32 val = lock.counter; >> + >> + return ((val >> 16) == (val & 0xffff)); >> +} >> + >> static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) >> { >> - u32 val = atomic_read(lock); >> + arch_spinlock_t val = READ_ONCE(*lock); >> - return ((val >> 16) != (val & 0xffff)); >> + return !arch_spin_value_unlocked(val); >> } >> static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) >> @@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) >> return (s16)((val >> 16) - (val & 0xffff)) > 1; >> } >> -static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >> -{ >> - return !arch_spin_is_locked(&lock); >> -} >> - >> #include <asm/qrwlock.h> >> #endif /* __ASM_GENERIC_SPINLOCK_H */ > > I am fine with the current change. However, modern optimizing compiler should be able to avoid the redundant memory read anyway. So this patch may not have an impact from the performance point of view. arch_spin_value_unlocked is called with lockref like this: #define CMPXCHG_LOOP(CODE, SUCCESS) do { \ int retry = 100; \ struct lockref old; \ BUILD_BUG_ON(sizeof(old) != 8); \ old.lock_count = READ_ONCE(lockref->lock_count); \ while (likely(arch_spin_value_unlocked(old.lock.rlock.raw_lock))) { \ With modern optimizing compiler, Is it possible that old value of old.lock.rlock.raw_lock is cached in register, despite that try_cmpxchg64_relaxed modifies the memory of old.lock_count with new value? Regards Bibo Mao struct lockref new = old; \ CODE \ if (likely(try_cmpxchg64_relaxed(&lockref->lock_count, \ &old.lock_count, \ new.lock_count))) { \ SUCCESS; \ } \ if (!--retry) \ break; \ } \ } while (0) > > Acked-by: Waiman Long <longman@redhat.com>
On 7/31/23 21:37, bibo mao wrote: > > 在 2023/7/31 23:16, Waiman Long 写道: >> On 7/30/23 22:33, guoren@kernel.org wrote: >>> From: Guo Ren <guoren@linux.alibaba.com> >>> >>> The arch_spin_value_unlocked would cause an unnecessary memory >>> access to the contended value. Although it won't cause a significant >>> performance gap in most architectures, the arch_spin_value_unlocked >>> argument contains enough information. Thus, remove unnecessary >>> atomic_read in arch_spin_value_unlocked(). >>> >>> The caller of arch_spin_value_unlocked() could benefit from this >>> change. Currently, the only caller is lockref. >>> >>> Signed-off-by: Guo Ren <guoren@kernel.org> >>> Cc: Waiman Long <longman@redhat.com> >>> Cc: David Laight <David.Laight@ACULAB.COM> >>> Cc: Peter Zijlstra <peterz@infradead.org> >>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com> >>> --- >>> Changelog >>> V2: >>> - Fixup commit log with Waiman advice. >>> - Add Waiman comment in the commit msg. >>> --- >>> include/asm-generic/spinlock.h | 16 +++++++++------- >>> 1 file changed, 9 insertions(+), 7 deletions(-) >>> >>> diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h >>> index fdfebcb050f4..90803a826ba0 100644 >>> --- a/include/asm-generic/spinlock.h >>> +++ b/include/asm-generic/spinlock.h >>> @@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) >>> smp_store_release(ptr, (u16)val + 1); >>> } >>> +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >>> +{ >>> + u32 val = lock.counter; >>> + >>> + return ((val >> 16) == (val & 0xffff)); >>> +} >>> + >>> static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) >>> { >>> - u32 val = atomic_read(lock); >>> + arch_spinlock_t val = READ_ONCE(*lock); >>> - return ((val >> 16) != (val & 0xffff)); >>> + return !arch_spin_value_unlocked(val); >>> } >>> static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) >>> @@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) >>> return (s16)((val >> 16) - (val & 0xffff)) > 1; >>> } >>> -static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >>> -{ >>> - return !arch_spin_is_locked(&lock); >>> -} >>> - >>> #include <asm/qrwlock.h> >>> #endif /* __ASM_GENERIC_SPINLOCK_H */ >> I am fine with the current change. However, modern optimizing compiler should be able to avoid the redundant memory read anyway. So this patch may not have an impact from the performance point of view. > arch_spin_value_unlocked is called with lockref like this: > > #define CMPXCHG_LOOP(CODE, SUCCESS) do { \ > int retry = 100; \ > struct lockref old; \ > BUILD_BUG_ON(sizeof(old) != 8); \ > old.lock_count = READ_ONCE(lockref->lock_count); \ > while (likely(arch_spin_value_unlocked(old.lock.rlock.raw_lock))) { \ > > With modern optimizing compiler, Is it possible that old value of > old.lock.rlock.raw_lock is cached in register, despite that try_cmpxchg64_relaxed > modifies the memory of old.lock_count with new value? What I meant is that the call to arch_spin_value_unlocked() as it is today will not generate 2 memory reads of the same location with or without the patch. Of course, a new memory read will be needed after a failed cmpxchg(). Cheers, Longman
在 2023/8/1 09:59, Waiman Long 写道: > On 7/31/23 21:37, bibo mao wrote: >> >> 在 2023/7/31 23:16, Waiman Long 写道: >>> On 7/30/23 22:33, guoren@kernel.org wrote: >>>> From: Guo Ren <guoren@linux.alibaba.com> >>>> >>>> The arch_spin_value_unlocked would cause an unnecessary memory >>>> access to the contended value. Although it won't cause a significant >>>> performance gap in most architectures, the arch_spin_value_unlocked >>>> argument contains enough information. Thus, remove unnecessary >>>> atomic_read in arch_spin_value_unlocked(). >>>> >>>> The caller of arch_spin_value_unlocked() could benefit from this >>>> change. Currently, the only caller is lockref. >>>> >>>> Signed-off-by: Guo Ren <guoren@kernel.org> >>>> Cc: Waiman Long <longman@redhat.com> >>>> Cc: David Laight <David.Laight@ACULAB.COM> >>>> Cc: Peter Zijlstra <peterz@infradead.org> >>>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com> >>>> --- >>>> Changelog >>>> V2: >>>> - Fixup commit log with Waiman advice. >>>> - Add Waiman comment in the commit msg. >>>> --- >>>> include/asm-generic/spinlock.h | 16 +++++++++------- >>>> 1 file changed, 9 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h >>>> index fdfebcb050f4..90803a826ba0 100644 >>>> --- a/include/asm-generic/spinlock.h >>>> +++ b/include/asm-generic/spinlock.h >>>> @@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) >>>> smp_store_release(ptr, (u16)val + 1); >>>> } >>>> +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >>>> +{ >>>> + u32 val = lock.counter; >>>> + >>>> + return ((val >> 16) == (val & 0xffff)); >>>> +} >>>> + >>>> static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) >>>> { >>>> - u32 val = atomic_read(lock); >>>> + arch_spinlock_t val = READ_ONCE(*lock); >>>> - return ((val >> 16) != (val & 0xffff)); >>>> + return !arch_spin_value_unlocked(val); >>>> } >>>> static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) >>>> @@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) >>>> return (s16)((val >> 16) - (val & 0xffff)) > 1; >>>> } >>>> -static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >>>> -{ >>>> - return !arch_spin_is_locked(&lock); >>>> -} >>>> - >>>> #include <asm/qrwlock.h> >>>> #endif /* __ASM_GENERIC_SPINLOCK_H */ >>> I am fine with the current change. However, modern optimizing compiler should be able to avoid the redundant memory read anyway. So this patch may not have an impact from the performance point of view. >> arch_spin_value_unlocked is called with lockref like this: >> >> #define CMPXCHG_LOOP(CODE, SUCCESS) do { \ >> int retry = 100; \ >> struct lockref old; \ >> BUILD_BUG_ON(sizeof(old) != 8); \ >> old.lock_count = READ_ONCE(lockref->lock_count); \ >> while (likely(arch_spin_value_unlocked(old.lock.rlock.raw_lock))) { \ >> >> With modern optimizing compiler, Is it possible that old value of >> old.lock.rlock.raw_lock is cached in register, despite that try_cmpxchg64_relaxed >> modifies the memory of old.lock_count with new value? > > What I meant is that the call to arch_spin_value_unlocked() as it is today will not generate 2 memory reads of the same location with or without the patch. Of course, a new memory read will be needed after a failed cmpxchg(). yeap, it can solve the issue with a new memory read after a failed cmpxchg(). Regards Bibo Mao > > Cheers, > Longman
On Sun, Jul 30, 2023 at 10:33:08PM -0400, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > The arch_spin_value_unlocked would cause an unnecessary memory > access to the contended value. Although it won't cause a significant > performance gap in most architectures, the arch_spin_value_unlocked > argument contains enough information. Thus, remove unnecessary > atomic_read in arch_spin_value_unlocked(). > > The caller of arch_spin_value_unlocked() could benefit from this > change. Currently, the only caller is lockref. > Have you verified you are getting an extra memory access from this in lockref? What architecture is it? I have no opinion about the patch itself, I will note though that the argument to the routine is *not* the actual memory-shared lockref, instead it's something from the local copy obtained with READ_ONCE from the real thing. So I would be surprised if the stock routine was generating accesses to that sucker. Nonetheless, if the patched routine adds nasty asm, that would be nice to sort out. FWIW on x86-64 qspinlock is used (i.e. not the stuff you are patching) and I verified there are only 2 memory accesses -- the initial READ_ONCE and later cmpxchg. I don't know which archs *don't* use qspinlock. It also turns out generated asm is quite atrocious and cleaning it up may yield a small win under more traffic. Maybe I'll see about it later this week. For example, disassembling lockref_put_return: <+0>: mov (%rdi),%rax <-- initial load, expected <+3>: mov $0x64,%r8d <+9>: mov %rax,%rdx <+12>: test %eax,%eax <-- retries loop back here <-- this is also the unlocked check <+14>: jne 0xffffffff8157aba3 <lockref_put_return+67> <+16>: mov %rdx,%rsi <+19>: mov %edx,%edx <+21>: sar $0x20,%rsi <+25>: lea -0x1(%rsi),%ecx <-- new.count--; <+28>: shl $0x20,%rcx <+32>: or %rcx,%rdx <+35>: test %esi,%esi <+37>: jle 0xffffffff8157aba3 <lockref_put_return+67> <+39>: lock cmpxchg %rdx,(%rdi) <-- the attempt to change <+44>: jne 0xffffffff8157ab9a <lockref_put_return+58> <+46>: shr $0x20,%rdx <+50>: mov %rdx,%rax <+53>: jmp 0xffffffff81af8540 <__x86_return_thunk> <+58>: mov %rax,%rdx <+61>: sub $0x1,%r8d <-- retry count check <+65>: jne 0xffffffff8157ab6c <lockref_put_return+12> <-- go back <+67>: mov $0xffffffff,%eax <+72>: jmp 0xffffffff81af8540 <__x86_return_thunk>
On Mon, Aug 07, 2023 at 08:36:55PM +0200, Mateusz Guzik wrote: > On Sun, Jul 30, 2023 at 10:33:08PM -0400, guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > The arch_spin_value_unlocked would cause an unnecessary memory > > access to the contended value. Although it won't cause a significant > > performance gap in most architectures, the arch_spin_value_unlocked > > argument contains enough information. Thus, remove unnecessary > > atomic_read in arch_spin_value_unlocked(). > > > > The caller of arch_spin_value_unlocked() could benefit from this > > change. Currently, the only caller is lockref. > > > > Have you verified you are getting an extra memory access from this in > lockref? What architecture is it? For riscv, this patch could optimize the lock_ref on the same compiling condition: - After lifting data dependencies, the compiler optimizes the prologue behavior, thus the callee-register-saved path becomes optional. This is a significant improvement on the lock_ref() self. - Compare the "98: & 9c:" lines before the patch and the "88:" line after the patch. We saved two memory accesses not only one load. ======================================================================== Before the patch: void lockref_get(struct lockref *lockref) { 78: fd010113 add sp,sp,-48 7c: 02813023 sd s0,32(sp) 80: 02113423 sd ra,40(sp) 84: 03010413 add s0,sp,48 0000000000000088 <.LBB296>: CMPXCHG_LOOP( 88: 00053783 ld a5,0(a0) 000000000000008c <.LBB265>: } static __always_inline int ticket_spin_is_locked(arch_spinlock_t *lock) { u32 val = atomic_read(&lock->val); return ((val >> 16) != (val & 0xffff)); 8c: 00010637 lui a2,0x10 0000000000000090 <.LBE265>: 90: 06400593 li a1,100 0000000000000094 <.LBB274>: 94: fff60613 add a2,a2,-1 # ffff <.LLST8+0xf49a> 0000000000000098 <.L8>: 98: fef42423 sw a5,-24(s0) 000000000000009c <.LBB269>: 9c: fe842703 lw a4,-24(s0) 00000000000000a0 <.LBE269>: a0: 0107569b srlw a3,a4,0x10 a4: 00c77733 and a4,a4,a2 a8: 04e69063 bne a3,a4,e8 <.L12> 00000000000000ac <.LBB282>: ac: 4207d693 sra a3,a5,0x20 b0: 02079713 sll a4,a5,0x20 b4: 0016869b addw a3,a3,1 b8: 02069693 sll a3,a3,0x20 bc: 02075713 srl a4,a4,0x20 c0: 00d76733 or a4,a4,a3 00000000000000c4 <.L0^B1>: c4: 100536af lr.d a3,(a0) c8: 00f69863 bne a3,a5,d8 <.L1^B1> cc: 1ae5382f sc.d.rl a6,a4,(a0) d0: fe081ae3 bnez a6,c4 <.L0^B1> d4: 0330000f fence rw,rw 00000000000000d8 <.L1^B1>: d8: 02d78a63 beq a5,a3,10c <.L7> 00000000000000dc <.LBE292>: dc: fff5859b addw a1,a1,-1 00000000000000e0 <.LBB293>: e0: 00068793 mv a5,a3 00000000000000e4 <.LBE293>: e4: fa059ae3 bnez a1,98 <.L8> 00000000000000e8 <.L12>: ======================================================================== After the patch: void lockref_get(struct lockref *lockref) { CMPXCHG_LOOP( 78: 00053783 ld a5,0(a0) 000000000000007c <.LBB212>: static __always_inline int ticket_spin_value_unlocked(arch_spinlock_t lock) { u32 val = lock.val.counter; return ((val >> 16) == (val & 0xffff)); 7c: 00010637 lui a2,0x10 0000000000000080 <.LBE212>: 80: 06400593 li a1,100 0000000000000084 <.LBB216>: 84: fff60613 add a2,a2,-1 # ffff <.LLST8+0xf4aa> 0000000000000088 <.L8>: 88: 0007871b sext.w a4,a5 000000000000008c <.LBB217>: 8c: 0107d69b srlw a3,a5,0x10 90: 00c77733 and a4,a4,a2 94: 04e69063 bne a3,a4,d4 <.L12> 0000000000000098 <.LBB218>: 98: 4207d693 sra a3,a5,0x20 9c: 02079713 sll a4,a5,0x20 a0: 0016869b addw a3,a3,1 a4: 02069693 sll a3,a3,0x20 a8: 02075713 srl a4,a4,0x20 ac: 00d76733 or a4,a4,a3 00000000000000b0 <.L0^B1>: b0: 100536af lr.d a3,(a0) b4: 00f69863 bne a3,a5,c4 <.L1^B1> b8: 1ae5382f sc.d.rl a6,a4,(a0) bc: fe081ae3 bnez a6,b0 <.L0^B1> c0: 0330000f fence rw,rw 00000000000000c4 <.L1^B1>: c4: 04d78a63 beq a5,a3,118 <.L18> 00000000000000c8 <.LBE228>: c8: fff5859b addw a1,a1,-1 00000000000000cc <.LBB229>: cc: 00068793 mv a5,a3 00000000000000d0 <.LBE229>: d0: fa059ce3 bnez a1,88 <.L8> 00000000000000d4 <.L12>: { d4: fe010113 add sp,sp,-32 d8: 00113c23 sd ra,24(sp) dc: 00813823 sd s0,16(sp) e0: 02010413 add s0,sp,32 ======================================================================== > > I have no opinion about the patch itself, I will note though that the > argument to the routine is *not* the actual memory-shared lockref, > instead it's something from the local copy obtained with READ_ONCE > from the real thing. So I would be surprised if the stock routine was > generating accesses to that sucker. > > Nonetheless, if the patched routine adds nasty asm, that would be nice > to sort out. > > FWIW on x86-64 qspinlock is used (i.e. not the stuff you are patching) > and I verified there are only 2 memory accesses -- the initial READ_ONCE > and later cmpxchg. I don't know which archs *don't* use qspinlock. > > It also turns out generated asm is quite atrocious and cleaning it up > may yield a small win under more traffic. Maybe I'll see about it later > this week. > > For example, disassembling lockref_put_return: > <+0>: mov (%rdi),%rax <-- initial load, expected > <+3>: mov $0x64,%r8d > <+9>: mov %rax,%rdx > <+12>: test %eax,%eax <-- retries loop back here > <-- this is also the unlocked > check > <+14>: jne 0xffffffff8157aba3 <lockref_put_return+67> > <+16>: mov %rdx,%rsi > <+19>: mov %edx,%edx > <+21>: sar $0x20,%rsi > <+25>: lea -0x1(%rsi),%ecx <-- new.count--; > <+28>: shl $0x20,%rcx > <+32>: or %rcx,%rdx > <+35>: test %esi,%esi > <+37>: jle 0xffffffff8157aba3 <lockref_put_return+67> > <+39>: lock cmpxchg %rdx,(%rdi) <-- the attempt to change > <+44>: jne 0xffffffff8157ab9a <lockref_put_return+58> > <+46>: shr $0x20,%rdx > <+50>: mov %rdx,%rax > <+53>: jmp 0xffffffff81af8540 <__x86_return_thunk> > <+58>: mov %rax,%rdx > <+61>: sub $0x1,%r8d <-- retry count check > <+65>: jne 0xffffffff8157ab6c <lockref_put_return+12> <-- go back > <+67>: mov $0xffffffff,%eax > <+72>: jmp 0xffffffff81af8540 <__x86_return_thunk> >
On 8/8/23, Guo Ren <guoren@kernel.org> wrote: > On Mon, Aug 07, 2023 at 08:36:55PM +0200, Mateusz Guzik wrote: >> On Sun, Jul 30, 2023 at 10:33:08PM -0400, guoren@kernel.org wrote: >> > From: Guo Ren <guoren@linux.alibaba.com> >> > >> > The arch_spin_value_unlocked would cause an unnecessary memory >> > access to the contended value. Although it won't cause a significant >> > performance gap in most architectures, the arch_spin_value_unlocked >> > argument contains enough information. Thus, remove unnecessary >> > atomic_read in arch_spin_value_unlocked(). >> > >> > The caller of arch_spin_value_unlocked() could benefit from this >> > change. Currently, the only caller is lockref. >> > >> >> Have you verified you are getting an extra memory access from this in >> lockref? What architecture is it? > For riscv, this patch could optimize the lock_ref on the same compiling > condition: > - After lifting data dependencies, the compiler optimizes the prologue > behavior, thus the callee-register-saved path becomes optional. This > is a significant improvement on the lock_ref() self. > - Compare the "98: & 9c:" lines before the patch and the "88:" line > after the patch. We saved two memory accesses not only one load. > Now that you mention it, I see riscv in cc. ;) Your commit message states "arch_spin_value_unlocked would cause an unnecessary memory access to the contended value" and that lockref uses it. Perhaps incorrectly I took it to claim lockref is suffering extra loads from the area it modifies with cmpxchg -- as I verified, this is not happening as the argument to arch_spin_value_unlocked is a copy of the target lockref struct. With this not being a problem, potential scalability impact goes down a lot. And so happens with the code from qspinlock on x86-64 there are no extra memory accesses to anything anyway. I don't speak riscv asm so can't comment on the result. I'll note again that extra work for single-threaded use is definitely worth shaving and may or may not affect throughput in face of other CPUs messing with lockref. You can easily test lockref with will-it-scale, I would suggest the access() system call which afaics has least amount of unrelated overhead. You can find the bench here: https://github.com/antonblanchard/will-it-scale/pull/36/files > ======================================================================== > Before the patch: > void lockref_get(struct lockref *lockref) > { > 78: fd010113 add sp,sp,-48 > 7c: 02813023 sd s0,32(sp) > 80: 02113423 sd ra,40(sp) > 84: 03010413 add s0,sp,48 > > 0000000000000088 <.LBB296>: > CMPXCHG_LOOP( > 88: 00053783 ld a5,0(a0) > > 000000000000008c <.LBB265>: > } > > static __always_inline int ticket_spin_is_locked(arch_spinlock_t *lock) > { > u32 val = atomic_read(&lock->val); > return ((val >> 16) != (val & 0xffff)); > 8c: 00010637 lui a2,0x10 > > 0000000000000090 <.LBE265>: > 90: 06400593 li a1,100 > > 0000000000000094 <.LBB274>: > 94: fff60613 add a2,a2,-1 # ffff <.LLST8+0xf49a> > > 0000000000000098 <.L8>: > 98: fef42423 sw a5,-24(s0) > > 000000000000009c <.LBB269>: > 9c: fe842703 lw a4,-24(s0) > > 00000000000000a0 <.LBE269>: > a0: 0107569b srlw a3,a4,0x10 > a4: 00c77733 and a4,a4,a2 > a8: 04e69063 bne a3,a4,e8 <.L12> > > 00000000000000ac <.LBB282>: > ac: 4207d693 sra a3,a5,0x20 > b0: 02079713 sll a4,a5,0x20 > b4: 0016869b addw a3,a3,1 > b8: 02069693 sll a3,a3,0x20 > bc: 02075713 srl a4,a4,0x20 > c0: 00d76733 or a4,a4,a3 > > 00000000000000c4 <.L0^B1>: > c4: 100536af lr.d a3,(a0) > c8: 00f69863 bne a3,a5,d8 <.L1^B1> > cc: 1ae5382f sc.d.rl a6,a4,(a0) > d0: fe081ae3 bnez a6,c4 <.L0^B1> > d4: 0330000f fence rw,rw > > 00000000000000d8 <.L1^B1>: > d8: 02d78a63 beq a5,a3,10c <.L7> > > 00000000000000dc <.LBE292>: > dc: fff5859b addw a1,a1,-1 > > 00000000000000e0 <.LBB293>: > e0: 00068793 mv a5,a3 > > 00000000000000e4 <.LBE293>: > e4: fa059ae3 bnez a1,98 <.L8> > > 00000000000000e8 <.L12>: > > ======================================================================== > After the patch: > void lockref_get(struct lockref *lockref) > { > CMPXCHG_LOOP( > 78: 00053783 ld a5,0(a0) > > 000000000000007c <.LBB212>: > > static __always_inline int ticket_spin_value_unlocked(arch_spinlock_t > lock) > { > u32 val = lock.val.counter; > > return ((val >> 16) == (val & 0xffff)); > 7c: 00010637 lui a2,0x10 > > 0000000000000080 <.LBE212>: > 80: 06400593 li a1,100 > > 0000000000000084 <.LBB216>: > 84: fff60613 add a2,a2,-1 # ffff <.LLST8+0xf4aa> > > 0000000000000088 <.L8>: > 88: 0007871b sext.w a4,a5 > > 000000000000008c <.LBB217>: > 8c: 0107d69b srlw a3,a5,0x10 > 90: 00c77733 and a4,a4,a2 > 94: 04e69063 bne a3,a4,d4 <.L12> > > 0000000000000098 <.LBB218>: > 98: 4207d693 sra a3,a5,0x20 > 9c: 02079713 sll a4,a5,0x20 > a0: 0016869b addw a3,a3,1 > a4: 02069693 sll a3,a3,0x20 > a8: 02075713 srl a4,a4,0x20 > ac: 00d76733 or a4,a4,a3 > > 00000000000000b0 <.L0^B1>: > b0: 100536af lr.d a3,(a0) > b4: 00f69863 bne a3,a5,c4 <.L1^B1> > b8: 1ae5382f sc.d.rl a6,a4,(a0) > bc: fe081ae3 bnez a6,b0 <.L0^B1> > c0: 0330000f fence rw,rw > > 00000000000000c4 <.L1^B1>: > c4: 04d78a63 beq a5,a3,118 <.L18> > > 00000000000000c8 <.LBE228>: > c8: fff5859b addw a1,a1,-1 > > 00000000000000cc <.LBB229>: > cc: 00068793 mv a5,a3 > > 00000000000000d0 <.LBE229>: > d0: fa059ce3 bnez a1,88 <.L8> > > 00000000000000d4 <.L12>: > { > d4: fe010113 add sp,sp,-32 > d8: 00113c23 sd ra,24(sp) > dc: 00813823 sd s0,16(sp) > e0: 02010413 add s0,sp,32 > ======================================================================== > >> >> I have no opinion about the patch itself, I will note though that the >> argument to the routine is *not* the actual memory-shared lockref, >> instead it's something from the local copy obtained with READ_ONCE >> from the real thing. So I would be surprised if the stock routine was >> generating accesses to that sucker. >> >> Nonetheless, if the patched routine adds nasty asm, that would be nice >> to sort out. >> >> FWIW on x86-64 qspinlock is used (i.e. not the stuff you are patching) >> and I verified there are only 2 memory accesses -- the initial READ_ONCE >> and later cmpxchg. I don't know which archs *don't* use qspinlock. >> >> It also turns out generated asm is quite atrocious and cleaning it up >> may yield a small win under more traffic. Maybe I'll see about it later >> this week. >> >> For example, disassembling lockref_put_return: >> <+0>: mov (%rdi),%rax <-- initial load, expected >> <+3>: mov $0x64,%r8d >> <+9>: mov %rax,%rdx >> <+12>: test %eax,%eax <-- retries loop back here >> <-- this is also the unlocked >> check >> <+14>: jne 0xffffffff8157aba3 <lockref_put_return+67> >> <+16>: mov %rdx,%rsi >> <+19>: mov %edx,%edx >> <+21>: sar $0x20,%rsi >> <+25>: lea -0x1(%rsi),%ecx <-- new.count--; >> <+28>: shl $0x20,%rcx >> <+32>: or %rcx,%rdx >> <+35>: test %esi,%esi >> <+37>: jle 0xffffffff8157aba3 <lockref_put_return+67> >> <+39>: lock cmpxchg %rdx,(%rdi) <-- the attempt to change >> <+44>: jne 0xffffffff8157ab9a <lockref_put_return+58> >> <+46>: shr $0x20,%rdx >> <+50>: mov %rdx,%rax >> <+53>: jmp 0xffffffff81af8540 <__x86_return_thunk> >> <+58>: mov %rax,%rdx >> <+61>: sub $0x1,%r8d <-- retry count check >> <+65>: jne 0xffffffff8157ab6c <lockref_put_return+12> <-- go back >> <+67>: mov $0xffffffff,%eax >> <+72>: jmp 0xffffffff81af8540 <__x86_return_thunk> >> >
On Tue, Aug 8, 2023 at 4:02 PM Mateusz Guzik <mjguzik@gmail.com> wrote: > > On 8/8/23, Guo Ren <guoren@kernel.org> wrote: > > On Mon, Aug 07, 2023 at 08:36:55PM +0200, Mateusz Guzik wrote: > >> On Sun, Jul 30, 2023 at 10:33:08PM -0400, guoren@kernel.org wrote: > >> > From: Guo Ren <guoren@linux.alibaba.com> > >> > > >> > The arch_spin_value_unlocked would cause an unnecessary memory > >> > access to the contended value. Although it won't cause a significant > >> > performance gap in most architectures, the arch_spin_value_unlocked > >> > argument contains enough information. Thus, remove unnecessary > >> > atomic_read in arch_spin_value_unlocked(). > >> > > >> > The caller of arch_spin_value_unlocked() could benefit from this > >> > change. Currently, the only caller is lockref. > >> > > >> > >> Have you verified you are getting an extra memory access from this in > >> lockref? What architecture is it? > > For riscv, this patch could optimize the lock_ref on the same compiling > > condition: > > - After lifting data dependencies, the compiler optimizes the prologue > > behavior, thus the callee-register-saved path becomes optional. This > > is a significant improvement on the lock_ref() self. > > - Compare the "98: & 9c:" lines before the patch and the "88:" line > > after the patch. We saved two memory accesses not only one load. > > > > Now that you mention it, I see riscv in cc. ;) > > Your commit message states "arch_spin_value_unlocked would cause an > unnecessary memory access to the contended value" and that lockref > uses it. Perhaps incorrectly I took it to claim lockref is suffering > extra loads from the area it modifies with cmpxchg -- as I verified, > this is not happening as the argument to arch_spin_value_unlocked is a > copy of the target lockref struct. With this not being a problem, > potential scalability impact goes down a lot. And so happens with the > code from qspinlock on x86-64 there are no extra memory accesses to > anything anyway. x86-64 don't use this code, and This is the ticket-lock. Only riscv & openrisc use it. I didn't send the patch like: static __always_inline int queued_spin_value_unlocked(struct qspinlock lock) { - return !atomic_read(&lock.val); + return !lock.val.counter; } So you can't compare an x86 qspinlock code with a riscv ticket-lock. That's strange. The ticket arch_spin_value_unlocked has a more complex ALU operation than qspinlock's. > > I don't speak riscv asm so can't comment on the result. I'll note Yes, you can't. The x86 is not a party to this patch. > again that extra work for single-threaded use is definitely worth > shaving and may or may not affect throughput in face of other CPUs > messing with lockref. > > You can easily test lockref with will-it-scale, I would suggest the > access() system call which afaics has least amount of unrelated > overhead. You can find the bench here: > https://github.com/antonblanchard/will-it-scale/pull/36/files > > > ======================================================================== > > Before the patch: > > void lockref_get(struct lockref *lockref) > > { > > 78: fd010113 add sp,sp,-48 > > 7c: 02813023 sd s0,32(sp) > > 80: 02113423 sd ra,40(sp) > > 84: 03010413 add s0,sp,48 > > > > 0000000000000088 <.LBB296>: > > CMPXCHG_LOOP( > > 88: 00053783 ld a5,0(a0) > > > > 000000000000008c <.LBB265>: > > } > > > > static __always_inline int ticket_spin_is_locked(arch_spinlock_t *lock) > > { > > u32 val = atomic_read(&lock->val); > > return ((val >> 16) != (val & 0xffff)); > > 8c: 00010637 lui a2,0x10 > > > > 0000000000000090 <.LBE265>: > > 90: 06400593 li a1,100 > > > > 0000000000000094 <.LBB274>: > > 94: fff60613 add a2,a2,-1 # ffff <.LLST8+0xf49a> > > > > 0000000000000098 <.L8>: > > 98: fef42423 sw a5,-24(s0) > > > > 000000000000009c <.LBB269>: > > 9c: fe842703 lw a4,-24(s0) > > > > 00000000000000a0 <.LBE269>: > > a0: 0107569b srlw a3,a4,0x10 > > a4: 00c77733 and a4,a4,a2 > > a8: 04e69063 bne a3,a4,e8 <.L12> > > > > 00000000000000ac <.LBB282>: > > ac: 4207d693 sra a3,a5,0x20 > > b0: 02079713 sll a4,a5,0x20 > > b4: 0016869b addw a3,a3,1 > > b8: 02069693 sll a3,a3,0x20 > > bc: 02075713 srl a4,a4,0x20 > > c0: 00d76733 or a4,a4,a3 > > > > 00000000000000c4 <.L0^B1>: > > c4: 100536af lr.d a3,(a0) > > c8: 00f69863 bne a3,a5,d8 <.L1^B1> > > cc: 1ae5382f sc.d.rl a6,a4,(a0) > > d0: fe081ae3 bnez a6,c4 <.L0^B1> > > d4: 0330000f fence rw,rw > > > > 00000000000000d8 <.L1^B1>: > > d8: 02d78a63 beq a5,a3,10c <.L7> > > > > 00000000000000dc <.LBE292>: > > dc: fff5859b addw a1,a1,-1 > > > > 00000000000000e0 <.LBB293>: > > e0: 00068793 mv a5,a3 > > > > 00000000000000e4 <.LBE293>: > > e4: fa059ae3 bnez a1,98 <.L8> > > > > 00000000000000e8 <.L12>: > > > > ======================================================================== > > After the patch: > > void lockref_get(struct lockref *lockref) > > { > > CMPXCHG_LOOP( > > 78: 00053783 ld a5,0(a0) > > > > 000000000000007c <.LBB212>: > > > > static __always_inline int ticket_spin_value_unlocked(arch_spinlock_t > > lock) > > { > > u32 val = lock.val.counter; > > > > return ((val >> 16) == (val & 0xffff)); > > 7c: 00010637 lui a2,0x10 > > > > 0000000000000080 <.LBE212>: > > 80: 06400593 li a1,100 > > > > 0000000000000084 <.LBB216>: > > 84: fff60613 add a2,a2,-1 # ffff <.LLST8+0xf4aa> > > > > 0000000000000088 <.L8>: > > 88: 0007871b sext.w a4,a5 > > > > 000000000000008c <.LBB217>: > > 8c: 0107d69b srlw a3,a5,0x10 > > 90: 00c77733 and a4,a4,a2 > > 94: 04e69063 bne a3,a4,d4 <.L12> > > > > 0000000000000098 <.LBB218>: > > 98: 4207d693 sra a3,a5,0x20 > > 9c: 02079713 sll a4,a5,0x20 > > a0: 0016869b addw a3,a3,1 > > a4: 02069693 sll a3,a3,0x20 > > a8: 02075713 srl a4,a4,0x20 > > ac: 00d76733 or a4,a4,a3 > > > > 00000000000000b0 <.L0^B1>: > > b0: 100536af lr.d a3,(a0) > > b4: 00f69863 bne a3,a5,c4 <.L1^B1> > > b8: 1ae5382f sc.d.rl a6,a4,(a0) > > bc: fe081ae3 bnez a6,b0 <.L0^B1> > > c0: 0330000f fence rw,rw > > > > 00000000000000c4 <.L1^B1>: > > c4: 04d78a63 beq a5,a3,118 <.L18> > > > > 00000000000000c8 <.LBE228>: > > c8: fff5859b addw a1,a1,-1 > > > > 00000000000000cc <.LBB229>: > > cc: 00068793 mv a5,a3 > > > > 00000000000000d0 <.LBE229>: > > d0: fa059ce3 bnez a1,88 <.L8> > > > > 00000000000000d4 <.L12>: > > { > > d4: fe010113 add sp,sp,-32 > > d8: 00113c23 sd ra,24(sp) > > dc: 00813823 sd s0,16(sp) > > e0: 02010413 add s0,sp,32 > > ======================================================================== > > > >> > >> I have no opinion about the patch itself, I will note though that the > >> argument to the routine is *not* the actual memory-shared lockref, > >> instead it's something from the local copy obtained with READ_ONCE > >> from the real thing. So I would be surprised if the stock routine was > >> generating accesses to that sucker. > >> > >> Nonetheless, if the patched routine adds nasty asm, that would be nice > >> to sort out. > >> > >> FWIW on x86-64 qspinlock is used (i.e. not the stuff you are patching) > >> and I verified there are only 2 memory accesses -- the initial READ_ONCE > >> and later cmpxchg. I don't know which archs *don't* use qspinlock. > >> > >> It also turns out generated asm is quite atrocious and cleaning it up > >> may yield a small win under more traffic. Maybe I'll see about it later > >> this week. > >> > >> For example, disassembling lockref_put_return: > >> <+0>: mov (%rdi),%rax <-- initial load, expected > >> <+3>: mov $0x64,%r8d > >> <+9>: mov %rax,%rdx > >> <+12>: test %eax,%eax <-- retries loop back here > >> <-- this is also the unlocked > >> check > >> <+14>: jne 0xffffffff8157aba3 <lockref_put_return+67> > >> <+16>: mov %rdx,%rsi > >> <+19>: mov %edx,%edx > >> <+21>: sar $0x20,%rsi > >> <+25>: lea -0x1(%rsi),%ecx <-- new.count--; > >> <+28>: shl $0x20,%rcx > >> <+32>: or %rcx,%rdx > >> <+35>: test %esi,%esi > >> <+37>: jle 0xffffffff8157aba3 <lockref_put_return+67> > >> <+39>: lock cmpxchg %rdx,(%rdi) <-- the attempt to change > >> <+44>: jne 0xffffffff8157ab9a <lockref_put_return+58> > >> <+46>: shr $0x20,%rdx > >> <+50>: mov %rdx,%rax > >> <+53>: jmp 0xffffffff81af8540 <__x86_return_thunk> > >> <+58>: mov %rax,%rdx > >> <+61>: sub $0x1,%r8d <-- retry count check > >> <+65>: jne 0xffffffff8157ab6c <lockref_put_return+12> <-- go back > >> <+67>: mov $0xffffffff,%eax > >> <+72>: jmp 0xffffffff81af8540 <__x86_return_thunk> > >> > > > > > -- > Mateusz Guzik <mjguzik gmail.com>
diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h index fdfebcb050f4..90803a826ba0 100644 --- a/include/asm-generic/spinlock.h +++ b/include/asm-generic/spinlock.h @@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) smp_store_release(ptr, (u16)val + 1); } +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) +{ + u32 val = lock.counter; + + return ((val >> 16) == (val & 0xffff)); +} + static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) { - u32 val = atomic_read(lock); + arch_spinlock_t val = READ_ONCE(*lock); - return ((val >> 16) != (val & 0xffff)); + return !arch_spin_value_unlocked(val); } static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) @@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) return (s16)((val >> 16) - (val & 0xffff)) > 1; } -static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) -{ - return !arch_spin_is_locked(&lock); -} - #include <asm/qrwlock.h> #endif /* __ASM_GENERIC_SPINLOCK_H */