Re: [RFC PATCH v11 for 4.15 01/24] Restartable sequences system call

[Thomas Gleixner]

On Thu, 16 Nov 2017, Mathieu Desnoyers wrote:
> ----- On Nov 16, 2017, at 1:43 PM, Peter Zijlstra peterz@xxxxxxxxxxxxx wrote:
> 
> > On Tue, Nov 14, 2017 at 03:03:51PM -0500, Mathieu Desnoyers wrote:
> >> +/*
> >> + * If parent process has a registered restartable sequences area, the
> >> + * child inherits. Only applies when forking a process, not a thread. In
> >> + * case a parent fork() in the middle of a restartable sequence, set the
> >> + * resume notifier to force the child to retry.
> >> + */
> >> +static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
> >> +{
> >> +	if (clone_flags & CLONE_THREAD) {
> >> +		t->rseq = NULL;
> >> +		t->rseq_len = 0;
> >> +		t->rseq_sig = 0;
> >> +	} else {
> >> +		t->rseq = current->rseq;
> >> +		t->rseq_len = current->rseq_len;
> >> +		t->rseq_sig = current->rseq_sig;
> >> +		rseq_set_notify_resume(t);
> >> +	}
> >> +}
> > 
> > This hurts my brain... what happens if you fork a multi-threaded
> > process?
> > 
> > Do we fully inherit the TLS state of the calling thread?
> 
> Yes, exactly. The user-space TLS should be inherited from that of
> the calling thread.
> 
> At kernel-level, the only thing that's not inherited here is the
> task struct rseq_event_mask, which tracks whether a restart is
> needed. But this would only be relevant if fork() can be invoked
> from a signal handler, or if fork() could be invoked from a
> rseq critical section (which really makes little sense).

Whether it makes sense or not does not matter much, especially in context
of user space. You cannot make assumptions like that. When something can be
done, then it's bound to happen sooner than later because somebody thinks
he is extra clever.

The first priority is robustness in any aspect which has to do with user
space.

> Should I copy the current->rseq_event_mask on process fork just to
> be on the safe side though ?

I think so, unless you let fork() fail when invoked from a rseq critical
section.

Thanks,

	tglx