On 2020-07-18 8:04 a.m., Helge Deller wrote:
> Hi Dave,
>
> just a few questions and suggestions below....
>
> On 17.07.20 23:15, John David Anglin wrote:
[...]
>> -static inline void arch_spin_lock(arch_spinlock_t *x)
>> +static inline int __pa_ldcw (volatile unsigned int *a)
>> +{
>> +#if __PA_LDCW_ALIGNMENT==16
>> + *(volatile char *)a = 0;
>> +#endif
>>
>> I assume this is planned as a kind of prefetching into cache here?
>> But doesn't it maybe introduce a bug when the first byte
>> (in which you write zero) wasn't zero at the beginning?
>> In that case the following ldcw():
The intention is to dirty the cache line. Note the above generates a stb instruction that operates
on the most significant byte of the lock word. The release uses a stw and sets bit 31 in the least
significant byte of the spin lock word. So, the stb doesn't affect the state of the lock.
When the cache line is dirty, the ldcw instruction may be optimized to operate in cache. This speeds
up the operation.
Another alternative is to use the stby instruction. See the programming note on page 7-135 of the
architecture manual. It doesn't write anything when the address is the left most byte of a word but
it still can be used to dirty the cache line.
Note this is only added when using ldcw and not when using ldcw,co.
>> + return __ldcw(a);
> returns zero, althought it wasn't zero initially?
>
>
>> +}
>> +
>
>> diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
>> index f05c9d5b6b9e..4ba43a3c7c67 100644
>> --- a/arch/parisc/kernel/syscall.S
>> +++ b/arch/parisc/kernel/syscall.S
>> @@ -659,8 +678,15 @@ cas_action:
>> /* Error occurred on load or store */
>> /* Free lock */
>> #ifdef CONFIG_SMP
>> -98: LDCW 0(%sr2,%r20), %r1 /* Barrier */
>> +98: sync /* Barrier */
>> 99: ALTERNATIVE(98b, 99b, ALT_COND_NO_SMP, INSN_NOP)
>> + nop
>> + nop
>> + nop
>> + nop
>> + nop
>> + nop
>> + nop
> I think you should put those nop's before the 99:ALTERNATIVE() instruction above.
> That way they all get replaced by one jump instead of multiple nops (when
> there is only one CPU in the SMP system).
Okay. I'll test. Thought the replacement was just nops.
Dave
--
John David Anglin dave.anglin@xxxxxxxx
