Re: [PATCH 5/5] nptl: Add public rseq symbols and <sys/rseq.h>

[Florian Weimer <fweimer@redhat.com>]

* Szabolcs Nagy via Libc-alpha:

>> +One use of the restartable sequences area is to read the current CPU
>> +number from its @code{cpu_id} field, as an inline version of
>> +@code{sched_getcpu}.  @Theglibc{} sets the @code{cpu_id} field to
>> +@code{RSEQ_CPU_ID_REGISTRATION_FAILED} if registration failed or was
>> +explicitly disabled.
>
> is it better to use RSEQ_CPU_ID_UNINITIALIZED for the disabled case?
>
> so code that uses rseq itself can distinguish enabled but failed rseq
> and disabled rseq (in the latter case it would try to register, in the
> former it would assume rseq is not supported).

I think RSEQ_CPU_ID_UNINITIALIZED was previously intended for a scenario
that is just not observable at all in the current glibc implementation,
basically an initialization ordering issue.

My expectation is that an application that knows about glibc's rseq
implementation will not want to use rseq if it has been explicitly
disabled.  That tunable is only for getting old rseq users to work
again.  If the application goes behind glibc's back and starts rseq
using again, it defeats the tunable (because there could be another rseq
user in the process image).

>> +@deftypevar {int} __rseq_offset
>> +@standards{Linux, sys/rseq.h}
>> +This variable contains the offset between the thread pointer (as defined
>> +by @code{__builtin_thread_pointer} or the thread pointer register for
>> +the architecture) and the restartable sequences area.  This value is the
>> +same for all threads in the process.  If the restartable sequences area
>> +is located at a lower address than the location to whic the  thread
>
> s/whic/which/
>
>> +pointer points, the value is negative.
>> +@end deftypevar
>> +
>> +@deftypevar {int} __rseq_size
>
> declaration below uses unsigned int.
>
>> +@standards{Linux, sys/rseq.h}
>> +This variable is either zero (if restartable sequence registration
>> +failed or has been disabled) or the size of the restartable sequence
>> +registration.  This can be less can be different from the size of
>
> s/can be less//
>
>> +@code{struct rseq} if the kernel has extended the size of the
>> +registration.  If registration is successful, @code{__rseq_size} is at
>> +least 32 (the initial size of @code{struct rseq}.
>
> missing )

All fixed, thanks.

>> +@end deftypevar
>> +
>> +@deftypevar {unsigned int} __rseq_flags
>> +@standards{Linux, sys/rseq.h}
>> +The flags used during restartable sequence registration with the kernel.
>
> the syscall argument is documented to be int and our header
> declares it as enum rseq_flags, not sure if this matters.

It shouldn't matter, unsigned int seems more conservative.  I'm always a
bit wary about enums and bitmasks.

Thanks,
Florian