On 2021-12-23 2:31 p.m., Helge Deller wrote:
On 12/23/21 20:17, John David Anglin wrote:
On 2021-12-23 1:47 p.m., Helge Deller wrote:
...
In order to do this, we need a mechanism to trigger COW breaks outside the
critical region. Fortunately, parisc has the "stbys,e" instruction. When
the leftmost byte of a word is addressed, this instruction triggers all
the exceptions of a normal store but it does not write to memory. Thus,
we can use it to trigger COW breaks outside the critical region without
modifying the data that is to be updated atomically.
...
diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h
index 9cd4dd6e63ad..8f97db995b04 100644
--- a/arch/parisc/include/asm/futex.h
+++ b/arch/parisc/include/asm/futex.h
...
+static inline bool _futex_force_interruptions(unsigned long ua)
+{
+ bool result;
+
+ __asm__ __volatile__(
+ "1:\tldw 0(%1), %0\n"
+ "2:\tstbys,e %%r0, 0(%1)\n"
+ "\tcomclr,= %%r0, %%r0, %0\n"
+ "3:\tldi 1, %0\n"
+ ASM_EXCEPTIONTABLE_ENTRY(1b, 3b)
+ ASM_EXCEPTIONTABLE_ENTRY(2b, 3b)
+ : "=&r" (result) : "r" (ua) : "memory"
+ );
+ return result;
I wonder if we can get rid of the comclr,= instruction by using
ASM_EXCEPTIONTABLE_ENTRY_EFAULT instead of ASM_EXCEPTIONTABLE_ENTRY,
e.g.:
diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h
index 8f97db995b04..ea052f013865 100644
--- a/arch/parisc/include/asm/futex.h
+++ b/arch/parisc/include/asm/futex.h
@@ -21,20 +21,21 @@ static inline unsigned long _futex_hash_index(unsigned long ua)
* if load and store fault.
*/
-static inline bool _futex_force_interruptions(unsigned long ua)
+static inline unsigned long _futex_force_interruptions(unsigned long ua)
{
- bool result;
+ register unsigned long error __asm__ ("r8") = 0;
+ register unsigned long temp;
__asm__ __volatile__(
- "1:\tldw 0(%1), %0\n"
- "2:\tstbys,e %%r0, 0(%1)\n"
- "\tcomclr,= %%r0, %%r0, %0\n"
- "3:\tldi 1, %0\n"
- ASM_EXCEPTIONTABLE_ENTRY(1b, 3b)
- ASM_EXCEPTIONTABLE_ENTRY(2b, 3b)
- : "=&r" (result) : "r" (ua) : "memory"
+ "1:\tldw 0(%2), %0\n"
+ "2:\tstbys,e %%r0, 0(%2)\n"
+ "3:\n"
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 3b)
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, 3b)
+ : "=r" (temp), "=r" (error)
+ : "r" (ua), "1" (error) : "memory"
);
- return result;
+ return error;
}
I don't think this is a win.
1) Register %r8 needs to get loaded with 0. So, that's one instruction.
True. On the other hand you don't need the "ldi 1,%0" then, which brings parity.
2) Register %r8 is a caller saves register. Using it will cause gcc to save and restore it from stack. This may
cause a stack frame to be created when one isn't needed. The save and restore instructions are more
expensive, particularly if they cause a TLB miss.
Because of this reason, wouln't it make sense to switch the uaccess functions away from r8 too,
and use another temp register in both?
Yes. I think it would be best to use %r28. Then, error will be in correct register for return.
The variables temp and error can combined.
Note that the comclr both clears result and nullifies the following ldi instruction, so it is not executed in the fast path.
Yes, but when emulating with qemu, it generates comparism and jumps, while the loading of r8 (see 1)), is trivial.
In this case, the comparison is always true. Does qemu detect that?
The comclr is equivalent to setting target register to 0 and a branch to .+8.
If we change the uaccess functions away from r8, then we can drop comclr (and save one instruction).
If we can use %r28, we should be able to save one instruction.
It's slightly less work for me if you change the uaccess and update the asm at the same time.
Dave
--
John David Anglin dave.anglin@xxxxxxxx
