Hi Matthew,
Am 21.07.2023 um 13:32 schrieb Matthew Wilcox:
On Fri, Jul 21, 2023 at 01:12:48PM +1200, Michael Schmitz wrote:
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
I'm not quite sure what values you're printing here? And whether
they're hex or decimal. Could you show me the printk string?
All hex, except the last (nr). Values are mem (as modified by the
original code), copy of mem (as modified by inline asm), the original
value of mem, and nr.
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:
Ah thanks. The perils of working from Motorola official docs and then
trying to use the GNU assembler ...
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).
OK, so the new code would set the byte to 0xff (that's how Sxx works).
But it's returning a bool, so that shouldn't matter. 2084/2084/2085
would make sense; one of the two functions has cleared the bottom bit.
Bit 7 was already set before xor, and wasn't cleared. I suspect that's why
the return value is no longer 1?
Ah, it's not supposed to be cleared. The way this works is that bit 0
is the lock bit; if someone's waiting on the folio, they set bit 7. If
bit 7 is set when we clear bit 0, we look on the wait queue. If there's
nobody on the wait queue, we clear bit 7.
Right, that's what I meant to say. I'd only seen cases where bit 0 had
been set and was cleared. This isn't an actual production system of
sorts, just an ARAnyM instance I can fire up quickly to see patched
kernels crash horribly.
So ... I think you've fixed it!
This is what I have tests running on right now:
static inline bool clear_bit_unlock_is_negative_byte(unsigned int nr,
volatile unsigned long *p)
{
unsigned char *cp = (unsigned char *) p;
char result;
char mask = 1 << nr; /* nr guaranteed to be < 7 */
__asm__ __volatile__ ("eori.b %1, %2; smi %0"
: "=d" (result)
: "i" (mask), "o" (*(cp+3))
: "memory");
return result;
}
I'm sure you can do all the casting to char and increment by 3 in the
asm argument...
If there's a simple way to exercise this code path using standard Unix
tools (or stress-ng which I ought to have somewhere), drop me a hint.
Cheers,
Michael
