Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1455804 > unrolled thread

Re: [RFC PATCH v7 1/7] Restartable sequences system call

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-03 16:00 +0200
    Re: [RFC PATCH v7 1/7] Restartable sequences system call "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-08-03 17:00 +0200
    Re: [RFC PATCH v7 1/7] Restartable sequences system call Boqun Feng <boqun.feng@gmail.com> - 2016-08-03 17:50 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-07 17:40 +0200
        Re: [RFC PATCH v7 1/7] Restartable sequences system call Boqun Feng <boqun.feng@gmail.com> - 2016-08-08 01:40 +0200
          Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-09 15:30 +0200
    Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-09 22:10 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-09 23:40 +0200
        Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 00:50 +0200
          Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 21:00 +0200
            Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 21:00 +0200
            Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-10 21:20 +0200
          Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-10 22:10 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 20:20 +0200
        Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 20:30 +0200
        Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-10 21:20 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 21:10 +0200
        Re: [RFC PATCH v7 1/7] Restartable sequences system call Andy Lutomirski <luto@amacapital.net> - 2016-08-10 21:20 +0200
          Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 22:10 +0200
            Re: [RFC PATCH v7 1/7] Restartable sequences system call Andy Lutomirski <luto@amacapital.net> - 2016-08-10 22:20 +0200
              Re: [RFC PATCH v7 1/7] Restartable sequences system call Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-08-10 23:10 +0200
                Re: [RFC PATCH v7 1/7] Restartable sequences system call Andy Lutomirski <luto@amacapital.net> - 2016-08-11 09:30 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Andy Lutomirski <luto@amacapital.net> - 2016-08-10 21:20 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-10 22:10 +0200
      Re: [RFC PATCH v7 1/7] Restartable sequences system call Peter Zijlstra <peterz@infradead.org> - 2016-08-10 22:10 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1459939

----- On Aug 10, 2016, at 4:09 PM, Andy Lutomirski luto@amacapital.net wrote:

> On Wed, Aug 10, 2016 at 1:06 PM, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

<snip>

>>> u64 is a perfectly valid, if odd, userspace pointer on all
>>> architecures that I know of, and it's certainly a valid userspace
>>> pointer on x86 32-bit userspace (the high bits will just all be zero).
>>> Can you just use u64?
>>
>> My concern is about a 32-bit user-space putting garbage rather than zeroes
>> (on purpose) to fool the kernel on those upper 32 bits. Doing
>>
>>   compat_ptr((compat_uptr_t)rseq_cs.start_ip)
>>
>> effectively ends up clearing the upper 32 bits.
>>
>> But since we only use those pointer values for comparisons, perhaps we
>> just don't care if a 32-bit userspace app try to shoot itself in
>> the foot by passing garbage upper 32 bits ?
>>
> 
> How is garbage in the high bits any different than garbage in any
> other bits in there?

It's not :)

> 
>>
>>> If this would be a performance problem on ARM, then maybe that's a
>>> reason to use compat helpers.
>>
>> We already use 64-bit values for the pointers, even on 32-bit. Normally
>> userspace just puts zeroes in the top bits. It's mostly a question of
>> clearing the top 32 bits or not when loading them in the kernel. If we
>> don't need to, then I can remove the compat code entirely, and we don't
>> care about user_64bit_mode() anymore, as you initially recommended.
>> Does it make sense ?
> 
> Yes, I think so.  I'd suggest just honoring all the bits.

OK, will do !

