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

----- On Nov 16, 2017, at 2:06 PM, Thomas Gleixner tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org wrote:

On Thu, 16 Nov 2017, Mathieu Desnoyers wrote:

quoted

----- On Nov 16, 2017, at 1:43 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:

quoted

On Tue, Nov 14, 2017 at 03:03:51PM -0500, Mathieu Desnoyers wrote:

quoted

+/*
+ * 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.

quoted

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.

Allright, I'll set the rseq_event_mask to 0 explicitly on exec() and
thread-fork, and copy it from the parent on process-fork.

Thanks,

Mathieu

Thanks,

	tglx

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