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

Groups > linux.kernel > #1353712 > unrolled thread

[RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@kernel.org> - 2016-03-09 02:30 +0100
    Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Ingo Molnar <mingo@kernel.org> - 2016-03-09 10:00 +0100
      Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Szabolcs Nagy <nsz@port70.net> - 2016-03-09 12:40 +0100
        Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Szabolcs Nagy <nsz@port70.net> - 2016-03-09 12:50 +0100
        Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-09 20:50 +0100
          Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@amacapital.net> - 2016-03-09 22:00 +0100
            Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-09 22:30 +0100
            Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-10 12:00 +0100
          Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-10 04:40 +0100
            Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-10 12:20 +0100
              Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-10 17:50 +0100
                Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-10 19:10 +0100
                  Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-11 00:30 +0100
                    Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Szabolcs Nagy <nsz@port70.net> - 2016-03-11 01:20 +0100
                      Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-11 01:50 +0100
                        Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@amacapital.net> - 2016-03-11 02:20 +0100
                        Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Szabolcs Nagy <nsz@port70.net> - 2016-03-11 02:40 +0100
                          Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-11 03:00 +0100
                          Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Szabolcs Nagy <nsz@port70.net> - 2016-03-11 03:00 +0100
                    Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-11 10:40 +0100
                      Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Szabolcs Nagy <nsz@port70.net> - 2016-03-11 12:40 +0100
                        Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 20:30 +0100
                          Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@amacapital.net> - 2016-03-11 20:40 +0100
                            Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 20:40 +0100
                              Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 20:50 +0100
                                Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-12 18:10 +0100
                                  Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-12 19:20 +0100
                          Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-12 18:10 +0100
                            Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-12 19:10 +0100
                              Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-12 19:50 +0100
                                Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Rich Felker <dalias@libc.org> - 2016-03-12 20:10 +0100
                        Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation  helpers Ingo Molnar <mingo@kernel.org> - 2016-03-12 18:10 +0100
    Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@amacapital.net> - 2016-03-09 19:00 +0100
      Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@amacapital.net> - 2016-03-09 22:30 +0100
        Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers Andy Lutomirski <luto@amacapital.net> - 2016-03-12 19:20 +0100

Page 1 of 2  [1] 2  Next page →

#1353712 — [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

musl implements system call cancellation in an unusual but clever way.
When a thread issues a cancellable syscall, musl issues the syscall
through a special thunk that looks roughly like this:

cancellable_syscall:
	test whether a cancel is queued
	jnz cancel_me
	int $0x80
end_cancellable_syscall:

If a pthread cancellation signal hits with
cancellable_syscall <= EIP < end_cancellable_syscall, then the
signal interrupted a cancellation point before the syscall in
question started.  If so, it rewrites the calling context to skip
the syscall and simulate a -EINTR return.  The caller will detect
this simulated -EINTR or an actual -EINTR and handle a possible
cancellation event.

This technique doesn't work if int $0x80 is replaced by a call to
AT_SYSINFO: the signal handler can no longer tell whether it's
interrupting a call to AT_SYSINFO or, if it is, where AT_SYSINFO was
called from.

Add minimal helpers so that musl's signal handler can learn the
status of a possible pending AT_SYSINFO invocation and, if it hasn't
entered the kernel yet, abort it without needing to parse the vdso
DWARF unwind data.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---

musl people-

Does this solve your AT_SYSINFO cancellation problem?  I'd like to
make sure it survives an actual implementation before I commit to the ABI.

x86 people-

Are you okay with this idea?


 arch/x86/entry/vdso/Makefile                      |   3 +-
 arch/x86/entry/vdso/vdso32/cancellation_helpers.c | 116 ++++++++++++++++++++++
 arch/x86/entry/vdso/vdso32/vdso32.lds.S           |   2 +
 tools/testing/selftests/x86/unwind_vdso.c         |  57 +++++++++--
 4 files changed, 171 insertions(+), 7 deletions(-)
 create mode 100644 arch/x86/entry/vdso/vdso32/cancellation_helpers.c

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index b88846471247..465052b49603 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -130,7 +130,7 @@ override obj-dirs = $(dir $(obj)) $(obj)/vdso32/
 
 targets += vdso32/vdso32.lds
 targets += vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o
-targets += vdso32/vclock_gettime.o
+targets += vdso32/vclock_gettime.o vdso32/cancellation_helpers.o
 
 KBUILD_AFLAGS_32 := $(filter-out -m64,$(KBUILD_AFLAGS)) -DBUILD_VDSO
 $(obj)/vdso32.so.dbg: KBUILD_AFLAGS = $(KBUILD_AFLAGS_32)
@@ -150,6 +150,7 @@ $(obj)/vdso32.so.dbg: KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)
 $(obj)/vdso32.so.dbg: FORCE \
 		      $(obj)/vdso32/vdso32.lds \
 		      $(obj)/vdso32/vclock_gettime.o \
+		      $(obj)/vdso32/cancellation_helpers.o \
 		      $(obj)/vdso32/note.o \
 		      $(obj)/vdso32/system_call.o \
 		      $(obj)/vdso32/sigreturn.o
diff --git a/arch/x86/entry/vdso/vdso32/cancellation_helpers.c b/arch/x86/entry/vdso/vdso32/cancellation_helpers.c
new file mode 100644
index 000000000000..3cb2e88baec6
--- /dev/null
+++ b/arch/x86/entry/vdso/vdso32/cancellation_helpers.c
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2016 Andrew Lutomirski
+ * Subject to the GNU Public License, v.2
+ *
+ * This provides helpers to enable libc implementations to cancel
+ * interrupted AT_SYSINFO invocations without needing to parse the
+ * DWARF unwinding instructions.
+ */
+
+#include <asm/signal.h>
+#include <asm/sigframe.h>
+
+extern char __kernel_vsyscall[] __attribute__((visibility("hidden")));
+extern char int80_landing_pad[] __attribute__((visibility("hidden")));
+
+static unsigned long *pending_syscall_retaddr_ptr(const void *context)
+{
+	const struct ucontext_ia32 *uc = context;
+	unsigned long ctx_eip = uc->uc_mcontext.ip;
+	unsigned long offset_into_vsyscall;
+	unsigned long *retaddr;
+
+	/*
+	 * An AT_SYSINFO system call is pending if and only if we're in
+	 * __kernel_vsyscall before int80_landing_pad.  If we're at
+	 * int80_landing_pad or beyond, we've finished the system call
+	 * and are on our way out.
+	 *
+	 * If we're at int80_landing_pad-2, then either we're using the
+	 * int $0x80 slow path because we have no fast system call
+	 * support or we are restarting a fast system call.  Either way,
+	 * the system call is still pending.
+	 */
+
+	if (ctx_eip < (unsigned long)__kernel_vsyscall ||
+	    ctx_eip >= (unsigned long)int80_landing_pad)
+		return NULL;
+
+	/*
+	 * The first three instructions of __kernel_vsyscall are one-byte
+	 * pushes.
+	 */
+	offset_into_vsyscall = (ctx_eip - (unsigned long)__kernel_vsyscall);
+	retaddr = (unsigned long *)uc->uc_mcontext.sp;
+	if (offset_into_vsyscall < 3)
+		retaddr += offset_into_vsyscall;
+	else
+		retaddr += 3;
+
+	/*
+	 * GCC (correctly) fails to deduce out that retaddr can't be NULL
+	 * in the success path.  Helping it out reduces code size.
+	 */
+	if (!retaddr)
+		__builtin_unreachable();
+
+	return retaddr;
+}
+
+/*
+ * If context is a sigcontet for a pending AT_SYSINFO syscall, returns
+ * the return address of that syscall.  Otherwise returns -1UL.
+ */
+unsigned long __vdso_pending_syscall_return_address(const void *context)
+{
+	unsigned long *retaddr = pending_syscall_retaddr_ptr(context);
+	return retaddr ? *retaddr : -1UL;
+}
+
+/*
+ * If context is a sigcontext for a pending AT_SYSINFO syscall, then
+ * this will pop off the call frame and point the context to
+ * AT_SYSINFO's return address.  ESP will contain whatever value it had
+ * immediately prior to the call instruction (i.e. ESP acts as though
+ * the system call returned normally).  EAX will be set to -EINTR.  All
+ * other GPRs will be clobbered.  __vdso_abort_pending_syscall will
+ * return 0.
+ *
+ * If context is a valid sigcontext that does not represent a pending
+ * AT_SYSINFO syscall, then __vdso_abort_pending_syscall returns
+ * -EINVAL.
+ *
+ * If context is not a valid sigcontext at all, behavior is undefined.
+ */
+long __vdso_abort_pending_syscall(void *context)
+{
+	struct ucontext_ia32 *uc = context;
+	unsigned long *retaddr = pending_syscall_retaddr_ptr(context);
+
+	if (!retaddr)
+		return -EINVAL;
+
+	uc->uc_mcontext.ip = *retaddr;
+	uc->uc_mcontext.sp = (unsigned long)(retaddr + 1);
+
+	/*
+	 * Clobber GPRs -- we don't want to implement full unwinding, and we
+	 * don't want userspace to start expecting anything about the final
+	 * state of the GPRs.
+	 *
+	 * (There really are subtleties here.  EAX can be clobbered by
+	 *  syscall restart, and register limitations mean that the
+	 *  saved context has at least one of the argument registers
+	 *  used for a different purpose by the calling sequence just
+	 *  prior to kernel entry.  In the current implementation, that
+	 *  register is EBP, but it could change.)
+	 */
+	uc->uc_mcontext.ax = -EINTR;
+	uc->uc_mcontext.bx = 0xFFFFFFFF;
+	uc->uc_mcontext.cx = 0xFFFFFFFF;
+	uc->uc_mcontext.dx = 0xFFFFFFFF;
+	uc->uc_mcontext.si = 0xFFFFFFFF;
+	uc->uc_mcontext.di = 0xFFFFFFFF;
+	uc->uc_mcontext.bp = 0xFFFFFFFF;
+	return 0;
+}
diff --git a/arch/x86/entry/vdso/vdso32/vdso32.lds.S b/arch/x86/entry/vdso/vdso32/vdso32.lds.S
index 31056cf294bf..f04e8bd30755 100644
--- a/arch/x86/entry/vdso/vdso32/vdso32.lds.S
+++ b/arch/x86/entry/vdso/vdso32/vdso32.lds.S
@@ -25,6 +25,8 @@ VERSION
 		__vdso_clock_gettime;
 		__vdso_gettimeofday;
 		__vdso_time;
