Hi Matthew,
Am 21.07.2023 um 10:37 schrieb Michael Schmitz:
Hi Matthew,
Am 21.07.2023 um 07:27 schrieb Matthew Wilcox:
I'm looking to implement clear_bit_unlock_is_negative_byte() on every
architecture so we can delete the ifdeffery around maybe-we-have-it
and remove the simple implementation from filemap.c. Here's what I've
come up with for m68k:
+static inline bool clear_bit_unlock_is_negative_byte(unsigned int nr,
+ volatile unsigned long *p)
+{
+ char result;
+ char mask = 1 << nr; /* nr guaranteed to be < 7 */
+
+ __asm__ __volatile__ ("eori %1, %2; smi %0"
+ : "=d" (result)
+ : "i" (mask), "o" (*p)
+ : "memory");
+ return result;
+}
It compiles, so I feel Very Pleased With Myself, since I haven't written
m68k assmbly in 25 years. But I have questions.
Unfortunately, after plugging this in, the boot hangs at the first block
device encountered:
calling proc_hardware_init+0x0/0x20 @ 1
initcall proc_hardware_init+0x0/0x20 returned 0 after 40 usecs
calling rtc_init+0x0/0x5e @ 1
initcall rtc_init+0x0/0x5e returned 0 after 40 usecs
calling atari_nvram_init+0x0/0x42 @ 1
initcall atari_nvram_init+0x0/0x42 returned 0 after 40 usecs
calling nfhd_init+0x0/0x206 @ 1
nfhd8: found device with 20971440 blocks (512 bytes)
Doesn't eat up memory, doesn't sit in a tight spin, just never proceeds
past that point.
Running old and new code side by side (well, actually sequentially) in
clear_bit_unlock_is_negative_byte() and comparing results, the return
values generated by both options match but the actual contents of the
pointer passed to the function (after the mods) does differ:
nfhd8: found device with 20971440 blocks (512 bytes)
cbu_inb memval mismatch: 2004 12005
cbu_inb memval mismatch: 2004 12005
nfhd8: AHDI p1 p2
cbu_inb memval mismatch: 26 10027
cbu_inb memval mismatch: 26 10027
(first value is from the old code, second value from the new code).
Logging the bit nr. and value passed in:
nfhd8: found device with 20971440 blocks (512 bytes)
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
nfhd8: AHDI p1 p2
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 36 10037 37 0
Plenty more later in the boot:
calling brd_init+0x0/0xca @ 1
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 36 10037 37 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 26 10027 27 0
cbu_inb memval mismatch: 2004 12005 2005 0
cbu_inb memval mismatch: 2004 12005 2005 0
It looks to me that the eori operates on a 16 bit word here?
And it's bits 32-17 of the longword.
The instruction you need is eori.b, and you'll have to increase the mem
pointer by 3 bytes. With that change, I see no further mismatches until
the return values begin to differ once disk access begins:
sd 0:0:1:0: [sdb] Preferred minimum I/O size 512 bytes
cbu_inb retval mismatch: 1 ff 2084 2084 2085 0
rtc-generic rtc-generic: registered as rtc0
cbu_inb retval mismatch: 1 ff 2094 2094 2095 0
...
sd 0:0:0:0: [sda] Attached SCSI disk
probe of 0:0:0:0 returned 0 after 58395182 usecs
cbu_inb retval mismatch: 1 ff 2094 2094 2095 0
sdb: RDSK (512) sdb1 (DOS^G)(res 2 spb 2) sdb2 (SFS^B)(res 2 spb 1)
sdb3 (SFS^B)(res 2 spb 2) sdb4 ((res 2 spb 1)
sdb: p4 size 18446744071971831216 extends beyond EOD, enabling native
capacity
cbu_inb retval mismatch: 1 ff 2084 2084 2085 0
(return value from old and new code, value of mem from old and new code,
original value, bit nr).
Bit 7 was already set before xor, and wasn't cleared. I suspect that's
why the return value is no longer 1?
Time to dig out that 68k programmer's manual...
Cheers,
Michael
(Disclaimer: I have written m68k assembly occasionally in the past
years, but I struggle with inline asm...)
Cheers,
Michael
First, m68k is big-endian, so I suspect I'm accessing the wrong byte.
Should something in there be adding 3 to 'p'? Better to do it in the
asm, or in the constraints so the compiler can see it?
Second, have I properly communicated to the assembler that this is
a byte-size operation, and it needs to check bit 7 and not bits 15 or 31
to set the negative flag?
Third, can this be done better? x86 has __GCC_ASM_FLAG_OUTPUTS__
so it doesn't need the equivalent of the SMI instruction to move the
condition to an output variable; it can just tell the compiler that
the N flag communicates the result that it's looking for. Does m68k
have __GCC_ASM_FLAG_OUTPUTS__ or did nobody do that work yet?
Fourth, we could do this is with ANDI instead of EORI. It's mildly
safer, but we really shouldn't have two threads clearing the lock bit
that race with each other. We can't do it with BCLR because that
doesn't set the N flag. If we do that, we'd need to invert the mask.
Appreciate your time looking at this.
