----- On Oct 11, 2018, at 12:20 PM, Szabolcs Nagy Szabolcs.Nagy@arm.com wrote:

On 11/10/18 16:13, Mathieu Desnoyers wrote:

quoted

----- On Oct 11, 2018, at 6:37 AM, Szabolcs Nagy Szabolcs.Nagy@arm.com wrote:

quoted

On 10/10/18 20:19, Mathieu Desnoyers wrote:

quoted

In order to integrate rseq into user-space applications, add a reference
counter field after the struct rseq TLS ABI so many rseq users can be
linked into the same application (e.g. librseq and glibc). The
reference count ensures that rseq syscall registration/unregistration
happens only for the most early/late user for each thread, thus ensuring
that rseq is registered across the lifetime of all rseq users for a
given thread.

...

quoted

+__attribute__((visibility("hidden"))) __thread
+volatile struct libc_rseq __lib_rseq_abi = {

...

quoted

+extern __attribute__((weak, alias("__lib_rseq_abi"))) __thread
+volatile struct rseq __rseq_abi;

...

quoted

@@ -70,7 +86,7 @@ int rseq_register_current_thread(void)
 	sigset_t oldset;
 
 	signal_off_save(&oldset);
-	if (refcount++)
+	if (__lib_rseq_abi.refcount++)
 		goto end;
 	rc = sys_rseq(&__rseq_abi, sizeof(struct rseq), 0, RSEQ_SIG);

why do you use a local refcounter instead of the __rseq_abi one?

There is no refcount in struct rseq (the ABI between kernel and user-space).
The registration refcount was part of an earlier version of the rseq system
call,
but we decided against keeping it in the kernel.

So I'm adding one _after_ struct rseq, purely to allow interaction between
various user-space components (program/libraries).

then all those components must use the same

 rseq_register_current_thread
 rseq_unregister_current_thread

functions and not call the syscall on their own.

Not quite. Each user (programs and shared objects) must handle the refcount in a
similar way if they wish to invoke the syscall by themselves. They can
alternately use the librseq APIs if they do not wish to have a local implementation
of the reference counting and syscall registration/unregistration.

in which case the refcount could be a static __thread variable.

Yes, but I want to limit the number of symbols we need to export
from glibc by appending the refcount field at the end of struct rseq.

but it's in a magic struct that's called "abi" which is confusing,
the counter is not abi, it's in a hidden object.

No, it is really an ABI between user-space apps/libs. It's not meant to be
hidden. glibc implements its own register/unregister functions (it does not
link against librseq). librseq exposes register/unregister functions as public
APIs. Those also use the refcount. I also plan to have existing libraries, e.g.
liblttng-ust and possibly liburcu flavors, implement the
registration/unregistration and refcount handling on their own, so we don't
have to add a requirement on additional linking on librseq for pre-existing
libraries.

So that refcount is not an ABI between kernel and user-space, but it's a
user-space ABI nevertheless (between program and shared objects).

quoted

quoted

what prevents calling rseq_register_current_thread more than 4G times?

Nothing. It would indeed be cleaner to error out if we detect that refcount is
at
INT_MAX. Is that what you have in mind ?

yes

Allright, will fix.

quoted

quoted

why cant the kernel see that the same address is registered again and succeed?

It can, and it does. However, refcounting at user-level is needed to ensure
the registration "lifetime" for rseq covers its entire use. If we have two
libraries
using rseq, we end up with the following scenario:

Thread 1

  libA registers rseq
  libB registers rseq
  libB unregisters rseq
  libA uses rseq -> bug! it's been unregistered by libB.
  libA unregisters rseq -> unexpected, it's already been unregistered.
 
same applies if libA unregisters rseq before libB (and libB try to use rseq
after libA has unregistered).

The refcount in user-space fixes this.

i see.

Thanks for the feedback!

Mathieu

quoted

Thoughts ?

Thanks,

Mathieu

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