Re: [PATCH 1/1] ARM mm: Fix RT life lock on ASID rollover

[Carsten Emde <C.Emde@osadl.org>]

Hi Will,

>> The original mechanism to synchronize all online CPUs after ASID
>> reallocation used an IPI mechanism with IRQs enabled. This is a valid
>> mechanism in mainline. An RT kernel, however, may hang forever due to a
>> life lock between sending the IPI and waiting for the ASID lock to be
>> freed. Such hangers were observed and analyzed using JTAG hardware
>> debugging on an OMAP4430 board. Mean uptime was about two days with a
>> maximum of seven days observed once.
>>
>> In 2012, Will Deacon provided a new ASID rollover synchronization
>> mechanism without IPI broadcasting. This *improved* a suboptimal
>> implementation in mainline - but it *fixed* a disastrous bug in RT
>> kernels that was extremely hard to decode.
>
> Ha, that's a nice and unanticipated side-effect :)
Yeah, thanks a lot for fixing it.

> [..]
> You seem to have a few extra bits and pieces in here, which you might not
> care about:
Was a long way from current mainline back to a 3.0 vendor kernel mess - 
so I used a shortcut that obviously picked up some more code than needed.

>> Index: linux-3.0.80-rt108/arch/arm/include/asm/tlbflush.h
>> ===================================================================
>> --- linux-3.0.80-rt108.orig/arch/arm/include/asm/tlbflush.h
>> +++ linux-3.0.80-rt108/arch/arm/include/asm/tlbflush.h
>> @@ -14,7 +14,6 @@
>>
>>   #include <asm/glue.h>
>>
>> -#define TLB_V3_PAGE    (1 << 0)
>>   #define TLB_V4_U_PAGE  (1 << 1)
>>   #define TLB_V4_D_PAGE  (1 << 2)
>>   #define TLB_V4_I_PAGE  (1 << 3)
>> @@ -22,7 +21,6 @@
>>   #define TLB_V6_D_PAGE  (1 << 5)
>>   #define TLB_V6_I_PAGE  (1 << 6)
>>
>> -#define TLB_V3_FULL    (1 << 8)
>>   #define TLB_V4_U_FULL  (1 << 9)
>>   #define TLB_V4_D_FULL  (1 << 10)
>>   #define TLB_V4_I_FULL  (1 << 11)
>> @@ -34,16 +32,15 @@
>>   #define TLB_V6_D_ASID  (1 << 17)
>>   #define TLB_V6_I_ASID  (1 << 18)
>>
>> -#define TLB_BTB                (1 << 28)
>> +#define TLB_V6_BP      (1 << 19)
>
> This hunk (and related ones) are from a patch adding branch predictor
> maintenance that I also wrote. It's harmless, but you likely don't need
> it.
OK, will check.

>> +#ifdef CONFIG_ARM_ERRATA_798181
>> +static inline void dummy_flush_tlb_a15_erratum(void)
>> +{
>> +       /*
>> +        * Dummy TLBIMVAIS. Using the unmapped address 0 and ASID 0.
>> +        */
>> +       asm("mcr p15, 0, %0, c8, c3, 1" : : "r" (0));
>> +       dsb();
>> +}
>> +#else
>> +static inline void dummy_flush_tlb_a15_erratum(void)
>> +{
>> +}
>> +#endif
>
> And this is an A15 erratum workaround from Catalin. Actually, the original
> version of that workaround didn't interact nicely with PREEMPT kernels, so
> you should double-check what you've got (it was fixed recently in mainline).
>
> Furthermore, the workaround requires IPIs on TLB invalidation, so you might
> have your livelock problem again...
Hmm, will check as well.

>>   /*
>> Index: linux-3.0.80-rt108/arch/arm/mm/alignment.c
>> ===================================================================
>> --- linux-3.0.80-rt108.orig/arch/arm/mm/alignment.c
>> +++ linux-3.0.80-rt108/arch/arm/mm/alignment.c
>> @@ -819,6 +819,7 @@ do_alignment(unsigned long addr, unsigne
>>                  break;
>>
>>          case 0x08000000:        /* ldm or stm, or thumb-2 32bit instruction */
>> +               offset.un = 0;
>>                  if (thumb2_32b)
>>                          handler = do_alignment_t32_to_handler(&instr, regs, &offset);
>>                  else
> Unrelated?
This fixes a compiler warning:
   CC      arch/arm/mm/alignment.o
arch/arm/mm/alignment.c: In function ‘do_alignment’:
arch/arm/mm/alignment.c:327:15: warning: ‘offset.un’ may be used 
uninitialized in this function [-Wuninitialized]
arch/arm/mm/alignment.c:749:21: note: ‘offset.un’ was declared here

Should have gone into a separate patch.

Thanks,
	-Carsten.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html