Re: [PATCH for 5.2 00/12] Restartable Sequences selftests updates

Mathieu Desnoyers
2019-05-03T19:22:37Z

----- On May 3, 2019, at 2:53 PM, shuah shuah@kernel.org wrote:

> On 5/3/19 12:36 PM, Mathieu Desnoyers wrote:
>> ----- On Apr 29, 2019, at 11:27 AM, Mathieu Desnoyers
>> mathieu.desnoyers@efficios.com wrote:
>>
>>> Those rseq selftests updates are hereby submitted to Shuah Khan,
>>> maintainer of kernel selftests, for the next merge window (5.2).
>>>
>>> They change the per-architecture pre-abort signatures to ensure those
>>> are valid trap instructions.
>>>
>>> The way exit points are presented to debuggers is enhanced, ensuring
>>> all exit points are present, so debuggers don't have to disassemble
>>> rseq critical section to properly skip over them.
>>>
>>> Discussions with the glibc community is reaching a concensus of exposing
>>> a __rseq_handled symbol from glibc to coexist with rseq early adopters.
>>> Update the rseq selftest code to expose and use this symbol.
>>>
>>> Support for compiling asm goto with clang is added with the
>>> "-no-integrated-as" compiler switch, similarly to the toplevel kernel
>>> Makefile.
>>
>> Hi Shuah,
>>
>> Is there anything else you need before you can pick up those patches ?
>>
>
> I was going to say "no more work needed" and noticed that the series has
> checkpatch errors and warns as I was running the series through
> pre-commit tests.
>
> Patches 1,2,3,8 have errors/warns based
> on quick look at the log.
>
>
> ERROR: need consistent spacing around '%' (ctx:WxV)
> #227: FILE: tools/testing/selftests/rseq/rseq-x86.h:104:
> + RSEQ_ASM_CMP_CPU_ID(cpu_id, RSEQ_CPU_ID_OFFSET(%[rseq_abi]), %l[error1])
>
>
> Will you be able to fix them and resend?

(CCing the che checkpatch maintainers)

checkpatch appears to be wrong for these errors. I suspect it thinks those are
'%' modulo operators (for which the style requires space before/after),
but those are actually part of the asm input and goto target operands.

Most warnings are about some lines over 80 cols. However, the areas where
this happens is due to following the style of already upstream code which
has the final "\" at the end of line sometimes beyond 80 col to accommodate
macros that take a bit of horizontal real estate.

For patch 8, the warning about "availble" being a typo is right. The
style error about space after "asm (" is right as well. Should I send only
this updated patch to you or should I send the whole patchset again ?

Thanks,

Mathieu

>
> thanks,
> -- Shuah

--
Mathieu Desnoyers
EfficiOS Inc.