Re: [PATCH v5 07/22] powerpc: Fix Race conditions in pthread cancellation [BZ#12683]

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 14/04/23 12:27, Paul E Murphy wrote:
> 
> 
> On 4/10/23 2:58 PM, Adhemerval Zanella via Libc-alpha wrote:
>> By adding the required syscall_cancel.S.
>>
>> Checked on powerpc64le-linux-gnu, powerpc64-linux-gnu and
>> powerpc-linux-gnu.
>> ---
> Thanks. A couple trivial comments, but otherwise LGTM.
> 
> Reviewed-by: Paul E. Murphy <murphyp@linux.ibm.com>
> 
>> +ENTRY (__syscall_cancel_arch)
>> +
>> +    .globl __syscall_cancel_arch_start
>> +__syscall_cancel_arch_start:
>> +
>> +    /* if (*cancelhandling & CANCELED_BITMASK)
>> +         __syscall_do_cancel()  */
>> +    lwz     r0,0(r3)
>> +    andi.   r0,r0,TCB_CANCELED_BITMASK
>> +    bne-    1f
> Really trivial opinion nit, the branch hint should be removed.

Ack.

> 
>> +
>> +    /* Issue a 6 argument syscall, the nr [r4] being the syscall
>> +       number.  */
>> +    mr      r0,r4
>> +    mr      r3,r5
>> +    mr      r4,r6
>> +    mr      r5,r7
>> +    mr      r6,r8
>> +    mr      r7,r9
>> +    mr      r8,r10
>> +    sc
> For consistency with the other syscall usage, can this also use the appropriate wrappers for using scv?

That is tricky for similar reasons i386 and i64 have to use the old
syscall mechanism instead of the vDSO: the cancel syscall bridge 
need to have a mark just after the syscall instruction to know
whether it has any side-effects.  Having two syscall mechanism means
essentially two marks depending of the syscall used.

I think it should be possible to two two marks for powerpc, one
for default 'sc' and another for 'svc'. It would require an arch-specific
cancellation-pc-check.h for powerpc.

> 
>> +
>> +    .globl __syscall_cancel_arch_end
>> +__syscall_cancel_arch_end:
>> +
>> +    bnslr+
>> +    neg    r3,r3
>> +    blr
>> +
>> +    /* Although the __syscall_do_cancel do not return, we need to stack
>> +       being set correctly for unwind.  */
>> +1:
>> +    TAIL_CALL_NO_RETURN (__syscall_do_cancel