| Message ID | 8186e8fe-1c64-c9fd-ca3c-f7514fb40428@bell.net (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | parisc: Fix mask used to select futex spinlock | expand |
Am Dienstag, 21. Dezember 2021, 19:33:16 CET schrieb John David Anglin: > Fix mask used to select futex spinlock. > > The address bits used to select the futex spinlock need to match those used > in the LWS code in syscall.S. The mask 0x3f8 only selects 7 bits. It > should select 8 bits. This change looks like this should become a helper macro or something like that so the code will stay in sync. Can the mask be shared with the LWS code with a constant while at it so it will also include that? Eike
On 2021-12-21 1:45 p.m., Rolf Eike Beer wrote: > Am Dienstag, 21. Dezember 2021, 19:33:16 CET schrieb John David Anglin: >> Fix mask used to select futex spinlock. >> >> The address bits used to select the futex spinlock need to match those used >> in the LWS code in syscall.S. The mask 0x3f8 only selects 7 bits. It >> should select 8 bits. > This change looks like this should become a helper macro or something like > that so the code will stay in sync. Can the mask be shared with the LWS code > with a constant while at it so it will also include that? I understand the point but it's rather convoluted. We would need a macro for the assembly code. Then the macro would need to be embedded in an asm for C. Then, there's the shift for the int* type in the C code. I am proposing to rewrite this code so the spinlock pointer is only computed once, but Helge wanted a change that could be easily back ported. Dave
On 12/21/21 20:27, John David Anglin wrote: > On 2021-12-21 1:45 p.m., Rolf Eike Beer wrote: >> Am Dienstag, 21. Dezember 2021, 19:33:16 CET schrieb John David Anglin: >>> Fix mask used to select futex spinlock. >>> >>> The address bits used to select the futex spinlock need to match those used >>> in the LWS code in syscall.S. The mask 0x3f8 only selects 7 bits. It >>> should select 8 bits. >> This change looks like this should become a helper macro or something like >> that so the code will stay in sync. Can the mask be shared with the LWS code >> with a constant while at it so it will also include that? > I understand the point but it's rather convoluted. We would need a macro for the assembly > code. Then the macro would need to be embedded in an asm for C. Then, there's the shift > for the int* type in the C code. > > I am proposing to rewrite this code so the spinlock pointer is only computed once, but Helge > wanted a change that could be easily back ported. Right. I think this is a small but important fix, which I can easily push back into older kernels. See Dave's other patch ("[PATCH v1] parisc: Rewrite light-weight syscall and futex code"). There he rewrote the code anyway. Helge
diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h index 70cf8f0a7617..9cd4dd6e63ad 100644 --- a/arch/parisc/include/asm/futex.h +++ b/arch/parisc/include/asm/futex.h @@ -14,7 +14,7 @@ static inline void _futex_spin_lock(u32 __user *uaddr) { extern u32 lws_lock_start[]; - long index = ((long)uaddr & 0x3f8) >> 1; + long index = ((long)uaddr & 0x7f8) >> 1; arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index]; preempt_disable(); arch_spin_lock(s); @@ -24,7 +24,7 @@ static inline void _futex_spin_unlock(u32 __user *uaddr) { extern u32 lws_lock_start[]; - long index = ((long)uaddr & 0x3f8) >> 1; + long index = ((long)uaddr & 0x7f8) >> 1; arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index]; arch_spin_unlock(s); preempt_enable();
Fix mask used to select futex spinlock. The address bits used to select the futex spinlock need to match those used in the LWS code in syscall.S. The mask 0x3f8 only selects 7 bits. It should select 8 bits. This change fixes the glibc nptl/tst-cond24 and nptl/tst-cond25 tests. Signed-off-by: John David Anglin <dave.anglin@bell.net> ---