Re: [PATCH 09/17] m68k: Implement xor_unlock_is_negative_byte
From: Greg Ungerer
Date: Wed Sep 20 2023 - 03:47:05 EST
On 19/9/23 00:37, Greg Ungerer wrote:
On 17/9/23 00:34, Matthew Wilcox wrote:
On Sat, Sep 16, 2023 at 11:11:32PM +1000, Greg Ungerer wrote:
On 16/9/23 04:36, Matthew Wilcox (Oracle) wrote:
Using EOR to clear the guaranteed-to-be-set lock bit will test the
negative flag just like the x86 implementation. This should be
more efficient than the generic implementation in filemap.c. It
would be better if m68k had __GCC_ASM_FLAG_OUTPUTS__.
Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
---
arch/m68k/include/asm/bitops.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/m68k/include/asm/bitops.h b/arch/m68k/include/asm/bitops.h
index e984af71df6b..909ebe7cab5d 100644
--- a/arch/m68k/include/asm/bitops.h
+++ b/arch/m68k/include/asm/bitops.h
@@ -319,6 +319,20 @@ arch___test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
return test_and_change_bit(nr, addr);
}
+static inline bool xor_unlock_is_negative_byte(unsigned long mask,
+ volatile unsigned long *p)
+{
+ char result;
+ char *cp = (char *)p + 3; /* m68k is big-endian */
+
+ __asm__ __volatile__ ("eor.b %1, %2; smi %0"
The ColdFire members of the 68k family do not support byte size eor:
CC mm/filemap.o
{standard input}: Assembler messages:
{standard input}:824: Error: invalid instruction for this architecture; needs 68000 or higher (68000 [68ec000, 68hc000, 68hc001, 68008, 68302, 68306, 68307, 68322, 68356], 68010, 68020 [68k, 68ec020], 68030 [68ec030], 68040 [68ec040], 68060 [68ec060], cpu32 [68330, 68331, 68332, 68333, 68334, 68336, 68340, 68341, 68349, 68360], fidoa [fido]) -- statement `eor.b #1,3(%a0)' ignored
Well, that sucks. What do you suggest for Coldfire?
I am not seeing an easy way to not fall back to something like the MIPS
implementation for ColdFire. Could obviously assemblerize this to do better
than gcc, but if it has to be atomic I think we are stuck with the irq locking.
static inline bool cf_xor_is_negative_byte(unsigned long mask,
volatile unsigned long *addr)
{
unsigned long flags;
unsigned long data;
local_irq_save(flags)
data = *addr;
*addr = data ^ mask;
local_irq_restore(flags);
return (data & BIT(7)) != 0;
}
The problem with this C implementation is that need to use loal_irq_save()
which results in some ugly header dependencies trying top include irqflags.h.
This version at least compiles and run, though we can probably do better still.
diff --git a/arch/m68k/include/asm/bitops.h b/arch/m68k/include/asm/bitops.h
index e984af71df6b..99392c26e784 100644
--- a/arch/m68k/include/asm/bitops.h
+++ b/arch/m68k/include/asm/bitops.h
@@ -319,6 +319,48 @@ arch___test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
return test_and_change_bit(nr, addr);
}
+static inline bool cf_xor_unlock_is_negative_byte(unsigned long mask,
+ volatile unsigned long *addr)
+{
+ unsigned long data;
+
+ asm volatile (
+ "move.w %%sr,%%d1 \n\t"
+ "move.w %%d1,%%d0 \n\t"
+ "ori.l #0x0700,%%d0 \n\t"
+ "move.w %%d0,%%sr \n\t"
+
+ "move.l %2@,%0 \n\t"
+ "eor.l %1,%0 \n\t"
+ "move.l %0,%2@ \n\t"
+
+ "movew %%d1,%%sr \n"
+ : "=d" (data)
+ : "di" (mask), "a" (addr)
+ : "cc", "%d0", "%d1", "memory");
+
+ return (data & BIT(7)) != 0;
+}
+
+static inline bool m68k_xor_unlock_is_negative_byte(unsigned long mask,
+ volatile unsigned long *p)
+{
+ char result;
+ char *cp = (char *)p + 3; /* m68k is big-endian */
+
+ __asm__ __volatile__ ("eor.b %1, %2; smi %0"
+ : "=d" (result)
+ : "di" (mask), "o" (*cp)
+ : "memory");
+ return result;
+}
+
+#if defined(CONFIG_COLDFIRE)
+#define xor_unlock_is_negative_byte(mask, p) cf_xor_unlock_is_negative_byte(mask, p)
+#else
+#define xor_unlock_is_negative_byte(mask, p) m68k_xor_unlock_is_negative_byte(mask, p)
+#endif
+
/*
* The true 68020 and more advanced processors support the "bfffo"
* instruction for finding bits. ColdFire and simple 68000 parts
Regards
Greg