Re: [PATCH] selftests/rseq: Fix handling of glibc without rseq support

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2025-01-15 12:57, Shuah Khan wrote:
> On 1/14/25 17:45, Mathieu Desnoyers wrote:
>> On 2025-01-14 19:14, Shuah Khan wrote:
>>> On 1/14/25 07:51, Mathieu Desnoyers wrote:
>>>> When porting librseq commit:
>>>>
>>>> commit c7b45750fa85 ("Adapt to glibc __rseq_size feature detection")
>>>>
>>>> from librseq to the kernel selftests, the following line was missed
>>>> at the end of rseq_init():
>>>>
>>>>    rseq_size = get_rseq_kernel_feature_size();
>>>>
>>>> which effectively leaves rseq_size initialized to -1U when glibc 
>>>> does not
>>>> have rseq support. glibc supports rseq from version 2.35 onwards.
>>>>
>>>> In a following librseq commit
>>>>
>>>> commit c67d198627c2 ("Only set 'rseq_size' on first thread 
>>>> registration")
>>>>
>>>> to mimic the libc behavior, a new approach is taken: don't set the
>>>> feature size in 'rseq_size' until at least one thread has successfully
>>>> registered. This allows using 'rseq_size' in fast-paths to test for 
>>>> both
>>>> registration status and available features. The caveat is that on libc
>>>> either all threads are registered or none are, while with bare librseq
>>>> it is the responsability of the user to register all threads using 
>>>> rseq.
>>>>
>>>> This combines the changes from the following librseq commits:
>>>>
>>>> commit c7b45750fa85 ("Adapt to glibc __rseq_size feature detection")
>>>> commit c67d198627c2 ("Only set 'rseq_size' on first thread 
>>>> registration")
>>>>
>>>> Fixes: 73a4f5a704a2 ("selftests/rseq: Fix mm_cid test failure")
> 
> Fixed this commit id
> commit c7b45750fa85 ("Adapt to glibc __rseq_size feature detection")

Just pointing out that you did the right thing in the commit that ends
up in linux-kselftest next:

https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=336d02bc4c6bec5c3d933e5d470a94970f830957

Fixes: a0cc649353bb ("selftests/rseq: Fix mm_cid test failure")

Which is fine.

Thanks!

Mathieu

> 
>>>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>>
>>> Hi Mathieu,
>>>
>>> Can you double check these commits and make sure these are right
>>> ones in the mainline rc7?
>>>
>>> I am seeing "Unknown commit id" warnings on all of these - my
>>> repo is at 6.13 rc7
>>
>> This is because those are commits in the librseq project at
>> https://git.kernel.org/pub/scm/libs/librseq/librseq.git/
>> which is a different tree from the Linux kernel. I am not
>> sure what is the preferred approach when citing a
>> commit ID from a different project ?
>>
>> I've been keeping both rseq selftests and librseq in
>> sync since 2018. I plan to eventually add a dependency
>> of the rseq selftests on librseq, but this cannot
>> happen until we freeze the API and cut a librseq
>> release.
>>
>> This was premature before we reached the major milestone
>> of having extensible rseq support in glibc.
>>
>> Now that it's merged into glibc (as of last week),
>> we can start looking forward to a librseq release,
>> which should eventually eliminate code duplication
>> with rseq selftests.
>>
>> Perhaps we should add a Link: to the librseq
>> repository ?
>>
>>>
>>> Also would you like to add Reported-by for Raghavendra Rao Ananta?
> 
> Added. The patch is now in linux-kselftest next
> 
> thanks,
> -- Shuah

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com