[PATCH] Revert "powerpc/irq: Don't open code irq_soft_mask helpers"

Michael Ellerman mpe at ellerman.id.au
Thu Sep 1 13:52:05 AEST 2022


Christophe Leroy <christophe.leroy at csgroup.eu> writes:
> Le 31/08/2022 à 15:10, Michael Ellerman a écrit :
>> This reverts commit ef5b570d3700fbb8628a58da0487486ceeb713cd.
>> 
>> Zhouyi reported that commit is causing crashes when running rcutorture
>> with KASAN enabled:
>> 
>>    BUG: using smp_processor_id() in preemptible [00000000] code: rcu_torture_rea/100
>>    caller is rcu_preempt_deferred_qs_irqrestore+0x74/0xed0
>>    CPU: 4 PID: 100 Comm: rcu_torture_rea Tainted: G        W          5.19.0-rc5-next-20220708-dirty #253
>>    Call Trace:
>>      dump_stack_lvl+0xbc/0x108 (unreliable)
>>      check_preemption_disabled+0x154/0x160
>>      rcu_preempt_deferred_qs_irqrestore+0x74/0xed0
>>      __rcu_read_unlock+0x290/0x3b0
>>      rcu_torture_read_unlock+0x30/0xb0
>>      rcutorture_one_extend+0x198/0x810
>>      rcu_torture_one_read+0x58c/0xc90
>>      rcu_torture_reader+0x12c/0x360
>>      kthread+0x1e8/0x220
>>      ret_from_kernel_thread+0x5c/0x64
>> 
>> KASAN will generate instrumentation instructions around the
>> WRITE_ONCE(local_paca->irq_soft_mask, mask):
>> 
>>     0xc000000000295cb0 <+0>:	addis   r2,r12,774
>>     0xc000000000295cb4 <+4>:	addi    r2,r2,16464
>>     0xc000000000295cb8 <+8>:	mflr    r0
>>     0xc000000000295cbc <+12>:	bl      0xc00000000008bb4c <mcount>
>>     0xc000000000295cc0 <+16>:	mflr    r0
>>     0xc000000000295cc4 <+20>:	std     r31,-8(r1)
>>     0xc000000000295cc8 <+24>:	addi    r3,r13,2354
>>     0xc000000000295ccc <+28>:	mr      r31,r13
>>     0xc000000000295cd0 <+32>:	std     r0,16(r1)
>>     0xc000000000295cd4 <+36>:	stdu    r1,-48(r1)
>>     0xc000000000295cd8 <+40>:	bl      0xc000000000609b98 <__asan_store1+8>
>>     0xc000000000295cdc <+44>:	nop
>>     0xc000000000295ce0 <+48>:	li      r9,1
>>     0xc000000000295ce4 <+52>:	stb     r9,2354(r31)
>>     0xc000000000295ce8 <+56>:	addi    r1,r1,48
>>     0xc000000000295cec <+60>:	ld      r0,16(r1)
>>     0xc000000000295cf0 <+64>:	ld      r31,-8(r1)
>>     0xc000000000295cf4 <+68>:	mtlr    r0
>> 
>> If there is a context switch before "stb     r9,2354(r31)", r31 may
>> not equal to r13, in such case, irq soft mask will not work.
>> 
>> The usual solution of marking the code ineligible for instrumentation
>> forces the code out-of-line, which we would prefer to avoid. Christophe
>> proposed a partial revert, but Nick raised some concerns with that. So
>> for now do a full revert.
>> 
>> Reported-by: Zhouyi Zhou <zhouzhouyi at gmail.com>
>> [mpe: Construct change log based on Zhouyi's original report]
>> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
>> ---
>>   arch/powerpc/include/asm/hw_irq.h | 43 ++++++++++++++++++++++++++-----
>>   1 file changed, 36 insertions(+), 7 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
>> index 3c8cb48f88ae..983551859891 100644
>> --- a/arch/powerpc/include/asm/hw_irq.h
>> +++ b/arch/powerpc/include/asm/hw_irq.h
>> @@ -113,7 +113,14 @@ static inline void __hard_RI_enable(void)
>>   
>>   static inline notrace unsigned long irq_soft_mask_return(void)
>>   {
>> -	return READ_ONCE(local_paca->irq_soft_mask);
>> +	unsigned long flags;
>> +
>> +	asm volatile(
>> +		"lbz %0,%1(13)"
>> +		: "=r" (flags)
>> +		: "i" (offsetof(struct paca_struct, irq_soft_mask)));
>> +
>> +	return flags;
>>   }
>>   
>>   /*
>> @@ -140,24 +147,46 @@ static inline notrace void irq_soft_mask_set(unsigned long mask)
>>   	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
>>   		WARN_ON(mask && !(mask & IRQS_DISABLED));
>>   
>> -	WRITE_ONCE(local_paca->irq_soft_mask, mask);
>> -	barrier();
>> +	asm volatile(
>> +		"stb %0,%1(13)"
>> +		:
>> +		: "r" (mask),
>> +		  "i" (offsetof(struct paca_struct, irq_soft_mask))
>> +		: "memory");
>>   }
>
> Only the above changes need to be reverted, below ones should remain as 
> they are.

I agree, except that it's nearly rc4.

So I want to go back to code that's been well tested, and we can do any
further changes in next for v6.1.

cheers


More information about the Linuxppc-dev mailing list