> 
>>
>>>
>>>>
>>>>>
>>>>>
>>>>>>>> +SYSCALL_DEFINE2(rseq, struct rseq __user *, rseq, int, flags)
>>>>>>>> +{
>>>>>>>> +    if (unlikely(flags))
>>>>>>>> +            return -EINVAL;
>>>>>>>
>>>>>>> (add whitespace)
>>>>>>
>>>>>> fixed.
>>>>>>
>>>>>>>
>>>>>>>> +    if (!rseq) {
>>>>>>>> +            if (!current->rseq)
>>>>>>>> +                    return -ENOENT;
>>>>>>>> +            return 0;
>>>>>>>> +    }
>>>>>
>>>>> This looks entirely wrong.  Setting rseq to NULL fails if it's already
>>>>> NULL but silently does nothing if rseq is already set?  Surely it
>>>>> should always succeed and it should actually do something if rseq is
>>>>> set.
>>>>
>>>> From the proposed rseq(2) manpage:
>>>>
>>>> "A NULL rseq value can be used to check whether rseq is registered
>>>> for the current thread."
>>>>
>>>> The implementation does just that: it returns -1, errno=ENOENT if no
>>>> rseq is currently registered, or 0 if rseq is currently registered.
>>>
>>> I think that's problematic.  Why can't you unregister an existing
>>> rseq?  If you can't, how is a thread supposed to clean up after
>>> itself?
>>>
>>
>> Unregistering an existing thread rseq would require that we keep reference
>> counting, in case multiple libs and/or the app are using rseq. I am
>> trying to keep things as simple as needed.
>>
>> If I understand your concern, the problematic scenario would be at
>> thread exit (this is my current approximate understanding of glibc
>> handling of library TLS variable reclaim at thread exit):
>>
>> thread exits in userspace:
>> - glibc frees its rseq TLS memory area (in case the TLS is in a library),
>> - thread preempted before really exiting,
>> - kernel reads/writes to freed TLS memory.
>>   - corruption may occur (e.g. memory re-allocated by another thread already)
>>
>> Am I getting it right ?
> 
> Yes.

Hrm, then we should:

- add a rseq_refcount field to the task struct,
- increment this refcount whenever rseq receives a registration, after
  ensuring that we are registering the same address as was previously
  requested by preceding registrations for the thread (except if the
  refcount was 0),
- When rseq receives a NULL address, decrement refcount. Set address to
  NULL when it reaches 0.

Doing the refcounting in kernel-space rather than user-space allows us to
keep both registration/unregistration and refcount atomic, which simplify
things if we plan to use rseq from signal handlers.

With current glibc, a library that would lazily register and use rseq
without knowledge of the application would then have to use pthread_key_create()
to set a destr_function to run at thread exit, which would take care of
unregistration.

We could add a RSEQ_FORCE_UNREGISTER flag to rseq flags to allow future
glibc versions to force unregistering rseq before freeing its TLS memory,
just in case a userspace library omits to unregister itself.

Thoughts ?

Thanks,

Mathieu


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

[toc] | [prev] | [next] | [standalone]

#1460215

On Aug 11, 2016 12:01 AM, "Mathieu Desnoyers"
<mathieu.desnoyers@efficios.com> wrote:
>
> ----- On Aug 10, 2016, at 4:09 PM, Andy Lutomirski luto@amacapital.net wrote:
>
> > On Wed, Aug 10, 2016 at 1:06 PM, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
> <snip>
>
> >>> u64 is a perfectly valid, if odd, userspace pointer on all
> >>> architecures that I know of, and it's certainly a valid userspace
> >>> pointer on x86 32-bit userspace (the high bits will just all be zero).
> >>> Can you just use u64?
> >>
> >> My concern is about a 32-bit user-space putting garbage rather than zeroes
> >> (on purpose) to fool the kernel on those upper 32 bits. Doing
> >>
> >>   compat_ptr((compat_uptr_t)rseq_cs.start_ip)
> >>
> >> effectively ends up clearing the upper 32 bits.
> >>
> >> But since we only use those pointer values for comparisons, perhaps we
> >> just don't care if a 32-bit userspace app try to shoot itself in
> >> the foot by passing garbage upper 32 bits ?
> >>
> >
> > How is garbage in the high bits any different than garbage in any
> > other bits in there?
>
> It's not :)
>
> >
> >>
> >>> If this would be a performance problem on ARM, then maybe that's a
> >>> reason to use compat helpers.
> >>
> >> We already use 64-bit values for the pointers, even on 32-bit. Normally
> >> userspace just puts zeroes in the top bits. It's mostly a question of
> >> clearing the top 32 bits or not when loading them in the kernel. If we
> >> don't need to, then I can remove the compat code entirely, and we don't
> >> care about user_64bit_mode() anymore, as you initially recommended.
> >> Does it make sense ?
> >
> > Yes, I think so.  I'd suggest just honoring all the bits.
>
> OK, will do !
>
> >
> >>
> >>>
> >>>>
> >>>>>
> >>>>>
> >>>>>>>> +SYSCALL_DEFINE2(rseq, struct rseq __user *, rseq, int, flags)
> >>>>>>>> +{
> >>>>>>>> +    if (unlikely(flags))
> >>>>>>>> +            return -EINVAL;
> >>>>>>>
> >>>>>>> (add whitespace)
> >>>>>>
> >>>>>> fixed.
> >>>>>>
> >>>>>>>
> >>>>>>>> +    if (!rseq) {
> >>>>>>>> +            if (!current->rseq)
> >>>>>>>> +                    return -ENOENT;
> >>>>>>>> +            return 0;
> >>>>>>>> +    }
> >>>>>
> >>>>> This looks entirely wrong.  Setting rseq to NULL fails if it's already
> >>>>> NULL but silently does nothing if rseq is already set?  Surely it
> >>>>> should always succeed and it should actually do something if rseq is
> >>>>> set.
> >>>>
> >>>> From the proposed rseq(2) manpage:
> >>>>
> >>>> "A NULL rseq value can be used to check whether rseq is registered
> >>>> for the current thread."
> >>>>
> >>>> The implementation does just that: it returns -1, errno=ENOENT if no
> >>>> rseq is currently registered, or 0 if rseq is currently registered.
> >>>
> >>> I think that's problematic.  Why can't you unregister an existing
> >>> rseq?  If you can't, how is a thread supposed to clean up after
> >>> itself?
> >>>
> >>
> >> Unregistering an existing thread rseq would require that we keep reference
> >> counting, in case multiple libs and/or the app are using rseq. I am
> >> trying to keep things as simple as needed.
> >>
> >> If I understand your concern, the problematic scenario would be at
> >> thread exit (this is my current approximate understanding of glibc
> >> handling of library TLS variable reclaim at thread exit):
> >>
> >> thread exits in userspace:
> >> - glibc frees its rseq TLS memory area (in case the TLS is in a library),
> >> - thread preempted before really exiting,
> >> - kernel reads/writes to freed TLS memory.
> >>   - corruption may occur (e.g. memory re-allocated by another thread already)
> >>
> >> Am I getting it right ?
> >
> > Yes.
>
> Hrm, then we should:
>
> - add a rseq_refcount field to the task struct,
> - increment this refcount whenever rseq receives a registration, after
>   ensuring that we are registering the same address as was previously
>   requested by preceding registrations for the thread (except if the
>   refcount was 0),
> - When rseq receives a NULL address, decrement refcount. Set address to
>   NULL when it reaches 0.
>
> Doing the refcounting in kernel-space rather than user-space allows us to
> keep both registration/unregistration and refcount atomic, which simplify
> things if we plan to use rseq from signal handlers.
>
> With current glibc, a library that would lazily register and use rseq
> without knowledge of the application would then have to use pthread_key_create()
> to set a destr_function to run at thread exit, which would take care of
> unregistration.

That sounds reasonable at first glance.

>
> We could add a RSEQ_FORCE_UNREGISTER flag to rseq flags to allow future
> glibc versions to force unregistering rseq before freeing its TLS memory,
> just in case a userspace library omits to unregister itself.

Sounds good too.

[toc] | [prev] | [next] | [standalone]

#1459632

On Tue, Aug 9, 2016 at 1:06 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> ----- On Aug 3, 2016, at 9:19 AM, Peter Zijlstra peterz@infradead.org wrote:
>

>>
>>> +#endif
>>>  /* CPU-specific state of this task */
>>>      struct thread_struct thread;
>>>  /*
>>> @@ -3387,4 +3392,67 @@ void cpufreq_add_update_util_hook(int cpu, struct
>>> update_util_data *data,
>>>  void cpufreq_remove_update_util_hook(int cpu);
>>>  #endif /* CONFIG_CPU_FREQ */
>>>
>>> +#ifdef CONFIG_RSEQ
>>> +static inline void rseq_set_notify_resume(struct task_struct *t)
>>> +{
>>> +    if (t->rseq)
>>> +            set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
>>> +}
>>
>> Maybe I missed it, but why do we want to hook into NOTIFY_RESUME and not
>> have our own TIF flag?
>
> The short answer is that used the same approach as Paul Turner's patchset. ;)
>
> Through a deeper look into this, the only times we set the flag is when
> preempting and delivering a signal to a thread that has registered to
> rseq.
>
> Upon return to user-space with the flag set, the performance difference
> between having our own flag and hopping into the NOTIFY_RESUME bandwagon
> is that we can skip the various tests in exit_to_usermode_loop()
> with our own flag, at the expense of crowding the thread flags even
> nearer to filling up 32 bits, which will at some point require extra
> tests on the fast-path.

I don't think we're anywhere near running out.  Several of those flags
can probably go away pretty easily, too.

>
> Thinking about it, one benchmark I have not done so far is to modify
> hackbench so it registers its threads with the rseq system call. We
> can then figure out whether reserving a flag for rseq is justified or
> not.
>
> Comparing 10 runs of hackbench registering its sender/receiver threads
> with unmodified hackbench: (hackbench -l 100000)
>
>     Configuration: 2 sockets * 8-core Intel(R) Xeon(R) CPU E5-2630 v3 @
>     2.40GHz (directly on hardware, hyperthreading disabled in BIOS, energy
>     saving disabled in BIOS, turboboost disabled in BIOS, cpuidle.off=1
>     kernel parameter), with a Linux v4.7 defconfig+localyesconfig,
>     restartable sequences series applied.
>
>                                     Avg. Time (s)    Std.dev. (s)
> Unmodified Hackbench                    40.5            0.1
> Rseq-Registered Hackbench Threads       40.4            0.1
>
> So initial results seems to indicate that adding the notify_resume
> handling upon preemption does not have noticeable effects on
> performance, so I don't consider it worthwhile to try optimizing
> it by reserving its own thread flag. Or perhaps am I missing something
> important here ?
>

I don't think so.  One benefit of using do_notify_resume would be less
arch code.

> Actually, we want copy_from_user() there. This executes upon
> resume to user-space, so we can take a page fault is needed, so
> no "inatomic" needed. I therefore suggest:

Running the code below via exit_to_usermode_loop...

>
> static bool rseq_get_rseq_cs(struct task_struct *t,
>                 void __user **start_ip,
>                 void __user **post_commit_ip,
>                 void __user **abort_ip)
> {
>         unsigned long ptr;
>         struct rseq_cs __user *urseq_cs;
>         struct rseq_cs rseq_cs;
>
>         if (__get_user(ptr, &t->rseq->rseq_cs))
>                 return false;
>         if (!ptr)
>                 return true;
> #ifdef CONFIG_COMPAT
>         if (in_compat_syscall()) {
>                 urseq_cs = compat_ptr((compat_uptr_t)ptr);
>                 if (copy_from_user(&rseq_cs, urseq_cs, sizeof(*rseq_cs)))
>                         return false;
>                 *start_ip = compat_ptr((compat_uptr_t)rseq_cs.start_ip);
>                 *post_commit_ip = compat_ptr((compat_uptr_t)rseq_cs.post_commit_ip);
>                 *abort_ip = compat_ptr((compat_uptr_t)rseq_cs.abort_ip);
>                 return true;
>         }
> #endif

...means that in_compat_syscall() is nonsense.  (It *works* there, but
I can't imagine that it does anything that is actually sensible for
this use.)

Can't you just define the ABI so that no compat junk is needed?
(Also, CRIU will thank you for doing that.)


>>> +SYSCALL_DEFINE2(rseq, struct rseq __user *, rseq, int, flags)
>>> +{
>>> +    if (unlikely(flags))
>>> +            return -EINVAL;
>>
>> (add whitespace)
>
> fixed.
>
>>
>>> +    if (!rseq) {
>>> +            if (!current->rseq)
>>> +                    return -ENOENT;
>>> +            return 0;
>>> +    }

This looks entirely wrong.  Setting rseq to NULL fails if it's already
NULL but silently does nothing if rseq is already set?  Surely it
should always succeed and it should actually do something if rseq is
set.


-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1459770

On Tue, Aug 09, 2016 at 08:06:40PM +0000, Mathieu Desnoyers wrote:
> On Aug 3, 2016, at 9:19 AM, Peter Zijlstra peterz@infradead.org wrote:

> >> +++ b/kernel/sched/core.c
> >> @@ -2664,6 +2664,7 @@ prepare_task_switch(struct rq *rq, struct task_struct
> >> *prev,
> >>  {
> >>  	sched_info_switch(rq, prev, next);
> >>  	perf_event_task_sched_out(prev, next);
> >> +	rseq_sched_out(prev);
> > 
> > One thing I considered is doing something like:
> > 
> > static inline void rseq_sched_out(struct task_struct *t)
> > {
> >	unsigned long ptr;
> >	int err;
> > 
> >	if (!t->rseq)
> >		return;
> > 
> >	err = __get_user(ptr, &t->rseq->rseq_cs);
> >	if (err || ptr)
> >		set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
> > }
> > 
> > That will optimistically try to read the rseq_cs pointer and, on success
> > and empty (the most likely case) avoid setting the TIF flag.
> > 
> > This will require an explicit migration hook to unconditionally set the
> > TIF flag such that we keep the cpu_id field correct of course.
> > 
> > And obviously we can do this later, as an optimization. Its just
> > something I figured might be worth it.
> 
> This won't work. The rseq mechanism proposed here is really the overlap
> of _two_ distinct restart mechanisms: a sequence counter for C code,
> and a ip-fixup-based mechanism for the assembly "finish" instruction
> sequence.
> 
> What you propose here only considers the fixup of the assembly instruction
> sequence, but not the C code that runs before. The C code between
> rseq_start() and rseq_finish() loads the current value of the sequence
> counter in rseq_start(), and then it gets compared with the new current
> value within the rseq_finish restartable sequence of instructions. So the
> sequence counter needs to be updated upon preemption/signal delivery that
> occurs on top of C code, even if not nesting over a sequence of
> restartable assembly instructions.

True; we could of course have the rseq_start() also set a !0 state
before reading the seq, but not sure that all is worth it.

[toc] | [prev] | [next] | [standalone]

#1459771

On Tue, Aug 09, 2016 at 08:06:40PM +0000, Mathieu Desnoyers wrote:
> > Also, do we want a comment somewhere that explains why overflow isn't a
> > problem?
> 
> I can add a comment about rseq_increment_event_counter stating:
> 
>  * Overflow of the event counter is not a problem in practice. It
>  * increments at most once between each user-space thread instruction
>  * executed, so we would need a thread to execute 2^32 instructions or
>  * more between rseq_start() and rseq_finish(), while single-stepping,
>  * for this to be an issue.
> 
> Is it fine, or should we be more conservative and care about the overflow,
> extending the counter to a 64-bit value in the process ?

I think its good enough; and using u64 has the unfortunate side effect
of not being able to share the word with the cpu number.

My point was more to have this stuff clearly documented.

> > Maybe I missed it, but why do we want to hook into NOTIFY_RESUME and not
> > have our own TIF flag?
> 
> The short answer is that used the same approach as Paul Turner's patchset. ;)
> 
> Through a deeper look into this, the only times we set the flag is when
> preempting and delivering a signal to a thread that has registered to
> rseq.

 <snip>

> So initial results seems to indicate that adding the notify_resume
> handling upon preemption does not have noticeable effects on
> performance, so I don't consider it worthwhile to try optimizing
> it by reserving its own thread flag. Or perhaps am I missing something
> important here ?

Not sure; seems like we can leave it as is for the moment. Again my
point was to make sure we've thought about the decision, and per the
above you clearly have now ;-)

> > Also, I think it would be good to have a comment explaining why this is
> > split in two structures? Don't you rely on the address dependency?
> 
> The comment above the rseq_cs fields needs clarification, how about:
> 
>         /*
>          * Restartable sequences rseq_cs field.
>          * Contains NULL when no critical section is active for the
>          * current thread, or holds a pointer to the currently active
>          * struct rseq_cs.
>          * Updated by user-space at the beginning and end of assembly
>          * instruction sequence block, and by the kernel when it
>          * restarts an assembly instruction sequence block. Read by the
>          * kernel with single-copy atomicity semantics. Aligned on
>          * 64-bit.
>          */
> 
> This really explains that rseq_cs field of struct rseq holds a pointer
> to the current struct rseq_cs (or NULL), which makes it obvious why this
> needs to be two different structures.

I think I'm still missing things as its not obvious to me at all :/

We could equally well have chosen a single structure and picked the
post_commit_ip field to trigger things from, no?

The only down side seems to be that we must then impose ordering (but UP
ordering, so that's cheap) between writing the abort_ip and
post_commit_ip.

That is; something like so:

struct rseq {
	union rseq_event_cpu u;

	u64 abort_ip;
	u64 post_commit_ip;
};

Where userspace must do:

	r->abort_ip = $abort_ip;
	barrier();
	WRITE_ONCE(r->post_commit_ip, $post_commit_ip);
	barrier();

Which is not much different from what Paul did, except he kept the
abort_ip in a register (which must be loaded before setting the
commit_ip).

And the kernel checks post_commit_ip, if 0, nothing happens, otherwise
we check instruction_pointer and do magic.

Then after the commit, we clear post_commit_ip again; just like we now
clear the rseq_cs pointer.

AFAICT this is an equally valid approach. So why split and put that
indirection in?

> Combined with other recent feedback, this becomes:
> 
>  *   The abort_ip address needs to be lesser than start_ip, or

Isn't it "less than" ?

>  *   greater-or-equal the post_commit_ip. Step [4] and the failure
>  *   code step [F1] need to be at addresses lesser than start_ip, or
>  *   greater-or-equal the post_commit_ip.
> 


> >> +	if (current->rseq) {
> >> +		/*
> >> +		 * If rseq is already registered, check whether
> >> +		 * the provided address differs from the prior
> >> +		 * one.
> >> +		 */
> >> +		if (current->rseq != rseq)
> >> +			return -EBUSY;
> > 
> > Why explicitly allow resetting the same value?
> 
> The foreseen use is as follows: let's assume we have one or more
> user-space libraries, and possibly the application, each using rseq.
> They would each define a struct rseq TLS. They are expected to all
> give it the same name (e.g. __rseq_thread_state), and mark it as a
> weak symbol, so all uses of that symbol within the process address
> space will refer to the same address for a given thread.

Cute!

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web