+		__vdso_pending_syscall_return_address;
+		__vdso_abort_pending_syscall;
 	};
 
 	LINUX_2.5 {
diff --git a/tools/testing/selftests/x86/unwind_vdso.c b/tools/testing/selftests/x86/unwind_vdso.c
index 00a26a82fa98..7c649b4b6834 100644
--- a/tools/testing/selftests/x86/unwind_vdso.c
+++ b/tools/testing/selftests/x86/unwind_vdso.c
@@ -35,6 +35,7 @@ int main()
 #include <syscall.h>
 #include <unistd.h>
 #include <string.h>
+#include <errno.h>
 #include <inttypes.h>
 #include <sys/mman.h>
 #include <signal.h>
@@ -88,8 +89,12 @@ static unsigned long sysinfo;
 static bool got_sysinfo = false;
 static unsigned long return_address;
 
+static unsigned long (*vdso_pending_syscall_return_address)(
+	const void *context);
+
 struct unwind_state {
 	unsigned long ip;	/* trap source */
+	unsigned long ax;	/* ax at call site */
 	int depth;		/* -1 until we hit the trap source */
 };
 
@@ -115,7 +120,7 @@ _Unwind_Reason_Code trace_fn(struct _Unwind_Context * ctx, void *opaque)
 		unsigned long ebp = _Unwind_GetGR(ctx, 5);
 		unsigned long esi = _Unwind_GetGR(ctx, 6);
 		unsigned long edi = _Unwind_GetGR(ctx, 7);
-		bool ok = (eax == SYS_getpid || eax == getpid()) &&
+		bool ok = (eax == SYS_break || eax == -ENOSYS) &&
 			ebx == 1 && ecx == 2 && edx == 3 &&
 			esi == 4 && edi == 5 && ebp == 6;
 
@@ -125,6 +130,8 @@ _Unwind_Reason_Code trace_fn(struct _Unwind_Context * ctx, void *opaque)
 		       (ok ? "OK" : "FAIL"),
 		       eax, ebx, ecx, edx, esi, edi, ebp);
 
+		state->ax = eax;
+
 		return _URC_NORMAL_STOP;
 	} else {
 		state->depth++;
@@ -137,6 +144,7 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
 	ucontext_t *ctx = (ucontext_t *)ctx_void;
 	struct unwind_state state;
 	unsigned long ip = ctx->uc_mcontext.gregs[REG_EIP];
+	unsigned long reported_return_address = 0;
 
 	if (!got_sysinfo && ip == sysinfo) {
 		got_sysinfo = true;
@@ -148,8 +156,15 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
 		       ip, return_address);
 	}
 
-	if (!got_sysinfo)
-		return;		/* Not there yet */
+	if (!got_sysinfo) {
+		if (vdso_pending_syscall_return_address &&
+		    vdso_pending_syscall_return_address(ctx_void) != -1UL) {
+			printf("[FAIL]\t__vdso_pending_syscall_return_address incorrectly detected a pending syscall\n");
+			nerrs++;
+		}
+
+		return;		/* We haven't started AT_SYSINFO yet */
+	}
 
 	if (ip == return_address) {
 		ctx->uc_mcontext.gregs[REG_EFL] &= ~X86_EFLAGS_TF;
@@ -157,11 +172,32 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
 		return;
 	}
 
-	printf("\tSIGTRAP at 0x%lx\n", ip);
+	if (vdso_pending_syscall_return_address) {
+		reported_return_address =
+			vdso_pending_syscall_return_address(ctx_void);
+		if (reported_return_address != -1UL)
+			printf("\tSIGTRAP at 0x%lx, pending syscall will return to 0x%lx\n",
+			       ip, reported_return_address);
+		else
+			printf("\tSIGTRAP at 0x%lx, no syscall pending\n", ip);
+	} else {
+		printf("\tSIGTRAP at 0x%lx\n", ip);
+	}
 
 	state.ip = ip;
 	state.depth = -1;
 	_Unwind_Backtrace(trace_fn, &state);
+
+	if (vdso_pending_syscall_return_address) {
+		unsigned long expected =
+			(state.ax == SYS_break ? return_address : -1UL);
+		if (reported_return_address != expected) {
+			printf("[FAIL]\t  __vdso_pending_syscall_return_address returned 0x%lx; expected 0x%lx\n", reported_return_address, expected);
+			nerrs++;
+		} else {
+			printf("[OK]\t  __vdso_pending_syscall_return_address returned the correct value\n");
+		}
+	}
 }
 
 int main()
@@ -177,12 +213,21 @@ int main()
 		       info.dli_fname, info.dli_fbase);
 	}
 
+	void *vdso = dlopen("linux-gate.so.1", RTLD_NOW);
+	if (vdso)
+		vdso_pending_syscall_return_address = dlsym(vdso, "__vdso_pending_syscall_return_address");
+
 	sethandler(SIGTRAP, sigtrap, 0);
 
-	syscall(SYS_getpid);  /* Force symbol binding without TF set. */
+	syscall(SYS_break);  /* Force symbol binding without TF set. */
 	printf("[RUN]\tSet TF and check a fast syscall\n");
 	set_eflags(get_eflags() | X86_EFLAGS_TF);
