Re: [patch V6 01/11] rseq: Add fields and constants for time slice extension

[Florian Weimer <fweimer@redhat.com>]

* Thomas Gleixner:

> I'm not completely opposed to make it process wide. For threads created
> after enablement, that's trivial because that can be done when the per
> thread RSEQ is registered. But when it gets enabled _after_ threads have
> been created already then we need code to chase the threads and enable
> it after the fact because we are not going to query the enablement in
> curr->mm::whatever just to have another conditional and another
> cacheline to access.

In glibc, we make sure that the registration for restartable sequences
happens before any user code (with the exception of IFUNC resolvers) can
run.  This includes code from signal handlers.  We started masking
signals on newly created threads for this reason, to make these
partially initialized states unobservable.

It's not clear to me what the expected outcome is.  If we ever want to
offer deadline extension as a mutex attribute (for example), then we
have to switch this on at process start unconditionally because we don't
know if this new API will be used by the new process (potentially after
dlopen, so we can't even use things likely analyzing the symbol
footprint ahead of time).

> The only option is to reject enablement when there is already more than
> one thread in the process, but there is a reasonable argument that a
> process might only enable it for a subset of threads, which have actual
> lock interaction and not bother with it for other things. I'm not seeing
> a reason to restrict the flexibility of configuration just because you
> envision magic use cases all over the place.

Sure, but it looks like this needs a custom/minimal libc.  It's like
repurposing set_robust_list for something else.  It can be done, but it
has a significant cost in terms of compatibility because some
functionality (that other libraries in the process depend on) will stop
working.

> On the other hand there is no guarantee that libc registers RSEQ when a
> thread is started as it can be disabled or not supported, so you have
> exactly the same problem there that the code which wants to use it needs
> to ensure that a RSEQ area is registered, no?

With glibc, if RSEQ is registered on the main thread, it will be
registered on all other threads, too.  Technically, it's possible to
unregister RSEQ with the kernel, of course, but that's totally
undefined, like unmapping memory originally returned from malloc.

>>> The prctl allows you to query the state, so all parties can make
>>> informed decisions. It's not any different from other mechanisms, which
>>> require coordination between different parts.
>>
>> I'm fine with having prctl enable the feature (for the whole process)
>> and query its state.
>>
>> The part I'm concerned with is the prctl disabling the feature, as
>> we're losing the availability invariant after setup.
>
>   close(0);
>
> has the same problem. How many instances of bugs in that area have you
> seen so far?

We've had significant issues due to incorrect close calls (maybe not
close(0) in particular, but definitely with double-closes removing
descriptors created by other threads.

We need the prctl to unregister for CRIU, though, otherwise CRIU won't
be able to use glibc directly (or would have to re-exec itself in a new
configuration).

Thanks,
Florian