Re: [PATCH v2 0/8] Extensible rseq integration

[Florian Weimer <fweimer@redhat.com>]

* Rich Felker:

> On Tue, Dec 07, 2021 at 01:59:26PM +0100, Florian Weimer via Libc-alpha wrote:
>> This series integrates the previous posted v2 for <thread_pointer.h>.
>> 
>> It incorporates Mathieu's and Paul E. McKenney suggestion to use a
>> volatile read for rseq_abi.cpu_id access, using a new
>> THREAD_GETMEM_VOLATILE macro.
>> 
>> The last patch in the series makes rseq registration consistent across
>> threads.
>> 
>> Florian Weimer (8):
>>   nptl: Add <thread_pointer.h> for defining __thread_pointer
>>   nptl: Introduce <tcb-access.h> for THREAD_* accessors
>>   nptl: Introduce THREAD_GETMEM_VOLATILE
>>   nptl: Add rseq registration
>>   Linux: Use rseq to accelerate sched_getcpu
>>   nptl: Add glibc.pthread.rseq tunable to control rseq registration
>>   nptl: Add public rseq symbols and <sys/rseq.h>
>>   nptl: rseq failure after registration on main thread is fatal
>
> I'm sorry for bringing this up so late; I wasn't aware that redesign
> of the rseq ABI was taking place. I wish this had been discussed in a
> cross-libc venue, since, in its current form, I don't think the ABI is
> suitable for inclusion in, or use as a third-party library with, musl.

Well, I Cc:ed you on the original proposal in November, and cross-posted
it to linux-api as well.

> The most pressing issue I see is that it does not admit lazy
> registration, which precludes it being implemented outside of libc
> (because it has to hook into pthread_create) and imposes runtime cost
> on programs which do not use it. RSEQ_CPU_ID_UNINITIALIZED exists to
> inform the application about an uninitialized state, but the
> application has no way to request an attempt at registration upon
> seeing it. I think that would be easy to add. Basically it's just
> making the syscall, which a consumer of the ABI could in theory do
> itself, but it's probably best not to have it do that and instead have
> registration mediated through the ABI/through libc.

I rejected that because the programming model is too complex: In the
extreme, a library that observes rseq support on the main thread may be
called again from another thread where rseq is not yet enabled, and
cannot be enabled.

I think it is also necessary to enable it unconditionally to force
people to actually implement support for it in their tools (e.g., CRIU).
Otherwise we'll never get to the point where it is reliable.  I doubt
we'd have learned about the CRIU issue by now unless we took that step.

> Related to this, if rseq is implemented outside of libc, I'm not sure
> if there's a safe way to ensure it's unregistered prior to thread
> exit. It may already be possible but I haven't sufficiently convinced
> myself.

I expect that asking for rseq to be implemented outside of libc is like
asking for robust mutexes to be implemented outside libc: it's really
pushing what can be done in a library.

> On another issue, while this isn't entirely a show-stopper, I'm not a
> fan of requiring constant __rseq_offset. This comes across as an
> instance-specific hack to make up for GD TLS being slow, when we
> already have a fully general solution to that which isn't being
> deployed: TLSDESC. As it stands in the current ABI, whatever library
> is providing rseq must be present at application startup; it can't be
> dlopened. And due to the ABI this applies *even if* we just wanted to
> make rseq always-fail in that case. The ABI simply doesn't admit not
> having memory pre-reserved for every thread (note: the size is
> something like a +30% increase to musl's per-thread memory usage and
> will surely increase over time, which is a lot for something we don't
> expect the vast majority of applications to use).

If the memory is not allocated, __rseq_size can be set to 0.

> One minor and hopefully non-controversial declared-ABI issue I see is
> that the __rseq_offset etc. objects are declared const, with a
> pre-relro access hack used to modify them at runtime. This is
> incompatible with LTO and static linking. If protecting them is
> desired, they should be declared non-const but live in non-modifiable
> memory, like string literals do. Otherwise a static linking LTO
> compiler is free to copy the initial values directly into code.

Yes, you'll need a compiler barrier with LTO.  It's not different from
other types of relocations.

> I'm not sure what the right thing to do on the verge of release is. If
> it were my choice, I would hold it back and wait until it was better
> reviewed and these issues worked out before making it public API/ABI,
> but I don't know what glibc's constraints here are and how to best
> weigh them against the ability to revise this ABI after release. Most
> of these things I think *are* of the sort that can be fixed in
> non-breaking ways, except that applications written to the current
> version might need to adjust before they can use a version of the
> API/ABI we'd be willing to adopt in musl.

Quoting for Mathieu's benefit.  Also Cc:ing Carlos as the release
manager.

Thanks,
Florian