-	syscall(SYS_getpid, 1, 2, 3, 4, 5, 6);
+
+	/*
+	 * We need a harmless syscall that will never return its own syscall
+	 * nr.  SYS_break is not implemented and returns -ENOSYS.
+	 */
+	syscall(SYS_break, 1, 2, 3, 4, 5, 6);
 	if (!got_sysinfo) {
 		set_eflags(get_eflags() & ~X86_EFLAGS_TF);
 
-- 
2.5.0

[toc] | [next] | [standalone]

#1353906

* Andy Lutomirski <luto@kernel.org> wrote:

> musl implements system call cancellation in an unusual but clever way.

So I'm sceptical about the concept.

Could someone remind me why cancellation points matter to user-space?

I know the pthread APIs and semantics that are behind it, I just don't see how it 
can be truly utilized for any meaningful programmatic property: for example the 
moment you add any sort of ad-hoc printf() based tracing or any other spontaneous 
logging IO to your application, you add in a lot of potential cancellation points 
into various places in your user-space logic ...

It's _very_ easy to add inadvertent cancellation point to the code in practice, so 
using the default pthread cancellation model and relying on what is a cancellation 
point is crazy and very libc dependent in general. POSIX seems to be pretty vague 
about it as well. So unless you make heavy use of pthread_setcancelstate() to 
explicitly mark your work atoms, it's a really bad interface to rely on.

And if you are using pthread_setcancelstate(), instead of relying on calcellation, 
then you are not really using the built-in cancellation points but have to spike 
your code with pthread_testcancel(). In that case, why not just use your own 
explicit 'cancellation' points in a few strategic places - which is mostly just a 
simple flag really. That's what most worker thread models that I've seen use.

I suspect more complex runtimes like java runtimes couldn't care less, so it's 
really something that only libc using C/C++ code cares about.

> When a thread issues a cancellable syscall, musl issues the syscall
> through a special thunk that looks roughly like this:
> 
> cancellable_syscall:
> 	test whether a cancel is queued
> 	jnz cancel_me
> 	int $0x80
> end_cancellable_syscall:
> 
> If a pthread cancellation signal hits with
> cancellable_syscall <= EIP < end_cancellable_syscall, then the
> signal interrupted a cancellation point before the syscall in
> question started.  If so, it rewrites the calling context to skip
> the syscall and simulate a -EINTR return.  The caller will detect
> this simulated -EINTR or an actual -EINTR and handle a possible
> cancellation event.

Why is so much complexity added to avoid a ~3 instructions window where 
calcellation is tested? Cancellation at work atom boundaries is a fundamentally 
'polling' model anyway, and signal delivery is asynchronous, with a fundamental 
IPI delay if it's cross-CPU.

> This technique doesn't work if int $0x80 is replaced by a call to
> AT_SYSINFO: the signal handler can no longer tell whether it's
> interrupting a call to AT_SYSINFO or, if it is, where AT_SYSINFO was
> called from.
> 
> Add minimal helpers so that musl's signal handler can learn the
> status of a possible pending AT_SYSINFO invocation and, if it hasn't
> entered the kernel yet, abort it without needing to parse the vdso
> DWARF unwind data.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> 
> musl people-
> 
> Does this solve your AT_SYSINFO cancellation problem?  I'd like to
> make sure it survives an actual implementation before I commit to the ABI.
> 
> x86 people-
> 
> Are you okay with this idea?
> 
> 
>  arch/x86/entry/vdso/Makefile                      |   3 +-
>  arch/x86/entry/vdso/vdso32/cancellation_helpers.c | 116 ++++++++++++++++++++++
>  arch/x86/entry/vdso/vdso32/vdso32.lds.S           |   2 +
>  tools/testing/selftests/x86/unwind_vdso.c         |  57 +++++++++--
>  4 files changed, 171 insertions(+), 7 deletions(-)
>  create mode 100644 arch/x86/entry/vdso/vdso32/cancellation_helpers.c

I'd really like to see a cost/benefit analysis here! Some before/after explanation 
- exactly what is not possible today (in practical terms), what are the practical 
effects of not being able to do that, and how would the bright future look like?

Thanks,

	Ingo

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

#1354039 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Ingo Molnar <mingo@kernel.org> [2016-03-09 09:56:31 +0100]:
> * Andy Lutomirski <luto@kernel.org> wrote:
> 
> > musl implements system call cancellation in an unusual but clever way.
> 
> So I'm sceptical about the concept.
> 
> Could someone remind me why cancellation points matter to user-space?
> 

because of standards.

> I know the pthread APIs and semantics that are behind it, I just don't see how it 
> can be truly utilized for any meaningful programmatic property: for example the 
> moment you add any sort of ad-hoc printf() based tracing or any other spontaneous 
> logging IO to your application, you add in a lot of potential cancellation points 
> into various places in your user-space logic ...
> 
> It's _very_ easy to add inadvertent cancellation point to the code in practice, so 
> using the default pthread cancellation model and relying on what is a cancellation 
> point is crazy and very libc dependent in general. POSIX seems to be pretty vague 
> about it as well. So unless you make heavy use of pthread_setcancelstate() to 
> explicitly mark your work atoms, it's a really bad interface to rely on.
> 

if the canceled thread only executes code that expects cancellation then
it should work (code that does not expect cancellation won't have cancellation
cleanup handlers set up and thus cancelling it can cause problems).

> And if you are using pthread_setcancelstate(), instead of relying on calcellation, 
> then you are not really using the built-in cancellation points but have to spike 
> your code with pthread_testcancel(). In that case, why not just use your own 
> explicit 'cancellation' points in a few strategic places - which is mostly just a 
> simple flag really. That's what most worker thread models that I've seen use.
> 

the point of cancellation is to be able to kill a thread that is in a
blocking syscall.  i don't see how a flag helps with that, this is hard
to do without libc help, hence pthread_cancel exists.

> I suspect more complex runtimes like java runtimes couldn't care less, so it's 
> really something that only libc using C/C++ code cares about.
> 

c++ code cannot be cancelled if it uses non-pod objects or c++ threads.
(destructor vs cancellation cleanup semantics is undefined)

this is for posix conforming code.

> > When a thread issues a cancellable syscall, musl issues the syscall
> > through a special thunk that looks roughly like this:
> > 
> > cancellable_syscall:
> > 	test whether a cancel is queued
> > 	jnz cancel_me
> > 	int $0x80
> > end_cancellable_syscall:
> > 
> > If a pthread cancellation signal hits with
> > cancellable_syscall <= EIP < end_cancellable_syscall, then the
> > signal interrupted a cancellation point before the syscall in
> > question started.  If so, it rewrites the calling context to skip
> > the syscall and simulate a -EINTR return.  The caller will detect
> > this simulated -EINTR or an actual -EINTR and handle a possible
> > cancellation event.
> 
> Why is so much complexity added to avoid a ~3 instructions window where 
> calcellation is tested? Cancellation at work atom boundaries is a fundamentally 
> 'polling' model anyway, and signal delivery is asynchronous, with a fundamental 
> IPI delay if it's cross-CPU.
> 

to avoid the race when the thread is cancelled after the test but before
the syscall see http://ewontfix.com/16/

> > This technique doesn't work if int $0x80 is replaced by a call to
> > AT_SYSINFO: the signal handler can no longer tell whether it's
> > interrupting a call to AT_SYSINFO or, if it is, where AT_SYSINFO was
> > called from.
> > 
> > Add minimal helpers so that musl's signal handler can learn the
> > status of a possible pending AT_SYSINFO invocation and, if it hasn't
> > entered the kernel yet, abort it without needing to parse the vdso
> > DWARF unwind data.
> > 
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > 
> > musl people-
> > 
> > Does this solve your AT_SYSINFO cancellation problem?  I'd like to
> > make sure it survives an actual implementation before I commit to the ABI.
> > 
> > x86 people-
> > 
> > Are you okay with this idea?
> > 
> > 
> >  arch/x86/entry/vdso/Makefile                      |   3 +-
> >  arch/x86/entry/vdso/vdso32/cancellation_helpers.c | 116 ++++++++++++++++++++++
> >  arch/x86/entry/vdso/vdso32/vdso32.lds.S           |   2 +
> >  tools/testing/selftests/x86/unwind_vdso.c         |  57 +++++++++--
> >  4 files changed, 171 insertions(+), 7 deletions(-)
> >  create mode 100644 arch/x86/entry/vdso/vdso32/cancellation_helpers.c
> 
> I'd really like to see a cost/benefit analysis here! Some before/after explanation 
> - exactly what is not possible today (in practical terms), what are the practical 
> effects of not being able to do that, and how would the bright future look like?
> 
> Thanks,
> 
> 	Ingo

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

#1354054 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Szabolcs Nagy <nsz@port70.net> [2016-03-09 12:34:50 +0100]:
> * Ingo Molnar <mingo@kernel.org> [2016-03-09 09:56:31 +0100]:
> > Why is so much complexity added to avoid a ~3 instructions window where 
> > calcellation is tested? Cancellation at work atom boundaries is a fundamentally 
> > 'polling' model anyway, and signal delivery is asynchronous, with a fundamental 
> > IPI delay if it's cross-CPU.
> > 
> 
> to avoid the race when the thread is cancelled after the test but before
> the syscall see http://ewontfix.com/16/
> 
wrong link
http://ewontfix.com/2/

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

#1354394 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Wed, Mar 9, 2016 at 3:34 AM, Szabolcs Nagy <nsz@port70.net> wrote:
>>
>> Could someone remind me why cancellation points matter to user-space?
>
> because of standards.

So quite frankly, if we have to do kernel support for this, then let's
do it right, instead of just perpetuating a hack that was done in user
space in a new way.

We already have support for cancelling blocking system calls early: we
do it for fatal signals (exactly because we know that it's ok to
return -EINTR without failing POSIX semantics - the dying thread will
never actually *see* the -EINTR because it's dying).

I suspect that what you guys want is the same semantics as a fatal
signal (return early with -EINTR), but without the actual fatality
(you want to do cleanup in the cancelled thread).

I suspect that we could fairly easily give those kinds of semantics.
We could add a new flag to the sigaction (sa_flags) that says "this
signal interrupts even uninterruptible system calls".

Would that be good for you?

And if not, can you explain the exact semantics you need? IThere might
be some reason why you cannot reserve a particular signal for this,
for example, but I'd like to know more precisely..

Because this "let's compare addresses" seems just excessively hacky.
It's a clever little hack when you're doing user space and don't want
to rely on kernel changes, but now that Andy is actuallty trying to
push kernel changes it turns into just disgusting.

              Linus

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

#1354446 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Wed, Mar 9, 2016 at 11:47 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Mar 9, 2016 at 3:34 AM, Szabolcs Nagy <nsz@port70.net> wrote:
>>>
>>> Could someone remind me why cancellation points matter to user-space?
>>
>> because of standards.
>
> So quite frankly, if we have to do kernel support for this, then let's
> do it right, instead of just perpetuating a hack that was done in user
> space in a new way.
>
> We already have support for cancelling blocking system calls early: we
> do it for fatal signals (exactly because we know that it's ok to
> return -EINTR without failing POSIX semantics - the dying thread will
> never actually *see* the -EINTR because it's dying).
>
> I suspect that what you guys want is the same semantics as a fatal
> signal (return early with -EINTR), but without the actual fatality
> (you want to do cleanup in the cancelled thread).
>

How safe would this be in a multithreaded process?  For example, if
open() gets canceled in the "killable" sense, is it guaranteed that no
file descriptor will be allocated?

> I suspect that we could fairly easily give those kinds of semantics.
> We could add a new flag to the sigaction (sa_flags) that says "this
> signal interrupts even uninterruptible system calls".
>
> Would that be good for you?
>
> And if not, can you explain the exact semantics you need? IThere might
> be some reason why you cannot reserve a particular signal for this,
> for example, but I'd like to know more precisely..
>
> Because this "let's compare addresses" seems just excessively hacky.
> It's a clever little hack when you're doing user space and don't want
> to rely on kernel changes, but now that Andy is actuallty trying to
> push kernel changes it turns into just disgusting.
>

Let me try to summarize my understanding of the semantics.

Thread A sends thread B a signal.  Thread B wants to ignore the signal
and defer handling unless it's either in a particular syscall and
returns -EINTR or unless the thread is about to do the syscall.

This would all be trivial if there were a way to set up a signal that
is *only* delivered in response to a syscall, no?  SA_ONLY_IN_SYSCALL,
perhaps?



Frankly, I'm a bir surprised that musl didn't take the approach of
"pthread cancellation is not such a great idea -- let's just not
support it".


>               Linus



-- 
Andy Lutomirski
AMA Capital Management, LLC

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

#1354463 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Wed, Mar 9, 2016 at 12:57 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> How safe would this be in a multithreaded process?  For example, if
> open() gets canceled in the "killable" sense, is it guaranteed that no
> file descriptor will be allocated?

Not all system calls can be killed, we only do the usual cases. A
system call has to have the proper EINTR logic in place, so it's not
like we kill system calls at any random point.

> Let me try to summarize my understanding of the semantics.
>
> Thread A sends thread B a signal.  Thread B wants to ignore the signal
> and defer handling unless it's either in a particular syscall and
> returns -EINTR or unless the thread is about to do the syscall.

Note that for the kernel, we don't actually have to use a signal for
this at all. Our existing "cancel system calls" code only works for
fatal signals, but that's just a trivial implementation issue.

We could add a system call that just sets a cancel flag in another
thread, and we'd just use that cancel flag to say "abort the currently
executing system call with EINTR" - in all the same places we
currently dot hat "fatal_signal_pending()" thing.

You'd still have to have all the user-space logic to do the
cancellation cleanup etc. But now you could actually cancel a write()
system call in the *middle*, which is currently just not an option.

                Linus

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

#1355003 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Andy Lutomirski <luto@amacapital.net> wrote:

> Let me try to summarize my understanding of the semantics.
> 
> Thread A sends thread B a signal.  Thread B wants to ignore the signal and defer 
> handling unless it's either in a particular syscall and returns -EINTR or unless 
> the thread is about to do the syscall.

s/the syscall/an interruptible syscall/

The fundamental intention is to essentially allow the asynchronous killing 
(cancellation) of pthread threads without corrupting user-space data structures 
such as malloc() state.

There's a long list of system calls listed at pthread(8) that must be cancellation 
points, plus an even longer list of system calls and libc APIs that may be 
cancellation points.

On glibc signal 32 (the first RT signal) is used as the cancellation signal.

But I guess you knew all this already!

So my original thinking was this:

  | What surprises me is why Musl even bothers with trying to detect system calls 
  | that are about to be executed. Cancellation is a fundamentally polling-type 
  | API, a very small, 2-3 instructions window to 'miss' the current system call 
  | has no practical latency effect - so why does it even attempt to detect that 
  | RIP range? Why doesn't Musl just check the cancellation flag (activated by 
  | signal 32) and is content? Am I misunderstanding something about it?

... and when I wrote that up I realized the detail that I missed: it's a 
problematic race if the thread starts a long-lived blocking system call (such as 
accept()), shortly after the cancellation signal has been sent.

So the signal-32 handler _has_ to check the RIP and make sure that the system call 
is not about to be executed - cancellation might be delayed indefinitely 
otherwise. It's essentially needed for correctness.

Linus's suggestion to allow system calls to be more interruptible via a new SA_ 
flag also makes sense, but that is a latency improvement change - while the aspect 
I was wondering about was a fundamental correctness detail.

So I withdraw my objection regarding AT_SYSINFO cancellation helpers. User-space 
needs to have a signal-atomic way to prevent system calls from being started after 
a cancellation signal has been received.

Thanks,

	Ingo

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

#1354826 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Wed, Mar 09, 2016 at 11:47:30AM -0800, Linus Torvalds wrote:
> On Wed, Mar 9, 2016 at 3:34 AM, Szabolcs Nagy <nsz@port70.net> wrote:
> >>
> >> Could someone remind me why cancellation points matter to user-space?
> >
> > because of standards.
> 
> So quite frankly, if we have to do kernel support for this, then let's
> do it right, instead of just perpetuating a hack that was done in user
> space in a new way.
> 
> We already have support for cancelling blocking system calls early: we
> do it for fatal signals (exactly because we know that it's ok to
> return -EINTR without failing POSIX semantics - the dying thread will
> never actually *see* the -EINTR because it's dying).
> 
> I suspect that what you guys want is the same semantics as a fatal
> signal (return early with -EINTR), but without the actual fatality
> (you want to do cleanup in the cancelled thread).

No, the semantics need to be identical to EINTR -- you can't cancel an
operation where some work has already been done. This is both a POSIX
requirement and a conceptual requirement. When a thread is cancelled,
the process is not terminating abnormally; it's continuing. It needs
to be able to know whether some work was completed, because that
changes what the cleanup code needs to do in order for a consistent
state to be maintained. This is most critical with syscalls that
allocate or free resources -- open, close, recvmsg accepting file
descriptors, etc. -- but it can even matter for reads and writes.
This is the whole reason we need a race-free cancellation rather than
the buggy implementation glibc historically used (which they are in
the process of fixing too).

Anyway, in the case where some but not all work was completed already
at the time the cancellation request was made, the function needs to
return and report whatever was successful.

> I suspect that we could fairly easily give those kinds of semantics.
> We could add a new flag to the sigaction (sa_flags) that says "this
> signal interrupts even uninterruptible system calls".

This would not help, because whether the system call should be
cancellable is a function of the caller, not the system call; some
syscalls are cancellable when used in one place but not in others.

Also it does not solve the race condition; it's possible that the
signal is delivered _after_ userspace checks the cancellation flag,
but _before_ the syscall is made. Thus we need a way to probe whether
the program counter is in a range between the userspace flag check and
the syscall instruction.

I believe a new kernel cancellation API with a sticky cancellation
flag (rather than a signal), and a flag or'd onto the syscall number
to make it cancellable at the call point, could work, but then
userspace needs to support fairly different old and new kernel APIs in
order to be able to run on old kernels while also taking advantage of
new ones, and it's not clear to me that it would actually be
worthwhile to do so. I could see doing it for a completely new syscall
API, but as a second syscall API for a system that already has one it
seems gratuitous. From my perspective the existing approach (checking
program counter from signal handler) is very clean and simple. After
all it made enough sense that I was able to convince the glibc folks
to adopt it.

Rich

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

#1355037 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Rich Felker <dalias@libc.org> wrote:

> [...]
>
> I believe a new kernel cancellation API with a sticky cancellation flag (rather 
> than a signal), and a flag or'd onto the syscall number to make it cancellable 
> at the call point, could work, but then userspace needs to support fairly 
> different old and new kernel APIs in order to be able to run on old kernels 
> while also taking advantage of new ones, and it's not clear to me that it would 
> actually be worthwhile to do so. I could see doing it for a completely new 
> syscall API, but as a second syscall API for a system that already has one it 
> seems gratuitous. From my perspective the existing approach (checking program 
> counter from signal handler) is very clean and simple. After all it made enough 
> sense that I was able to convince the glibc folks to adopt it.

I concur with your overall analysis, but things get a bit messy once we consider 
AT_SYSINFO which is a non-atomic mix of user-space and kernel-space code. Trying 
to hand cancellation status through that results in extra complexity:

 arch/x86/entry/vdso/Makefile                      |   3 +-
 arch/x86/entry/vdso/vdso32/cancellation_helpers.c | 116 ++++++++++++++++++++++
 arch/x86/entry/vdso/vdso32/vdso32.lds.S           |   2 +
 tools/testing/selftests/x86/unwind_vdso.c         |  57 +++++++++--
 4 files changed, 171 insertions(+), 7 deletions(-)

So instead of a sticky cancellation flag, we could introduce a sticky cancellation 
signal.

A 'sticky signal' is not cleared from signal_pending() when the signal handler 
executes, but it's automatically blocked so no signal handler recursion occurs.
(A sticky signal could still be cleared via a separate mechanism, by the 
 cancellation cleanup code.)

Such a 'sticky cancellation signal' would, in the racy situation, cause new 
blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
could still be used. (So the cancellation signal handler itself would still have 
access to various fundamental system calls.)

I think this would avoid messy coupling between the kernel's increasingly more 
varied system call entry code and C libraries.

Sticky signals could be requested via a new SA_ flag.

What do you think?

Thanks,

	Ingo

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

#1355260 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Thu, Mar 10, 2016 at 12:16:46PM +0100, Ingo Molnar wrote:
> 
> * Rich Felker <dalias@libc.org> wrote:
> 
> > [...]
> >
> > I believe a new kernel cancellation API with a sticky cancellation flag (rather 
> > than a signal), and a flag or'd onto the syscall number to make it cancellable 
> > at the call point, could work, but then userspace needs to support fairly 
> > different old and new kernel APIs in order to be able to run on old kernels 
> > while also taking advantage of new ones, and it's not clear to me that it would 
> > actually be worthwhile to do so. I could see doing it for a completely new 
> > syscall API, but as a second syscall API for a system that already has one it 
> > seems gratuitous. From my perspective the existing approach (checking program 
> > counter from signal handler) is very clean and simple. After all it made enough 
> > sense that I was able to convince the glibc folks to adopt it.
> 
> I concur with your overall analysis, but things get a bit messy once we consider 
> AT_SYSINFO which is a non-atomic mix of user-space and kernel-space code. Trying 
> to hand cancellation status through that results in extra complexity:
> 
>  arch/x86/entry/vdso/Makefile                      |   3 +-
>  arch/x86/entry/vdso/vdso32/cancellation_helpers.c | 116 ++++++++++++++++++++++
>  arch/x86/entry/vdso/vdso32/vdso32.lds.S           |   2 +
>  tools/testing/selftests/x86/unwind_vdso.c         |  57 +++++++++--
>  4 files changed, 171 insertions(+), 7 deletions(-)
> 
> So instead of a sticky cancellation flag, we could introduce a sticky cancellation 
> signal.
> 
> A 'sticky signal' is not cleared from signal_pending() when the signal handler 
> executes, but it's automatically blocked so no signal handler recursion occurs.
> (A sticky signal could still be cleared via a separate mechanism, by the 
>  cancellation cleanup code.)
> 
> Such a 'sticky cancellation signal' would, in the racy situation, cause new 
> blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
> could still be used. (So the cancellation signal handler itself would still have 
> access to various fundamental system calls.)
> 
> I think this would avoid messy coupling between the kernel's increasingly more 
> varied system call entry code and C libraries.
> 
> Sticky signals could be requested via a new SA_ flag.
> 
> What do you think?

This still doesn't address the issue that the code making the syscall
needs to be able to control whether it's cancellable or not. Not only
do some syscalls whose public functions are cancellation points need
to be used internally in non-cancellable ways; there's also the
pthread_setcancelstate interface that allows deferring cancellation so
that it's possible to call functions which are cancellation points
without invoking cancellation.

Ideally all syscalls would be like pselect/ppoll and take a sigset_t
to unmask/remask atomically with respect to the syscall action. Then
implementing cancellation (as well as using EINTR race-free) would be
trivial. But this is obviously not a practical change to make.

From my standpoint the simplest and cleanest solution is for vdso to
provide a predicate function that takes a ucontext_t and returns
true/false for whether it represents a state prior to entering (or
reentering, for restart state) the vdso syscall. If vdso exports this
symbol libc can use vdso syscall with cancellation. If not, it can
just fallback to straight inline syscall like now.

Rich

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

#1355298 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Rich Felker <dalias@libc.org> wrote:

> > So instead of a sticky cancellation flag, we could introduce a sticky 
> > cancellation signal.
> > 
> > A 'sticky signal' is not cleared from signal_pending() when the signal handler 
> > executes, but it's automatically blocked so no signal handler recursion 
> > occurs. (A sticky signal could still be cleared via a separate mechanism, by 
> > the cancellation cleanup code.)
> > 
> > Such a 'sticky cancellation signal' would, in the racy situation, cause new 
> > blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
> > could still be used. (So the cancellation signal handler itself would still 
> > have access to various fundamental system calls.)
> > 
> > I think this would avoid messy coupling between the kernel's increasingly more 
> > varied system call entry code and C libraries.
> > 
> > Sticky signals could be requested via a new SA_ flag.
> > 
> > What do you think?
> 
> This still doesn't address the issue that the code making the syscall needs to 
> be able to control whether it's cancellable or not. Not only do some syscalls 
> whose public functions are cancellation points need to be used internally in 
> non-cancellable ways; there's also the pthread_setcancelstate interface that 
> allows deferring cancellation so that it's possible to call functions which are 
> cancellation points without invoking cancellation.

I don't think there's a problem - but I might be wrong:

One way I think it would work is the following: a sticky signal is not the 
cancellation flag - it's a helper construct to implement the flag in user-space in 
a race-free way.

Say you have RT signal-32 as the cancellation signal, and it's a sticky signal.

When pthread_cancel() wants to cancel another thread, it first (atomically) sets 
the desired cancel state of the target thread. If that state signals that the 
thread is cancellable right now, and that we initiated its cancellation, then we 
send signal-32. I.e. the signal only ever gets sent if the thread is in a 
cancellable state.

libc internal functions and the pthread_setcancelstate() API can temporarily 
change the cancel state of a thread to non-cancellable - but pthread_cancel() 
observes those state transitions.

The 'sticky' nature of signal-32 will make a difference in the following race 
condition, if the cancellation flag is checked before a system call by the C 
library, and signal-32 arrives before the system call is executed. In that case 
the 'sticky' nature of the signal makes sure that all subsequent system calls 
return immediately.

The sticky signal is only ever sent when the thread is in cancellable state - and 
if the target thread notices the cancellation request before the signal arrives, 
it first waits for its arrival before executing any new system calls (as part of 
the teardown, etc.).

So the C library never has to do complex work with a sticky signal pending.

Does that make more sense to you?

> From my standpoint the simplest and cleanest solution is for vdso to provide a 
> predicate function that takes a ucontext_t and returns true/false for whether it 
> represents a state prior to entering (or reentering, for restart state) the vdso 
> syscall. If vdso exports this symbol libc can use vdso syscall with 
> cancellation. If not, it can just fallback to straight inline syscall like now.

Offering such a flag pushes unreasonable conceptual overhead into the vDSO proper 
in the long run: right now it might be easy to implement because the code paths 
are relatively simple and we can generate the flag passively via RIP checking - 
but if the vDSO grows more complex interfaces in the future, we'd essentially have 
to track our entry/exit state dynamically which sucks ...

I think the real solution is to push all such overhead to the cancellation API 
side: it can track its state, and it can use sticky signals to make sure blocking 
system calls return immediately once a cancellation is in progress.

Thanks,

	Ingo

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

#1355453 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
> 
> * Rich Felker <dalias@libc.org> wrote:
> 
> > > So instead of a sticky cancellation flag, we could introduce a sticky 
> > > cancellation signal.
> > > 
> > > A 'sticky signal' is not cleared from signal_pending() when the signal handler 
> > > executes, but it's automatically blocked so no signal handler recursion 
> > > occurs. (A sticky signal could still be cleared via a separate mechanism, by 
> > > the cancellation cleanup code.)
> > > 
> > > Such a 'sticky cancellation signal' would, in the racy situation, cause new 
> > > blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
> > > could still be used. (So the cancellation signal handler itself would still 
> > > have access to various fundamental system calls.)
> > > 
> > > I think this would avoid messy coupling between the kernel's increasingly more 
> > > varied system call entry code and C libraries.
> > > 
> > > Sticky signals could be requested via a new SA_ flag.
> > > 
> > > What do you think?
> > 
> > This still doesn't address the issue that the code making the syscall needs to 
> > be able to control whether it's cancellable or not. Not only do some syscalls 
> > whose public functions are cancellation points need to be used internally in 
> > non-cancellable ways; there's also the pthread_setcancelstate interface that 
> > allows deferring cancellation so that it's possible to call functions which are 
> > cancellation points without invoking cancellation.
> 
> I don't think there's a problem - but I might be wrong:
> 
> One way I think it would work is the following: a sticky signal is not the 
> cancellation flag - it's a helper construct to implement the flag in user-space in 
> a race-free way.
> 
> Say you have RT signal-32 as the cancellation signal, and it's a sticky signal.
> 
> When pthread_cancel() wants to cancel another thread, it first (atomically) sets 
> the desired cancel state of the target thread. If that state signals that the 
> thread is cancellable right now, and that we initiated its cancellation, then we 
> send signal-32. I.e. the signal only ever gets sent if the thread is in a 
> cancellable state.
> 
> libc internal functions and the pthread_setcancelstate() API can temporarily 
> change the cancel state of a thread to non-cancellable - but pthread_cancel() 
> observes those state transitions.
> 
> The 'sticky' nature of signal-32 will make a difference in the following race 
> condition, if the cancellation flag is checked before a system call by the C 
> library, and signal-32 arrives before the system call is executed. In that case 
> the 'sticky' nature of the signal makes sure that all subsequent system calls 
> return immediately.
> 
> The sticky signal is only ever sent when the thread is in cancellable state - and 
> if the target thread notices the cancellation request before the signal arrives, 
> it first waits for its arrival before executing any new system calls (as part of 
> the teardown, etc.).
> 
> So the C library never has to do complex work with a sticky signal pending.
> 
> Does that make more sense to you?

No, it doesn't work. Cancellability of the target thread at the time
of the cancellation request (when you would decide whether or not to
send the signal) has no relation to cancellability at the time of
calling the cancellation point. Consider 2 threads A and B and the
following sequence of events:

1. A has cancellation enabled
2. B calls pthread_cancel(A) and sets sticky pending signal
3. A disables cancellation
4. A calls cancellation point and syscall wrongly gets interrupted

This can be solved with more synchronization in pthread_cancel and
pthread_setcancelstate, but it seems costly. pthread_setcancelstate
would have to clear pending sticky cancellation signals, and any
internal non-cancellable syscalls would have to be made using the same
mechanism (effectively calling pthread_setcancelstate). A naive
implementation of such clearing would involve a syscall itself,
defeating the purpose of using the vdso syscall at all (since an extra
syscall costs a lot more than the cycles you save from sysenter vs int
$0x80). It should be possible to track the state of the pending signal
in userspace, so that syscalls to clear it can be avoided except when
it's actually pending, but this requires some very tricky locking to
implement since most of these syscalls have to be async-signal-safe
but would also need to be using locking that synchronizes with the
thread calling pthread_cancel. At worst, implementing such locking
would require blocking all signals before taking the lock, which would
again introduce the requirement of more syscalls.

> > From my standpoint the simplest and cleanest solution is for vdso to provide a 
> > predicate function that takes a ucontext_t and returns true/false for whether it 
> > represents a state prior to entering (or reentering, for restart state) the vdso 
> > syscall. If vdso exports this symbol libc can use vdso syscall with 
> > cancellation. If not, it can just fallback to straight inline syscall like now.
> 
> Offering such a flag pushes unreasonable conceptual overhead into the vDSO proper 
> in the long run: right now it might be easy to implement because the code paths 
> are relatively simple and we can generate the flag passively via RIP checking - 
> but if the vDSO grows more complex interfaces in the future, we'd essentially have 
> to track our entry/exit state dynamically which sucks ...

I don't see what you think it would grow. We're not talking about all
functionality in the vdso, only the vdso syscall/sysenter replacement
(AT_SYSINFO) to be used in place of int $0x80. The only way it would
get more complex is if whole syscalls were being fast-pathed in
userspace, but I think it was already determined that this approach
was wrong and that the vdso should export public symbols (like
__vdso_clock_gettime) instead of transparently fast-pathing them in
userspace via the AT_SYSINFO function. Also, any function that would
be a candidate for fast-pathing in userspace would be a
non-cancellation-point anyway.

Rich

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

#1355502 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Rich Felker <dalias@libc.org> [2016-03-10 18:28:20 -0500]:
> On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
> > 
> > * Rich Felker <dalias@libc.org> wrote:
> > 
> > > > So instead of a sticky cancellation flag, we could introduce a sticky 
> > > > cancellation signal.
> > > > 
> > > > A 'sticky signal' is not cleared from signal_pending() when the signal handler 
> > > > executes, but it's automatically blocked so no signal handler recursion 
> > > > occurs. (A sticky signal could still be cleared via a separate mechanism, by 
> > > > the cancellation cleanup code.)
> > > > 
> > > > Such a 'sticky cancellation signal' would, in the racy situation, cause new 
> > > > blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
> > > > could still be used. (So the cancellation signal handler itself would still 
> > > > have access to various fundamental system calls.)
> > > > 
> > > > I think this would avoid messy coupling between the kernel's increasingly more 
> > > > varied system call entry code and C libraries.
> > > > 
> > > > Sticky signals could be requested via a new SA_ flag.
> > > > 
> > > > What do you think?
> > > 
> > > This still doesn't address the issue that the code making the syscall needs to 
> > > be able to control whether it's cancellable or not. Not only do some syscalls 
> > > whose public functions are cancellation points need to be used internally in 
> > > non-cancellable ways; there's also the pthread_setcancelstate interface that 
> > > allows deferring cancellation so that it's possible to call functions which are 
> > > cancellation points without invoking cancellation.
> > 
> > I don't think there's a problem - but I might be wrong:
> > 
> > One way I think it would work is the following: a sticky signal is not the 
> > cancellation flag - it's a helper construct to implement the flag in user-space in 
> > a race-free way.
> > 
> > Say you have RT signal-32 as the cancellation signal, and it's a sticky signal.
> > 
> > When pthread_cancel() wants to cancel another thread, it first (atomically) sets 
> > the desired cancel state of the target thread. If that state signals that the 
> > thread is cancellable right now, and that we initiated its cancellation, then we 
> > send signal-32. I.e. the signal only ever gets sent if the thread is in a 
> > cancellable state.
> > 
> > libc internal functions and the pthread_setcancelstate() API can temporarily 
> > change the cancel state of a thread to non-cancellable - but pthread_cancel() 
> > observes those state transitions.
> > 
> > The 'sticky' nature of signal-32 will make a difference in the following race 
> > condition, if the cancellation flag is checked before a system call by the C 
> > library, and signal-32 arrives before the system call is executed. In that case 
> > the 'sticky' nature of the signal makes sure that all subsequent system calls 
> > return immediately.
> > 
> > The sticky signal is only ever sent when the thread is in cancellable state - and 
> > if the target thread notices the cancellation request before the signal arrives, 
> > it first waits for its arrival before executing any new system calls (as part of 
> > the teardown, etc.).
> > 
> > So the C library never has to do complex work with a sticky signal pending.
> > 
> > Does that make more sense to you?
> 
> No, it doesn't work. Cancellability of the target thread at the time
> of the cancellation request (when you would decide whether or not to
> send the signal) has no relation to cancellability at the time of
> calling the cancellation point. Consider 2 threads A and B and the
> following sequence of events:
> 
> 1. A has cancellation enabled
> 2. B calls pthread_cancel(A) and sets sticky pending signal
> 3. A disables cancellation
> 4. A calls cancellation point and syscall wrongly gets interrupted
> 
> This can be solved with more synchronization in pthread_cancel and
> pthread_setcancelstate, but it seems costly. pthread_setcancelstate
> would have to clear pending sticky cancellation signals, and any
> internal non-cancellable syscalls would have to be made using the same
> mechanism (effectively calling pthread_setcancelstate). A naive
> implementation of such clearing would involve a syscall itself,

i think a syscall in setcancelstate in case of pending sticky signal
is not that bad given that cancellation is very rarely used.

however maintaining two completely different cancellation designs
is expensive and only the current one works on old kernels.

> defeating the purpose of using the vdso syscall at all (since an extra
> syscall costs a lot more than the cycles you save from sysenter vs int
> $0x80). It should be possible to track the state of the pending signal
> in userspace, so that syscalls to clear it can be avoided except when
> it's actually pending, but this requires some very tricky locking to
> implement since most of these syscalls have to be async-signal-safe
> but would also need to be using locking that synchronizes with the
> thread calling pthread_cancel. At worst, implementing such locking
> would require blocking all signals before taking the lock, which would
> again introduce the requirement of more syscalls.
> 
> > > From my standpoint the simplest and cleanest solution is for vdso to provide a 
> > > predicate function that takes a ucontext_t and returns true/false for whether it 
> > > represents a state prior to entering (or reentering, for restart state) the vdso 
> > > syscall. If vdso exports this symbol libc can use vdso syscall with 
> > > cancellation. If not, it can just fallback to straight inline syscall like now.
> > 
> > Offering such a flag pushes unreasonable conceptual overhead into the vDSO proper 
> > in the long run: right now it might be easy to implement because the code paths 
> > are relatively simple and we can generate the flag passively via RIP checking - 
> > but if the vDSO grows more complex interfaces in the future, we'd essentially have 
> > to track our entry/exit state dynamically which sucks ...
> 
> I don't see what you think it would grow. We're not talking about all
> functionality in the vdso, only the vdso syscall/sysenter replacement
> (AT_SYSINFO) to be used in place of int $0x80. The only way it would
> get more complex is if whole syscalls were being fast-pathed in
> userspace, but I think it was already determined that this approach
> was wrong and that the vdso should export public symbols (like
> __vdso_clock_gettime) instead of transparently fast-pathing them in
> userspace via the AT_SYSINFO function. Also, any function that would
> be a candidate for fast-pathing in userspace would be a
> non-cancellation-point anyway.
> 
> Rich

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

#1355516 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Fri, Mar 11, 2016 at 01:18:54AM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2016-03-10 18:28:20 -0500]:
> > On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
> > > 
> > > * Rich Felker <dalias@libc.org> wrote:
> > > 
> > > > > So instead of a sticky cancellation flag, we could introduce a sticky 
> > > > > cancellation signal.
> > > > > 
> > > > > A 'sticky signal' is not cleared from signal_pending() when the signal handler 
> > > > > executes, but it's automatically blocked so no signal handler recursion 
> > > > > occurs. (A sticky signal could still be cleared via a separate mechanism, by 
> > > > > the cancellation cleanup code.)
> > > > > 
> > > > > Such a 'sticky cancellation signal' would, in the racy situation, cause new 
> > > > > blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
> > > > > could still be used. (So the cancellation signal handler itself would still 
> > > > > have access to various fundamental system calls.)
> > > > > 
> > > > > I think this would avoid messy coupling between the kernel's increasingly more 
> > > > > varied system call entry code and C libraries.
> > > > > 
> > > > > Sticky signals could be requested via a new SA_ flag.
> > > > > 
> > > > > What do you think?
> > > > 
> > > > This still doesn't address the issue that the code making the syscall needs to 
> > > > be able to control whether it's cancellable or not. Not only do some syscalls 
> > > > whose public functions are cancellation points need to be used internally in 
> > > > non-cancellable ways; there's also the pthread_setcancelstate interface that 
> > > > allows deferring cancellation so that it's possible to call functions which are 
> > > > cancellation points without invoking cancellation.
> > > 
> > > I don't think there's a problem - but I might be wrong:
> > > 
> > > One way I think it would work is the following: a sticky signal is not the 
> > > cancellation flag - it's a helper construct to implement the flag in user-space in 
> > > a race-free way.
> > > 
> > > Say you have RT signal-32 as the cancellation signal, and it's a sticky signal.
> > > 
> > > When pthread_cancel() wants to cancel another thread, it first (atomically) sets 
> > > the desired cancel state of the target thread. If that state signals that the 
> > > thread is cancellable right now, and that we initiated its cancellation, then we 
> > > send signal-32. I.e. the signal only ever gets sent if the thread is in a 
> > > cancellable state.
> > > 
> > > libc internal functions and the pthread_setcancelstate() API can temporarily 
> > > change the cancel state of a thread to non-cancellable - but pthread_cancel() 
> > > observes those state transitions.
> > > 
> > > The 'sticky' nature of signal-32 will make a difference in the following race 
> > > condition, if the cancellation flag is checked before a system call by the C 
> > > library, and signal-32 arrives before the system call is executed. In that case 
> > > the 'sticky' nature of the signal makes sure that all subsequent system calls 
> > > return immediately.
> > > 
> > > The sticky signal is only ever sent when the thread is in cancellable state - and 
> > > if the target thread notices the cancellation request before the signal arrives, 
> > > it first waits for its arrival before executing any new system calls (as part of 
> > > the teardown, etc.).
> > > 
> > > So the C library never has to do complex work with a sticky signal pending.
> > > 
> > > Does that make more sense to you?
> > 
> > No, it doesn't work. Cancellability of the target thread at the time
> > of the cancellation request (when you would decide whether or not to
> > send the signal) has no relation to cancellability at the time of
> > calling the cancellation point. Consider 2 threads A and B and the
> > following sequence of events:
> > 
> > 1. A has cancellation enabled
> > 2. B calls pthread_cancel(A) and sets sticky pending signal
> > 3. A disables cancellation
> > 4. A calls cancellation point and syscall wrongly gets interrupted
> > 
> > This can be solved with more synchronization in pthread_cancel and
> > pthread_setcancelstate, but it seems costly. pthread_setcancelstate
> > would have to clear pending sticky cancellation signals, and any
> > internal non-cancellable syscalls would have to be made using the same
> > mechanism (effectively calling pthread_setcancelstate). A naive
> > implementation of such clearing would involve a syscall itself,
> 
> i think a syscall in setcancelstate in case of pending sticky signal
> is not that bad given that cancellation is very rarely used.

I agree, but it's not clear to me whether you could eliminate syscalls
in the case where it's not pending, since AS-safe lock machinery is
hard to get right. I don't see a way it can be done with just atomics
because the syscall that sends the signal cannot be atomic with the
memory operating setting a flag, which suggests a lock is needed, and
then there are all sorts of issues to deal with.

> however maintaining two completely different cancellation designs
> is expensive and only the current one works on old kernels.

Indeed. I think it would be hard to justify supporting a new one in
musl unless there's some easy way to isolate the complexity of having
both, being that vdso syscall is of marginal value to begin with
anyway...

Rich

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

#1355523 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Thu, Mar 10, 2016 at 4:48 PM, Rich Felker <dalias@libc.org> wrote:
> On Fri, Mar 11, 2016 at 01:18:54AM +0100, Szabolcs Nagy wrote:
>> * Rich Felker <dalias@libc.org> [2016-03-10 18:28:20 -0500]:
>> > On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
>> > >
>> > > * Rich Felker <dalias@libc.org> wrote:
>> > >
>> > > > > So instead of a sticky cancellation flag, we could introduce a sticky
>> > > > > cancellation signal.
>> > > > >
>> > > > > A 'sticky signal' is not cleared from signal_pending() when the signal handler
>> > > > > executes, but it's automatically blocked so no signal handler recursion
>> > > > > occurs. (A sticky signal could still be cleared via a separate mechanism, by
>> > > > > the cancellation cleanup code.)
>> > > > >
>> > > > > Such a 'sticky cancellation signal' would, in the racy situation, cause new
>> > > > > blocking system calls to immediately return with -EINTR. Non-blocking syscalls
>> > > > > could still be used. (So the cancellation signal handler itself would still
>> > > > > have access to various fundamental system calls.)
>> > > > >
>> > > > > I think this would avoid messy coupling between the kernel's increasingly more
>> > > > > varied system call entry code and C libraries.
>> > > > >
>> > > > > Sticky signals could be requested via a new SA_ flag.
>> > > > >
>> > > > > What do you think?
>> > > >
>> > > > This still doesn't address the issue that the code making the syscall needs to
>> > > > be able to control whether it's cancellable or not. Not only do some syscalls
>> > > > whose public functions are cancellation points need to be used internally in
>> > > > non-cancellable ways; there's also the pthread_setcancelstate interface that
>> > > > allows deferring cancellation so that it's possible to call functions which are
>> > > > cancellation points without invoking cancellation.
>> > >
>> > > I don't think there's a problem - but I might be wrong:
>> > >
>> > > One way I think it would work is the following: a sticky signal is not the
>> > > cancellation flag - it's a helper construct to implement the flag in user-space in
>> > > a race-free way.
>> > >
>> > > Say you have RT signal-32 as the cancellation signal, and it's a sticky signal.
>> > >
>> > > When pthread_cancel() wants to cancel another thread, it first (atomically) sets
>> > > the desired cancel state of the target thread. If that state signals that the
>> > > thread is cancellable right now, and that we initiated its cancellation, then we
>> > > send signal-32. I.e. the signal only ever gets sent if the thread is in a
>> > > cancellable state.
>> > >
>> > > libc internal functions and the pthread_setcancelstate() API can temporarily
>> > > change the cancel state of a thread to non-cancellable - but pthread_cancel()
>> > > observes those state transitions.
>> > >
>> > > The 'sticky' nature of signal-32 will make a difference in the following race
>> > > condition, if the cancellation flag is checked before a system call by the C
>> > > library, and signal-32 arrives before the system call is executed. In that case
>> > > the 'sticky' nature of the signal makes sure that all subsequent system calls
>> > > return immediately.
>> > >
>> > > The sticky signal is only ever sent when the thread is in cancellable state - and
>> > > if the target thread notices the cancellation request before the signal arrives,
>> > > it first waits for its arrival before executing any new system calls (as part of
>> > > the teardown, etc.).
>> > >
>> > > So the C library never has to do complex work with a sticky signal pending.
>> > >
>> > > Does that make more sense to you?
>> >
>> > No, it doesn't work. Cancellability of the target thread at the time
>> > of the cancellation request (when you would decide whether or not to
>> > send the signal) has no relation to cancellability at the time of
>> > calling the cancellation point. Consider 2 threads A and B and the
>> > following sequence of events:
>> >
>> > 1. A has cancellation enabled
>> > 2. B calls pthread_cancel(A) and sets sticky pending signal
>> > 3. A disables cancellation
>> > 4. A calls cancellation point and syscall wrongly gets interrupted
>> >
>> > This can be solved with more synchronization in pthread_cancel and
>> > pthread_setcancelstate, but it seems costly. pthread_setcancelstate
>> > would have to clear pending sticky cancellation signals, and any
>> > internal non-cancellable syscalls would have to be made using the same
>> > mechanism (effectively calling pthread_setcancelstate). A naive
>> > implementation of such clearing would involve a syscall itself,
>>
>> i think a syscall in setcancelstate in case of pending sticky signal
>> is not that bad given that cancellation is very rarely used.
>
> I agree, but it's not clear to me whether you could eliminate syscalls
> in the case where it's not pending, since AS-safe lock machinery is
> hard to get right. I don't see a way it can be done with just atomics
> because the syscall that sends the signal cannot be atomic with the
> memory operating setting a flag, which suggests a lock is needed, and
> then there are all sorts of issues to deal with.
>
>> however maintaining two completely different cancellation designs
>> is expensive and only the current one works on old kernels.
>
> Indeed. I think it would be hard to justify supporting a new one in
> musl unless there's some easy way to isolate the complexity of having
> both, being that vdso syscall is of marginal value to begin with
> anyway...

I would argue that vdso syscalls are of considerably more than
marginal utility.  They are vastly faster.  The difference isn't
subtle.

However... while it seems straightforward that a pthread cancellation
implementation should be correct, is there any reason that it needs to
fast?  After all musl could always do:

if (this thread is cancellable right now)
  use int $0x80 and eat the performance hit
else
  call AT_SYSINFO

Aside from a branch, this adds minimal overhead to sane programs, and
programs crazy enough to try use pthread cancellation get penalized on
x86_32.

If I read it right, that's what musl already does.  But I could be
reading it wrong:

if ((st=(self=__pthread_self())->canceldisable)
   && (st==PTHREAD_CANCEL_DISABLE || nr==SYS_close))
return __syscall(nr, u, v, w, x, y, z);

... slow path ...

Figuring out what "(st=(self=__pthread_self())->canceldisable) && ..."
does and why requires more cross-referencing that I care to do right
now.  Shouldn't that "&&" at least be a "," or perhaps just be a
separate statement?  On glibc, at least, PTHREAD_CANCEL_DISABLE == 1,
so this looks nearly tautological.

--Andy

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

#1355526 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Rich Felker <dalias@libc.org> [2016-03-10 19:48:59 -0500]:
> On Fri, Mar 11, 2016 at 01:18:54AM +0100, Szabolcs Nagy wrote:
> > * Rich Felker <dalias@libc.org> [2016-03-10 18:28:20 -0500]:
> > > On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
> > > > 
> > > > The sticky signal is only ever sent when the thread is in cancellable state - and 
> > > > if the target thread notices the cancellation request before the signal arrives, 
        ^^^^^^...
> > > > it first waits for its arrival before executing any new system calls (as part of 
        ^^^^^^...
> > > > the teardown, etc.).
> > > > 
> > > > So the C library never has to do complex work with a sticky signal pending.
> > > > 
> > > > Does that make more sense to you?
> > > 
> > > No, it doesn't work. Cancellability of the target thread at the time
> > > of the cancellation request (when you would decide whether or not to
> > > send the signal) has no relation to cancellability at the time of
> > > calling the cancellation point. Consider 2 threads A and B and the
> > > following sequence of events:
> > > 
> > > 1. A has cancellation enabled
> > > 2. B calls pthread_cancel(A) and sets sticky pending signal
> > > 3. A disables cancellation
> > > 4. A calls cancellation point and syscall wrongly gets interrupted
> > > 
> > > This can be solved with more synchronization in pthread_cancel and
> > > pthread_setcancelstate, but it seems costly. pthread_setcancelstate
> > > would have to clear pending sticky cancellation signals, and any
> > > internal non-cancellable syscalls would have to be made using the same
> > > mechanism (effectively calling pthread_setcancelstate). A naive
> > > implementation of such clearing would involve a syscall itself,
> > 
> > i think a syscall in setcancelstate in case of pending sticky signal
> > is not that bad given that cancellation is very rarely used.
> 
> I agree, but it's not clear to me whether you could eliminate syscalls
> in the case where it's not pending, since AS-safe lock machinery is
> hard to get right. I don't see a way it can be done with just atomics
> because the syscall that sends the signal cannot be atomic with the
> memory operating setting a flag, which suggests a lock is needed, and
> then there are all sorts of issues to deal with.
> 

i think this is not a problem and the above marked text hints for
a solution: just call pause() to wait for the sticky signal if
self->cancelstate indicates that there is one comming or pending.

t->cancelstate always have to be atomically modified but sending
the sticky signal can be delayed (does not have to be atomic with
the memory op).

(of course there migth be other caveats and it certainly needs
more atomic ops and more state than the current design)

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

#1355531 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

On Fri, Mar 11, 2016 at 02:39:47AM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2016-03-10 19:48:59 -0500]:
> > On Fri, Mar 11, 2016 at 01:18:54AM +0100, Szabolcs Nagy wrote:
> > > * Rich Felker <dalias@libc.org> [2016-03-10 18:28:20 -0500]:
> > > > On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
> > > > > 
> > > > > The sticky signal is only ever sent when the thread is in cancellable state - and 
> > > > > if the target thread notices the cancellation request before the signal arrives, 
>         ^^^^^^...
> > > > > it first waits for its arrival before executing any new system calls (as part of 
>         ^^^^^^...
> > > > > the teardown, etc.).
> > > > > 
> > > > > So the C library never has to do complex work with a sticky signal pending.
> > > > > 
> > > > > Does that make more sense to you?
> > > > 
> > > > No, it doesn't work. Cancellability of the target thread at the time
> > > > of the cancellation request (when you would decide whether or not to
> > > > send the signal) has no relation to cancellability at the time of
> > > > calling the cancellation point. Consider 2 threads A and B and the
> > > > following sequence of events:
> > > > 
> > > > 1. A has cancellation enabled
> > > > 2. B calls pthread_cancel(A) and sets sticky pending signal
> > > > 3. A disables cancellation
> > > > 4. A calls cancellation point and syscall wrongly gets interrupted
> > > > 
> > > > This can be solved with more synchronization in pthread_cancel and
> > > > pthread_setcancelstate, but it seems costly. pthread_setcancelstate
> > > > would have to clear pending sticky cancellation signals, and any
> > > > internal non-cancellable syscalls would have to be made using the same
> > > > mechanism (effectively calling pthread_setcancelstate). A naive
> > > > implementation of such clearing would involve a syscall itself,
> > > 
> > > i think a syscall in setcancelstate in case of pending sticky signal
> > > is not that bad given that cancellation is very rarely used.
> > 
> > I agree, but it's not clear to me whether you could eliminate syscalls
> > in the case where it's not pending, since AS-safe lock machinery is
> > hard to get right. I don't see a way it can be done with just atomics
> > because the syscall that sends the signal cannot be atomic with the
> > memory operating setting a flag, which suggests a lock is needed, and
> > then there are all sorts of issues to deal with.
> 
> i think this is not a problem and the above marked text hints for
> a solution: just call pause() to wait for the sticky signal if
> self->cancelstate indicates that there is one comming or pending.

There are multiple problems with this approach, at least:

- pause does not 'consume' the signal; sigwaitinfo might.

- pause might return on a different signal that happens to arrive
  between setting the flag and sending the cancel signal

- If the thread calling pthread_cancel is interrupted by a signal
  after setting the flag but before sending the signal, the target
  thread may be arbitrarily delayed; in complex cases it may even
  deadlock. This should be easy to solve though by having
  pthread_cancel run with signals masked.

> t->cancelstate always have to be atomically modified but sending
> the sticky signal can be delayed (does not have to be atomic with
> the memory op).

Right.

> (of course there migth be other caveats and it certainly needs
> more atomic ops and more state than the current design)

I think it might be possible to do by having pthread_cancel run with
signals blocked and having sigwaitinfo consume the sticky signal if
the atomic-set cancellation-pending flag was seen, but I haven't
thought about all the corner cases of signal handlers and nested
cancellation points. POSIX might be making the behavior of the
affected cases undefined, though. So I think solving this might be
plausible, but nontrivial.

Rich

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

#1355534 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Szabolcs Nagy <nsz@port70.net> [2016-03-11 02:39:47 +0100]:
> * Rich Felker <dalias@libc.org> [2016-03-10 19:48:59 -0500]:
> > On Fri, Mar 11, 2016 at 01:18:54AM +0100, Szabolcs Nagy wrote:
> > > * Rich Felker <dalias@libc.org> [2016-03-10 18:28:20 -0500]:
> > > > On Thu, Mar 10, 2016 at 07:03:31PM +0100, Ingo Molnar wrote:
> > > > > 
> > > > > The sticky signal is only ever sent when the thread is in cancellable state - and 
> > > > > if the target thread notices the cancellation request before the signal arrives, 
>         ^^^^^^...
> > > > > it first waits for its arrival before executing any new system calls (as part of 
>         ^^^^^^...
> > > > > the teardown, etc.).
> > > > > 
> > > > > So the C library never has to do complex work with a sticky signal pending.
> > > > > 
> > > > > Does that make more sense to you?
> > > > 
> > > > No, it doesn't work. Cancellability of the target thread at the time
> > > > of the cancellation request (when you would decide whether or not to
> > > > send the signal) has no relation to cancellability at the time of
> > > > calling the cancellation point. Consider 2 threads A and B and the
> > > > following sequence of events:
> > > > 
> > > > 1. A has cancellation enabled
> > > > 2. B calls pthread_cancel(A) and sets sticky pending signal
> > > > 3. A disables cancellation
> > > > 4. A calls cancellation point and syscall wrongly gets interrupted
> > > > 
> > > > This can be solved with more synchronization in pthread_cancel and
> > > > pthread_setcancelstate, but it seems costly. pthread_setcancelstate
> > > > would have to clear pending sticky cancellation signals, and any
> > > > internal non-cancellable syscalls would have to be made using the same
> > > > mechanism (effectively calling pthread_setcancelstate). A naive
> > > > implementation of such clearing would involve a syscall itself,
> > > 
> > > i think a syscall in setcancelstate in case of pending sticky signal
> > > is not that bad given that cancellation is very rarely used.
> > 
> > I agree, but it's not clear to me whether you could eliminate syscalls
> > in the case where it's not pending, since AS-safe lock machinery is
> > hard to get right. I don't see a way it can be done with just atomics
> > because the syscall that sends the signal cannot be atomic with the
> > memory operating setting a flag, which suggests a lock is needed, and
> > then there are all sorts of issues to deal with.
> > 
> 
> i think this is not a problem and the above marked text hints for
> a solution: just call pause() to wait for the sticky signal if
> self->cancelstate indicates that there is one comming or pending.
> 
> t->cancelstate always have to be atomically modified but sending
> the sticky signal can be delayed (does not have to be atomic with
> the memory op).
> 

i take this back, if there are signals between the check of
self->cancelstate and pause() in setcancelstate that can
cause problems (the sticky signal will not hit pause but
something else).

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

#1355766 — Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

* Rich Felker <dalias@libc.org> wrote:

> No, it doesn't work. Cancellability of the target thread at the time
> of the cancellation request (when you would decide whether or not to
> send the signal) has no relation to cancellability at the time of
> calling the cancellation point. Consider 2 threads A and B and the
> following sequence of events:
> 
> 1. A has cancellation enabled
> 2. B calls pthread_cancel(A) and sets sticky pending signal
> 3. A disables cancellation
> 4. A calls cancellation point and syscall wrongly gets interrupted

As I (tried to!) describe it when describing the cancellation signal, if a 
cancellation signal is in flight, it must be waited for in the unlikely event of 
cancellation being disabled in the small window where the signal is sent.

So in your above example, it would do:

> 1. A has cancellation enabled
> 2. B calls pthread_cancel(A) and sets sticky pending signal
> 3. A disables cancellation
  3b. Notices that cancellation request is pending and waits for it
      and clears the sticky signal.
  4. A calls cancellation point and syscall correctly executes
  5. Once A enables cancellation again, the cancellation propagates.

So I still see no problem.

> This can be solved with more synchronization in pthread_cancel and 
> pthread_setcancelstate, but it seems costly. [...]

An active signal round trip in itself is very costly (thousands of cycles), a 
thread exit is tens of thousands of cycles, and this is a 'slow path' anyway, and 
the window is small in any case.

It's just a correctness synchronization to make sure no sticky signal is pending, 
not a real performance concern in practice.

Thanks,

	Ingo

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web