On 2021-10-17 12:30 p.m., Helge Deller wrote:
It seems the warnings are gone if I remove the irq masking.
I see the options:
a) revert the irq masking in syscall.S. Not sure if it really hurts performance.
b) revert the patch from Sven.
c) insert code to turn back irq on in the fault handler if we are on the gateway page.
What is your thought?
After some thought, I believe option a) is the best. I no longer think interrupts can be
disabled in the futex and cmpxchg operations because of COW breaks. This not ideal but
I suspect it's the best we can do.
For the cmpxchg operations in syscall.S, we rely on the code to not schedule off the gateway
page. For the futex, I added code to disable preemption.
So far, I haven't seen the warnings with the attached change but the change is only lightly tested.
Signed-off-by: Dave Anglin <dave.anglin@xxxxxxxx>
---
diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h
index fceb9cf02fb3..c170cbe2c6d6 100644
--- a/arch/parisc/include/asm/futex.h
+++ b/arch/parisc/include/asm/futex.h
@@ -13,35 +13,37 @@
sixteen four-word locks. */
static inline void
-_futex_spin_lock_irqsave(u32 __user *uaddr, unsigned long int *flags)
+_futex_spin_lock(u32 __user *uaddr)
{
extern u32 lws_lock_start[];
long index = ((long)uaddr & 0x3f8) >> 1;
arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
- local_irq_save(*flags);
+ preempt_disable();
arch_spin_lock(s);
}
static inline void
-_futex_spin_unlock_irqrestore(u32 __user *uaddr, unsigned long int *flags)
+_futex_spin_unlock(u32 __user *uaddr)
{
extern u32 lws_lock_start[];
long index = ((long)uaddr & 0x3f8) >> 1;
arch_spinlock_t *s = (arch_spinlock_t *)&lws_lock_start[index];
arch_spin_unlock(s);
- local_irq_restore(*flags);
+ preempt_enable();
}
static inline int
arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- unsigned long int flags;
int oldval, ret;
u32 tmp;
- _futex_spin_lock_irqsave(uaddr, &flags);
-
ret = -EFAULT;
+ if (unlikely(get_user(oldval, uaddr) != 0))
+ goto out_pagefault_enable;
+
+ _futex_spin_lock(uaddr);
+
if (unlikely(get_user(oldval, uaddr) != 0))
goto out_pagefault_enable;
@@ -72,7 +74,7 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
ret = -EFAULT;
out_pagefault_enable:
- _futex_spin_unlock_irqrestore(uaddr, &flags);
+ _futex_spin_unlock(uaddr);
if (!ret)
*oval = oldval;
@@ -85,7 +87,6 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
u32 oldval, u32 newval)
{
u32 val;
- unsigned long flags;
/* futex.c wants to do a cmpxchg_inatomic on kernel NULL, which is
* our gateway page, and causes no end of trouble...
@@ -102,19 +103,19 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
* address. This should scale to a couple of CPUs.
*/
- _futex_spin_lock_irqsave(uaddr, &flags);
+ _futex_spin_lock(uaddr);
if (unlikely(get_user(val, uaddr) != 0)) {
- _futex_spin_unlock_irqrestore(uaddr, &flags);
+ _futex_spin_unlock(uaddr);
return -EFAULT;
}
if (val == oldval && unlikely(put_user(newval, uaddr) != 0)) {
- _futex_spin_unlock_irqrestore(uaddr, &flags);
+ _futex_spin_unlock(uaddr);
return -EFAULT;
}
*uval = val;
- _futex_spin_unlock_irqrestore(uaddr, &flags);
+ _futex_spin_unlock(uaddr);
return 0;
}
diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
index 3f24a0af1e04..5b8feeeedf40 100644
--- a/arch/parisc/kernel/syscall.S
+++ b/arch/parisc/kernel/syscall.S
@@ -603,13 +603,11 @@ cas_nocontend:
# endif
/* ENABLE_LWS_DEBUG */
- rsm PSW_SM_I, %r0 /* Disable interrupts */
/* COW breaks can cause contention on UP systems */
LDCW 0(%sr2,%r20), %r28 /* Try to acquire the lock */
cmpb,<>,n %r0, %r28, cas_action /* Did we get it? */
cas_wouldblock:
ldo 2(%r0), %r28 /* 2nd case */
- ssm PSW_SM_I, %r0
b lws_exit /* Contended... */
ldo -EAGAIN(%r0), %r21 /* Spin in userspace */
@@ -645,8 +643,6 @@ cas_action:
/* Clear thread register indicator */
stw %r0, 4(%sr2,%r20)
#endif
- /* Enable interrupts */
- ssm PSW_SM_I, %r0
/* Return to userspace, set no error */
b lws_exit
copy %r0, %r21
@@ -658,7 +654,6 @@ cas_action:
#if ENABLE_LWS_DEBUG
stw %r0, 4(%sr2,%r20)
#endif
- ssm PSW_SM_I, %r0
b lws_exit
ldo -EFAULT(%r0),%r21 /* set errno */
nop
@@ -770,13 +765,11 @@ cas2_lock_start:
shlw %r20, 4, %r20
add %r20, %r28, %r20
- rsm PSW_SM_I, %r0 /* Disable interrupts */
/* COW breaks can cause contention on UP systems */
LDCW 0(%sr2,%r20), %r28 /* Try to acquire the lock */
cmpb,<>,n %r0, %r28, cas2_action /* Did we get it? */
cas2_wouldblock:
ldo 2(%r0), %r28 /* 2nd case */
- ssm PSW_SM_I, %r0
b lws_exit /* Contended... */
ldo -EAGAIN(%r0), %r21 /* Spin in userspace */
@@ -856,8 +849,6 @@ cas2_action:
cas2_end:
/* Free lock */
stw,ma %r20, 0(%sr2,%r20)
- /* Enable interrupts */
- ssm PSW_SM_I, %r0
/* Return to userspace, set no error */
b lws_exit
copy %r0, %r21
@@ -866,7 +857,6 @@ cas2_end:
/* Error occurred on load or store */
/* Free lock */
stw,ma %r20, 0(%sr2,%r20)
- ssm PSW_SM_I, %r0
ldo 1(%r0),%r28
b lws_exit
ldo -EFAULT(%r0),%r21 /* set errno */
