Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH] x86: Deprecate a.out support
From: Linus Torvalds @ 2019-03-05 16:22 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alan Cox, Matthew Wilcox, Jann Horn, Al Viro, Thomas Gleixner,
	kernel list, linux-fsdevel, the arch/x86 maintainers, Linux API,
	Andrew Morton, Richard Weinberger, Anton Ivanov
In-Reply-To: <20190305145717.GD8256@zn.tnic>

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]

On Tue, Mar 5, 2019 at 6:59 AM Borislav Petkov <bp@alien8.de> wrote:
>
> We can at least deprecate it on x86...

I'd prefer to try to deprecate a.out core dumping first.. That's the
part that is actually broken, no?

In fact, I'd be happy to deprecate a.out entirely, but if somebody
_does_ complain, I'd like to be able to bring it back without the core
dumping.

Because I think the likeliihood that anybody cares about a.out core
dumps is basically zero. While the likelihood that we have some odd
old binary that is still a.out is slightly above zero.

So I'd be much happier with this if it was a two-stage thing where we
just delete a.out core dumping entirely first, and then deprecate even
running a.out binaries separately.

Because I think all the known *bugs* we had were with the core dumping
code, weren't they?

Removing it looks trivial. Untested patch attached.

Then I'd be much happier with your "let's deprecate a.out entirely" as
a second patch, because I think it's a unrelated issue and much more
likely to have somebody pipe up and say "hey, I have this sequence
that generates executables dynamically, and I use a.out because it's
much simpler than ELF, and now it's broken". Or something.

           Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 8506 bytes --]

 arch/x86/ia32/ia32_aout.c | 159 ----------------------------------------------
 fs/binfmt_aout.c          |  82 ------------------------
 2 files changed, 241 deletions(-)

diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index 7dbbe9ffda17..3c135084e1eb 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -39,82 +39,10 @@
 static int load_aout_binary(struct linux_binprm *);
 static int load_aout_library(struct file *);
 
-#ifdef CONFIG_COREDUMP
-static int aout_core_dump(struct coredump_params *);
-
-static unsigned long get_dr(int n)
-{
-	struct perf_event *bp = current->thread.ptrace_bps[n];
-	return bp ? bp->hw.info.address : 0;
-}
-
-/*
- * fill in the user structure for a core dump..
- */
-static void fill_dump(struct pt_regs *regs, struct user32 *dump)
-{
-	u32 fs, gs;
-	memset(dump, 0, sizeof(*dump));
-
-/* changed the size calculations - should hopefully work better. lbt */
-	dump->magic = CMAGIC;
-	dump->start_code = 0;
-	dump->start_stack = regs->sp & ~(PAGE_SIZE - 1);
-	dump->u_tsize = ((unsigned long) current->mm->end_code) >> PAGE_SHIFT;
-	dump->u_dsize = ((unsigned long)
-			 (current->mm->brk + (PAGE_SIZE-1))) >> PAGE_SHIFT;
-	dump->u_dsize -= dump->u_tsize;
-	dump->u_debugreg[0] = get_dr(0);
-	dump->u_debugreg[1] = get_dr(1);
-	dump->u_debugreg[2] = get_dr(2);
-	dump->u_debugreg[3] = get_dr(3);
-	dump->u_debugreg[6] = current->thread.debugreg6;
-	dump->u_debugreg[7] = current->thread.ptrace_dr7;
-
-	if (dump->start_stack < 0xc0000000) {
-		unsigned long tmp;
-
-		tmp = (unsigned long) (0xc0000000 - dump->start_stack);
-		dump->u_ssize = tmp >> PAGE_SHIFT;
-	}
-
-	dump->regs.ebx = regs->bx;
-	dump->regs.ecx = regs->cx;
-	dump->regs.edx = regs->dx;
-	dump->regs.esi = regs->si;
-	dump->regs.edi = regs->di;
-	dump->regs.ebp = regs->bp;
-	dump->regs.eax = regs->ax;
-	dump->regs.ds = current->thread.ds;
-	dump->regs.es = current->thread.es;
-	savesegment(fs, fs);
-	dump->regs.fs = fs;
-	savesegment(gs, gs);
-	dump->regs.gs = gs;
-	dump->regs.orig_eax = regs->orig_ax;
-	dump->regs.eip = regs->ip;
-	dump->regs.cs = regs->cs;
-	dump->regs.eflags = regs->flags;
-	dump->regs.esp = regs->sp;
-	dump->regs.ss = regs->ss;
-
-#if 1 /* FIXME */
-	dump->u_fpvalid = 0;
-#else
-	dump->u_fpvalid = dump_fpu(regs, &dump->i387);
-#endif
-}
-
-#endif
-
 static struct linux_binfmt aout_format = {
 	.module		= THIS_MODULE,
 	.load_binary	= load_aout_binary,
 	.load_shlib	= load_aout_library,
-#ifdef CONFIG_COREDUMP
-	.core_dump	= aout_core_dump,
-#endif
-	.min_coredump	= PAGE_SIZE
 };
 
 static int set_brk(unsigned long start, unsigned long end)
@@ -126,93 +54,6 @@ static int set_brk(unsigned long start, unsigned long end)
 	return vm_brk(start, end - start);
 }
 
-#ifdef CONFIG_COREDUMP
-/*
- * These are the only things you should do on a core-file: use only these
- * macros to write out all the necessary info.
- */
-
-#include <linux/coredump.h>
-
-#define START_DATA(u)	(u.u_tsize << PAGE_SHIFT)
-#define START_STACK(u)	(u.start_stack)
-
-/*
- * Routine writes a core dump image in the current directory.
- * Currently only a stub-function.
- *
- * Note that setuid/setgid files won't make a core-dump if the uid/gid
- * changed due to the set[u|g]id. It's enforced by the "current->mm->dumpable"
- * field, which also makes sure the core-dumps won't be recursive if the
- * dumping of the process results in another error..
- */
-
-static int aout_core_dump(struct coredump_params *cprm)
-{
-	mm_segment_t fs;
-	int has_dumped = 0;
-	unsigned long dump_start, dump_size;
-	struct user32 dump;
-
-	fs = get_fs();
-	set_fs(KERNEL_DS);
-	has_dumped = 1;
-
-	fill_dump(cprm->regs, &dump);
-
-	strncpy(dump.u_comm, current->comm, sizeof(current->comm));
-	dump.u_ar0 = offsetof(struct user32, regs);
-	dump.signal = cprm->siginfo->si_signo;
-
-	/*
-	 * If the size of the dump file exceeds the rlimit, then see
-	 * what would happen if we wrote the stack, but not the data
-	 * area.
-	 */
-	if ((dump.u_dsize + dump.u_ssize + 1) * PAGE_SIZE > cprm->limit)
-		dump.u_dsize = 0;
-
-	/* Make sure we have enough room to write the stack and data areas. */
-	if ((dump.u_ssize + 1) * PAGE_SIZE > cprm->limit)
-		dump.u_ssize = 0;
-
-	/* make sure we actually have a data and stack area to dump */
-	set_fs(USER_DS);
-	if (!access_ok((void *) (unsigned long)START_DATA(dump),
-		       dump.u_dsize << PAGE_SHIFT))
-		dump.u_dsize = 0;
-	if (!access_ok((void *) (unsigned long)START_STACK(dump),
-		       dump.u_ssize << PAGE_SHIFT))
-		dump.u_ssize = 0;
-
-	set_fs(KERNEL_DS);
-	/* struct user */
-	if (!dump_emit(cprm, &dump, sizeof(dump)))
-		goto end_coredump;
-	/* Now dump all of the user data.  Include malloced stuff as well */
-	if (!dump_skip(cprm, PAGE_SIZE - sizeof(dump)))
-		goto end_coredump;
-	/* now we start writing out the user space info */
-	set_fs(USER_DS);
-	/* Dump the data area */
-	if (dump.u_dsize != 0) {
-		dump_start = START_DATA(dump);
-		dump_size = dump.u_dsize << PAGE_SHIFT;
-		if (!dump_emit(cprm, (void *)dump_start, dump_size))
-			goto end_coredump;
-	}
-	/* Now prepare to dump the stack area */
-	if (dump.u_ssize != 0) {
-		dump_start = START_STACK(dump);
-		dump_size = dump.u_ssize << PAGE_SHIFT;
-		if (!dump_emit(cprm, (void *)dump_start, dump_size))
-			goto end_coredump;
-	}
-end_coredump:
-	set_fs(fs);
-	return has_dumped;
-}
-#endif
 
 /*
  * create_aout_tables() parses the env- and arg-strings in new user
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index ca9725f18e00..aa0f1e53113f 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -34,92 +34,10 @@
 static int load_aout_binary(struct linux_binprm *);
 static int load_aout_library(struct file*);
 
-#ifdef CONFIG_COREDUMP
-/*
- * Routine writes a core dump image in the current directory.
- * Currently only a stub-function.
- *
- * Note that setuid/setgid files won't make a core-dump if the uid/gid
- * changed due to the set[u|g]id. It's enforced by the "current->mm->dumpable"
- * field, which also makes sure the core-dumps won't be recursive if the
- * dumping of the process results in another error..
- */
-static int aout_core_dump(struct coredump_params *cprm)
-{
-	mm_segment_t fs;
-	int has_dumped = 0;
-	void __user *dump_start;
-	int dump_size;
-	struct user dump;
-#ifdef __alpha__
-#       define START_DATA(u)	((void __user *)u.start_data)
-#else
-#	define START_DATA(u)	((void __user *)((u.u_tsize << PAGE_SHIFT) + \
-				 u.start_code))
-#endif
-#       define START_STACK(u)   ((void __user *)u.start_stack)
-
-	fs = get_fs();
-	set_fs(KERNEL_DS);
-	has_dumped = 1;
-       	strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
-	dump.u_ar0 = offsetof(struct user, regs);
-	dump.signal = cprm->siginfo->si_signo;
-	aout_dump_thread(cprm->regs, &dump);
-
-/* If the size of the dump file exceeds the rlimit, then see what would happen
-   if we wrote the stack, but not the data area.  */
-	if ((dump.u_dsize + dump.u_ssize+1) * PAGE_SIZE > cprm->limit)
-		dump.u_dsize = 0;
-
-/* Make sure we have enough room to write the stack and data areas. */
-	if ((dump.u_ssize + 1) * PAGE_SIZE > cprm->limit)
-		dump.u_ssize = 0;
-
-/* make sure we actually have a data and stack area to dump */
-	set_fs(USER_DS);
-	if (!access_ok(START_DATA(dump), dump.u_dsize << PAGE_SHIFT))
-		dump.u_dsize = 0;
-	if (!access_ok(START_STACK(dump), dump.u_ssize << PAGE_SHIFT))
-		dump.u_ssize = 0;
-
-	set_fs(KERNEL_DS);
-/* struct user */
-	if (!dump_emit(cprm, &dump, sizeof(dump)))
-		goto end_coredump;
-/* Now dump all of the user data.  Include malloced stuff as well */
-	if (!dump_skip(cprm, PAGE_SIZE - sizeof(dump)))
-		goto end_coredump;
-/* now we start writing out the user space info */
-	set_fs(USER_DS);
-/* Dump the data area */
-	if (dump.u_dsize != 0) {
-		dump_start = START_DATA(dump);
-		dump_size = dump.u_dsize << PAGE_SHIFT;
-		if (!dump_emit(cprm, dump_start, dump_size))
-			goto end_coredump;
-	}
-/* Now prepare to dump the stack area */
-	if (dump.u_ssize != 0) {
-		dump_start = START_STACK(dump);
-		dump_size = dump.u_ssize << PAGE_SHIFT;
-		if (!dump_emit(cprm, dump_start, dump_size))
-			goto end_coredump;
-	}
-end_coredump:
-	set_fs(fs);
-	return has_dumped;
-}
-#else
-#define aout_core_dump NULL
-#endif
-
 static struct linux_binfmt aout_format = {
 	.module		= THIS_MODULE,
 	.load_binary	= load_aout_binary,
 	.load_shlib	= load_aout_library,
-	.core_dump	= aout_core_dump,
-	.min_coredump	= PAGE_SIZE
 };
 
 #define BAD_ADDR(x)	((unsigned long)(x) >= TASK_SIZE)

^ permalink raw reply related

* Re: [PATCH] x86: Deprecate a.out support
From: Jann Horn @ 2019-03-05 16:30 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Borislav Petkov, Alan Cox, Matthew Wilcox, Al Viro,
	Thomas Gleixner, kernel list, linux-fsdevel,
	the arch/x86 maintainers, Linux API, Andrew Morton,
	Richard Weinberger, Anton Ivanov
In-Reply-To: <CAHk-=wi2E4kugAW7BQwNSVwYHOr=B_9EgzBRMpQ2+1dxAqydDg@mail.gmail.com>

On Tue, Mar 5, 2019 at 5:22 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Mar 5, 2019 at 6:59 AM Borislav Petkov <bp@alien8.de> wrote:
> > We can at least deprecate it on x86...
>
> I'd prefer to try to deprecate a.out core dumping first.. That's the
> part that is actually broken, no?
>
> In fact, I'd be happy to deprecate a.out entirely, but if somebody
> _does_ complain, I'd like to be able to bring it back without the core
> dumping.
>
> Because I think the likeliihood that anybody cares about a.out core
> dumps is basically zero. While the likelihood that we have some odd
> old binary that is still a.out is slightly above zero.
>
> So I'd be much happier with this if it was a two-stage thing where we
> just delete a.out core dumping entirely first, and then deprecate even
> running a.out binaries separately.
>
> Because I think all the known *bugs* we had were with the core dumping
> code, weren't they?
>
> Removing it looks trivial. Untested patch attached.

We might want to also delete the then-unused aout_dump_thread()
implementations from the various arch trees. We probably want to
delete arch/*/include/asm/a.out-core.h, since that seems to be the
only thing these headers do, and the only thing that uses them is
fs/binfmt_aout.c?

^ permalink raw reply

* Re: [PATCH] x86: Deprecate a.out support
From: Borislav Petkov @ 2019-03-05 17:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alan Cox, Matthew Wilcox, Jann Horn, Al Viro, Thomas Gleixner,
	kernel list, linux-fsdevel, the arch/x86 maintainers, Linux API,
	Andrew Morton, Richard Weinberger, Anton Ivanov
In-Reply-To: <CAHk-=wi2E4kugAW7BQwNSVwYHOr=B_9EgzBRMpQ2+1dxAqydDg@mail.gmail.com>

On Tue, Mar 05, 2019 at 08:22:21AM -0800, Linus Torvalds wrote:
> Because I think all the known *bugs* we had were with the core dumping
> code, weren't they?

Well, I'm aware of only this one I fixed but who knows what else has
bitrotten out there through the years...

> Removing it looks trivial. Untested patch attached.
> 
> Then I'd be much happier with your "let's deprecate a.out entirely" as
> a second patch, because I think it's a unrelated issue and much more
> likely to have somebody pipe up and say "hey, I have this sequence
> that generates executables dynamically, and I use a.out because it's
> much simpler than ELF, and now it's broken". Or something.

Hohumm, yap, it all makes sense to me. That's definitely the safer
approach without us having to revert patches later.

So how do you wanna handle this?

I guess the easiest would be if you apply your patch directly now and
add the a.out phase-out strategy we're going for in its commit message
so that people are aware of what we're doing.

This will allow for other arch maintainers to delete
arch/*/include/asm/a.out-core.h, Jann mentions in the other mail, after
-rc1 is cut.

And then I'll carry any other patches through the tip tree ontop of -rc1
too.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH] x86: Deprecate a.out support
From: Borislav Petkov @ 2019-03-05 17:32 UTC (permalink / raw)
  To: Jann Horn
  Cc: Linus Torvalds, Alan Cox, Matthew Wilcox, Al Viro,
	Thomas Gleixner, kernel list, linux-fsdevel,
	the arch/x86 maintainers, Linux API, Andrew Morton,
	Richard Weinberger, Anton Ivanov
In-Reply-To: <CAG48ez0=AWipDy+aDBtxwEs6h8AmiBnfoU7GMi=_yW3sS14MNQ@mail.gmail.com>

On Tue, Mar 05, 2019 at 05:30:13PM +0100, Jann Horn wrote:
> We might want to also delete the then-unused aout_dump_thread()
> implementations from the various arch trees. We probably want to
> delete arch/*/include/asm/a.out-core.h, since that seems to be the
> only thing these headers do, and the only thing that uses them is
> fs/binfmt_aout.c?

Right.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH] x86: Deprecate a.out support
From: Linus Torvalds @ 2019-03-05 17:58 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alan Cox, Matthew Wilcox, Jann Horn, Al Viro, Thomas Gleixner,
	kernel list, linux-fsdevel, the arch/x86 maintainers, Linux API,
	Andrew Morton, Richard Weinberger, Anton Ivanov
In-Reply-To: <20190305173134.GE8256@zn.tnic>

On Tue, Mar 5, 2019 at 9:31 AM Borislav Petkov <bp@alien8.de> wrote:
>
> So how do you wanna handle this?
>
> I guess the easiest would be if you apply your patch directly now and
> add the a.out phase-out strategy we're going for in its commit message
> so that people are aware of what we're doing.

Yup, I'll just do that. I'll fix up my patch to be more complete (ie
the a.out-core.h removal Jann pointed out).

I'll talk about removing a.out support entirely in the commit message.
I'd be ok for that to be a total removal, but maybe there are some
architectures that still have that as a "main" format?

Or I could just apply your patch after removing the core-dumping. Comments?

               Linus

^ permalink raw reply

* Re: [PATCH] x86: Deprecate a.out support
From: Borislav Petkov @ 2019-03-05 18:11 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alan Cox, Matthew Wilcox, Jann Horn, Al Viro, Thomas Gleixner,
	kernel list, linux-fsdevel, the arch/x86 maintainers, Linux API,
	Andrew Morton, Richard Weinberger, Anton Ivanov
In-Reply-To: <CAHk-=wg0bY=RFhD4NW9x++fC0bME=_-9xM3kexGGM3gefHQTyQ@mail.gmail.com>

On Tue, Mar 05, 2019 at 09:58:28AM -0800, Linus Torvalds wrote:
> I'd be ok for that to be a total removal, but maybe there are some
> architectures that still have that as a "main" format?

Well, my limited experience with feature removal says that people only
say something when the feature is *actually* long gone, several releases
have passed by and the removal has finally percolated downstream.

I guess you could Cc arch maintainers with the a.out-core.h removal
patch to see if anyone screams.

> Or I could just apply your patch after removing the core-dumping. Comments?

Sure. And it can be reverted very easily if someone runs out of the
woods later and handwaving about an important a.out use case on x86.
Although x86 should be in ELF-land for a quarter of a decade now.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH] x86: Deprecate a.out support
From: Borislav Petkov @ 2019-03-05 18:18 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alan Cox, Matthew Wilcox, Jann Horn, Al Viro, Thomas Gleixner,
	kernel list, linux-fsdevel, the arch/x86 maintainers, Linux API,
	Andrew Morton, Richard Weinberger, Anton Ivanov, linux-alpha,
	linux-m68k
In-Reply-To: <20190305181138.GG8256@zn.tnic>

On Tue, Mar 05, 2019 at 07:11:38PM +0100, Borislav Petkov wrote:
> I guess you could Cc arch maintainers with the a.out-core.h removal
> patch to see if anyone screams.

And they're like two for which we need confirmation:

$ git ls-files | grep a.out-core.h
arch/alpha/include/asm/a.out-core.h
arch/m68k/include/asm/a.out-core.h
arch/um/include/asm/a.out-core.h
arch/x86/include/asm/a.out-core.h

um and x86 are clear.

Adding alpha and m68k MLs to Cc.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Mathieu Desnoyers @ 2019-03-05 19:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk

Those changes aiming at 5.1 include one comment cleanup, the removal of
the rseq_len field from the task struct which serves no purpose
considering that the struct size is fixed by the ABI, and a selftest
improvement adapting the number of threads to the number of detected
CPUs, which is nicer for smaller systems.

Thanks,

Mathieu

Mathieu Desnoyers (3):
  rseq: cleanup: Reflect removal of event counter in comments
  rseq: cleanup: remove rseq_len from task_struct
  rseq/selftests: Adapt number of threads to the number of detected cpus

 arch/arm/kernel/signal.c                       | 3 +--
 arch/x86/kernel/signal.c                       | 5 +----
 include/linux/sched.h                          | 4 ----
 kernel/rseq.c                                  | 9 +++------
 tools/testing/selftests/rseq/run_param_test.sh | 7 +++++--
 5 files changed, 10 insertions(+), 18 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH for 5.1 1/3] rseq: cleanup: Reflect removal of event counter in comments
From: Mathieu Desnoyers @ 2019-03-05 19:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20190305194755.2602-1-mathieu.desnoyers@efficios.com>

The "event counter" was removed from rseq before it was merged upstream.
However, a few comments in the source code still refer to it. Adapt the
comments to match reality.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
---
 arch/arm/kernel/signal.c | 3 +--
 arch/x86/kernel/signal.c | 5 +----
 kernel/rseq.c            | 3 +--
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 76bb8de6bf6b..be5edfdde558 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -549,8 +549,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 	int ret;
 
 	/*
-	 * Increment event counter and perform fixup for the pre-signal
-	 * frame.
+	 * Perform fixup for the pre-signal frame.
 	 */
 	rseq_signal_deliver(ksig, regs);
 
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 08dfd4c1a4f9..22c233b509da 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -688,10 +688,7 @@ setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
 	sigset_t *set = sigmask_to_save();
 	compat_sigset_t *cset = (compat_sigset_t *) set;
 
-	/*
-	 * Increment event counter and perform fixup for the pre-signal
-	 * frame.
-	 */
+	/* Perform fixup for the pre-signal frame. */
 	rseq_signal_deliver(ksig, regs);
 
 	/* Set up the stack frame */
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 25e9a7b60eba..849afe749131 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -254,8 +254,7 @@ static int rseq_ip_fixup(struct pt_regs *regs)
  * - signal delivery,
  * and return to user-space.
  *
- * This is how we can ensure that the entire rseq critical section,
- * consisting of both the C part and the assembly instruction sequence,
+ * This is how we can ensure that the entire rseq critical section
  * will issue the commit instruction only if executed atomically with
  * respect to other threads scheduled on the same CPU, and with respect
  * to signal handlers.
-- 
2.11.0

^ permalink raw reply related

* [PATCH for 5.1 2/3] rseq: cleanup: remove rseq_len from task_struct
From: Mathieu Desnoyers @ 2019-03-05 19:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20190305194755.2602-1-mathieu.desnoyers@efficios.com>

The rseq system call, when invoked with flags of "0" or
"RSEQ_FLAG_UNREGISTER" values, expects the rseq_len parameter to
be equal to sizeof(struct rseq), which is fixed-size and fixed-layout,
specified in uapi linux/rseq.h.

Expecting a fixed size for rseq_len is a design choice that ensures
multiple libraries and application defining __rseq_abi in the same
process agree on its exact size.

Considering that this size is and will always be the same value, there
is no point in saving this value within task_struct rseq_len. Remove
this field from task_struct.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
 include/linux/sched.h | 4 ----
 kernel/rseq.c         | 6 ++----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f9b43c989577..e355688fa82c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1059,7 +1059,6 @@ struct task_struct {
 
 #ifdef CONFIG_RSEQ
 	struct rseq __user *rseq;
-	u32 rseq_len;
 	u32 rseq_sig;
 	/*
 	 * RmW on rseq_event_mask must be performed atomically
@@ -1852,12 +1851,10 @@ 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;
 		t->rseq_event_mask = 0;
 	} else {
 		t->rseq = current->rseq;
-		t->rseq_len = current->rseq_len;
 		t->rseq_sig = current->rseq_sig;
 		t->rseq_event_mask = current->rseq_event_mask;
 	}
@@ -1866,7 +1863,6 @@ static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
 static inline void rseq_execve(struct task_struct *t)
 {
 	t->rseq = NULL;
-	t->rseq_len = 0;
 	t->rseq_sig = 0;
 	t->rseq_event_mask = 0;
 }
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 849afe749131..9424ee90589e 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -313,7 +313,7 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
 		/* Unregister rseq for current thread. */
 		if (current->rseq != rseq || !current->rseq)
 			return -EINVAL;
-		if (current->rseq_len != rseq_len)
+		if (rseq_len != sizeof(*rseq))
 			return -EINVAL;
 		if (current->rseq_sig != sig)
 			return -EPERM;
@@ -321,7 +321,6 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
 		if (ret)
 			return ret;
 		current->rseq = NULL;
-		current->rseq_len = 0;
 		current->rseq_sig = 0;
 		return 0;
 	}
@@ -335,7 +334,7 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
 		 * the provided address differs from the prior
 		 * one.
 		 */
-		if (current->rseq != rseq || current->rseq_len != rseq_len)
+		if (current->rseq != rseq || rseq_len != sizeof(*rseq))
 			return -EINVAL;
 		if (current->rseq_sig != sig)
 			return -EPERM;
@@ -353,7 +352,6 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
 	if (!access_ok(rseq, rseq_len))
 		return -EFAULT;
 	current->rseq = rseq;
-	current->rseq_len = rseq_len;
 	current->rseq_sig = sig;
 	/*
 	 * If rseq was previously inactive, and has just been
-- 
2.11.0

^ permalink raw reply related

* [PATCH for 5.1 3/3] rseq/selftests: Adapt number of threads to the number of detected cpus
From: Mathieu Desnoyers @ 2019-03-05 19:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20190305194755.2602-1-mathieu.desnoyers@efficios.com>

On smaller systems, running a test with 200 threads can take a long
time on machines with smaller number of CPUs.

Detect the number of online cpus at test runtime, and multiply that
by 6 to have 6 rseq threads per cpu preempting each other.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Joel Fernandes <joelaf@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Watson <davejwatson@fb.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: linux-kselftest@vger.kernel.org
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Chris Lameter <cl@linux.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Paul Turner <pjt@google.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ben Maurer <bmaurer@fb.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 tools/testing/selftests/rseq/run_param_test.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/rseq/run_param_test.sh b/tools/testing/selftests/rseq/run_param_test.sh
index 3acd6d75ff9f..e426304fd4a0 100755
--- a/tools/testing/selftests/rseq/run_param_test.sh
+++ b/tools/testing/selftests/rseq/run_param_test.sh
@@ -1,6 +1,8 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0+ or MIT
 
+NR_CPUS=`grep '^processor' /proc/cpuinfo | wc -l`
+
 EXTRA_ARGS=${@}
 
 OLDIFS="$IFS"
@@ -28,15 +30,16 @@ IFS="$OLDIFS"
 
 REPS=1000
 SLOW_REPS=100
+NR_THREADS=$((6*${NR_CPUS}))
 
 function do_tests()
 {
 	local i=0
 	while [ "$i" -lt "${#TEST_LIST[@]}" ]; do
 		echo "Running test ${TEST_NAME[$i]}"
-		./param_test ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+		./param_test ${TEST_LIST[$i]} -r ${REPS} -t ${NR_THREADS} ${@} ${EXTRA_ARGS} || exit 1
 		echo "Running compare-twice test ${TEST_NAME[$i]}"
-		./param_test_compare_twice ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+		./param_test_compare_twice ${TEST_LIST[$i]} -r ${REPS} -t ${NR_THREADS} ${@} ${EXTRA_ARGS} || exit 1
 		let "i++"
 	done
 }
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Mathieu Desnoyers @ 2019-03-05 20:18 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H. Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
	Linus Torvalds, Catalin Marinas, Will Deacon, Michael
In-Reply-To: <20190305194755.2602-1-mathieu.desnoyers@efficios.com>

----- On Mar 5, 2019, at 2:47 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> Those changes aiming at 5.1 include one comment cleanup, the removal of
> the rseq_len field from the task struct which serves no purpose
> considering that the struct size is fixed by the ABI, and a selftest
> improvement adapting the number of threads to the number of detected
> CPUs, which is nicer for smaller systems.

For those interested, here is a status update on how things are evolving
in terms of rseq Linux ecosystem integration:

I've been working with glibc maintainers for the past months to get rseq
registration integrated into glibc. The patchset is awaiting feedback
from glibc maintainers at this point. An important part of that integration
is the user-level ABI defining interaction between the executable and
libraries wishing to register rseq within the same process. This is needed
because the rseq system call only supports a single rseq registration per
thread (this was done on purpose). If all goes well we should see rseq
integration in glibc as part of the glibc 2.30 release in August 2019.

For those interested in upcoming rseq kernel patches I have ready, those are
available at [1].

The main reason why we're not seeing more users of rseq out there right now
is because the user-level ABI to interact between libc, applications, and
early adopter libraries needs to be specified before projects start using
rseq. An early adopter of rseq should not be incompatible with a glibc
which introduces rseq registration support.

Once glibc integration is done, here are a few things I have ready:

* NUMA node ID in TLS

Having the NUMA node ID available in a TLS variable would allow glibc to
perform interesting NUMA performance improvements within its locking
implementation, so I have a patch adding NUMA node ID support to rseq
as a new rseq system call flag.

* Adaptative mutex improvements

I have done a prototype using rseq to implement an adaptative mutex which
can detect preemption using a rseq critical section. This ensures the
thread doesn't continue to busy-loop after it returns from preemption, and
calls sys_futex() instead. This is part of a user-space prototype branch [2],
and does not require any kernel change.

* cpu_opv system call

Use-cases requiring access to remote per-cpu data such as memory migration
in a memory allocator and access to specific per-cpu buffers from a
ring-buffer consumer will end up requiring this additional system call.
I'm awaiting a broader adoption of rseq (which depends on glibc integration)
for simpler use-cases before pushing the cpu_opv system call again.

Thanks,

Mathieu

[1] https://git.kernel.org/pub/scm/linux/kernel/git/rseq/linux-rseq.git/
[2] https://github.com/compudj/rseq-test/blob/adapt-lock/test-rseq-adaptative-lock.c

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

^ permalink raw reply

* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yury Norov @ 2019-03-05 20:56 UTC (permalink / raw)
  To: Yury Norov
  Cc: Catalin Marinas, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	linux-doc, linux-arch, linux-api, Adam Borowski, Alexander Graf,
	Alexey Klimov, Andreas Schwab, Andrew Pinski, Bamvor Zhangjian,
	Chris Metcalf, Christoph Muellner, Dave Martin, David S . Miller,
	Florian Weimer, Geert Uytterhoeven, Heiko Carstens,
	James Hogan <jame>
In-Reply-To: <20180516081910.10067-1-ynorov@caviumnetworks.com>

On Wed, May 16, 2018 at 11:18:45AM +0300, Yury Norov wrote:
> This series enables AARCH64 with ILP32 mode.
> 
> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> option that is enabled for existing 32-bit architectures but disabled
> for new arches (so 64-bit off_t userspace type is used by new userspace).
> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
> 
> Based on kernel v4.16. Tested with LTP, glibc testsuite, trinity, lmbench,
> CPUSpec.
> 
> This series on github: 
> https://github.com/norov/linux/tree/ilp32-4.16
> Linaro toolchain:
> http://snapshots.linaro.org/components/toolchain/binaries/7.3-2018.04-rc1/aarch64-linux-gnu_ilp32/
> Debian repo:
> http://people.linaro.org/~wookey/ilp32/
> OpenSUSE repo:
> https://build.opensuse.org/project/show/devel:ARM:Factory:Contrib:ILP32
> 
> Changes:
> v3: https://lkml.org/lkml/2014/9/3/704
> v4: https://lkml.org/lkml/2015/4/13/691
> v5: https://lkml.org/lkml/2015/9/29/911
> v6: https://lkml.org/lkml/2016/5/23/661
> v7: https://lkml.org/lkml/2017/1/9/213
> v8: https://lkml.org/lkml/2017/6/19/624
> v9: - rebased on top of v4.16;
>     - signal subsystem reworked to avoid code duplication, as requested
>       by Dave Martin (patches 18 and 20);
>     - new files introduced in series use SPDX notation for license;
>     - linux-api and linux-arch CCed as the series changes kernel ABI;
>     - checkpatch and other minor fixes.
>     - Zhou Chengming's reported-by for patch 2 and signed-off-by for
>       patch 21 removed because his email became invalid. Zhou, please
>       share your new email.

Hi all,

This is the 5.0-based version.
https://github.com/norov/linux/tree/ilp32-5.0

Since 4.20:
 - all new files are consistently GPL2.0-licensed;
 - AUDIT_ARCH_AARCH64ILP32 added to let syscall_get_arch()
   differentiate ARM64/ILP32 from ARM64/ILP32.

Yury

^ permalink raw reply

* Re: [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Peter Zijlstra @ 2019-03-05 21:49 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Thomas Gleixner, linux-kernel, linux-api, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20190305194755.2602-1-mathieu.desnoyers@efficios.com>

On Tue, Mar 05, 2019 at 02:47:52PM -0500, Mathieu Desnoyers wrote:
> Mathieu Desnoyers (3):
>   rseq: cleanup: Reflect removal of event counter in comments
>   rseq: cleanup: remove rseq_len from task_struct
>   rseq/selftests: Adapt number of threads to the number of detected cpus
> 
>  arch/arm/kernel/signal.c                       | 3 +--
>  arch/x86/kernel/signal.c                       | 5 +----
>  include/linux/sched.h                          | 4 ----
>  kernel/rseq.c                                  | 9 +++------
>  tools/testing/selftests/rseq/run_param_test.sh | 7 +++++--
>  5 files changed, 10 insertions(+), 18 deletions(-)

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

^ permalink raw reply

* Re: [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Peter Zijlstra @ 2019-03-05 21:58 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Thomas Gleixner, linux-kernel, linux-api, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H. Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
	Linus Torvalds, Catalin Marinas, Will Deacon, Michael
In-Reply-To: <1689743723.311.1551817115045.JavaMail.zimbra@efficios.com>

On Tue, Mar 05, 2019 at 03:18:35PM -0500, Mathieu Desnoyers wrote:
> * NUMA node ID in TLS
> 
> Having the NUMA node ID available in a TLS variable would allow glibc to
> perform interesting NUMA performance improvements within its locking
> implementation, so I have a patch adding NUMA node ID support to rseq
> as a new rseq system call flag.

Details? There's just not much room in the futex word, and futexes
themselves are not numa aware.

Before all this spectre nonsense; tglx and me were looking at a futex2
syscall that would, among other things, cure this.

> * Adaptative mutex improvements
> 
> I have done a prototype using rseq to implement an adaptative mutex which
> can detect preemption using a rseq critical section. This ensures the
> thread doesn't continue to busy-loop after it returns from preemption, and
> calls sys_futex() instead. This is part of a user-space prototype branch [2],
> and does not require any kernel change.

I'm still not convinced that is actually the right way to go about
things. The kernel heuristic is spin while the _owner_ runs, and we
don't get preempted, obviously.

And the only userspace spinning that makes sense is to cover the cost of
the syscall. Now Obviously PTI wrecked everything, but before that
syscalls were actually plenty fast and you didn't need many cmpxchg
cycles to amortize the syscall itself -- which could then do kernel side
adaptive spinning (when required).

^ permalink raw reply

* Re: [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Mathieu Desnoyers @ 2019-03-05 22:32 UTC (permalink / raw)
  To: Peter Zijlstra, H.J. Lu, libc-alpha
  Cc: Thomas Gleixner, linux-kernel, linux-api, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H. Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
	Linus Torvalds, Catalin Marinas, Will Deacon, Michael
In-Reply-To: <20190305215848.GQ32477@hirez.programming.kicks-ass.net>

----- On Mar 5, 2019, at 4:58 PM, Peter Zijlstra peterz@infradead.org wrote:

> On Tue, Mar 05, 2019 at 03:18:35PM -0500, Mathieu Desnoyers wrote:
>> * NUMA node ID in TLS
>> 
>> Having the NUMA node ID available in a TLS variable would allow glibc to
>> perform interesting NUMA performance improvements within its locking
>> implementation, so I have a patch adding NUMA node ID support to rseq
>> as a new rseq system call flag.
> 
> Details? There's just not much room in the futex word, and futexes
> themselves are not numa aware.

It was discussed in this libc-alpha mailing list thread:

https://public-inbox.org/libc-alpha/CAMe9rOo7i_-keOooa0D+P_wzatVCdKkTRiFiJ-cxpnvi+eApuQ@mail.gmail.com/

(adding the relevant people in CC)

I'd like to hear in more details on how they intend to design
NUMA-aware spinlocks within glibc. All I know is that quick
access to the node ID would help for this.

I would suspect we could split a lock into per-numa-node locks.
Grabbing the local numa lock would then allow grabbing the global
lock. This should help reducing remote NUMA accesses on the global
lock in contended cases, but I'm really just guessing here.

> 
> Before all this spectre nonsense; tglx and me were looking at a futex2
> syscall that would, among other things, cure this.

The email thread I point to above talks about "spinlocks", so I'm not
sure whether their intent is to apply this to mutexes as well.

> 
>> * Adaptative mutex improvements
>> 
>> I have done a prototype using rseq to implement an adaptative mutex which
>> can detect preemption using a rseq critical section. This ensures the
>> thread doesn't continue to busy-loop after it returns from preemption, and
>> calls sys_futex() instead. This is part of a user-space prototype branch [2],
>> and does not require any kernel change.
> 
> I'm still not convinced that is actually the right way to go about
> things. The kernel heuristic is spin while the _owner_ runs, and we
> don't get preempted, obviously.
> 
> And the only userspace spinning that makes sense is to cover the cost of
> the syscall. Now Obviously PTI wrecked everything, but before that
> syscalls were actually plenty fast and you didn't need many cmpxchg
> cycles to amortize the syscall itself -- which could then do kernel side
> adaptive spinning (when required).

Indeed with PTI the system calls are back to their slow self. ;)

You point about owner is interesting. Perhaps there is one tweak that I
should add in there. We could write the owner thread ID in the lock word.

When trying to grab a lock, one of a few situations can happen:

- It's unlocked, so we grab it by storing our thread ID,
- It's locked, and we can fetch the CPU number of the thread owning it
  if we can access its (struct rseq *)->cpu_id through a lookup using its
  thread ID, We can then check whether it's the same CPU we are running on.
  - If so, we _know_ we should let the owner run, so we call futex right away,
    no spinning. We can even boost it for priority inheritance mutexes,
  - If it's owned by a thread which was last running on a different CPU,
    then it may make sense to actively try to grab the lock by spinning
    up to a certain number of loops (which can be either fixed or adaptative).
    After that limit, call futex. If preempted while looping, call futex.

Do you see this as an improvement over what exists today, or am I
on the wrong track ?

Thanks,

Mathieu


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

^ permalink raw reply

* Re: [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Peter Zijlstra @ 2019-03-06  8:21 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: H.J. Lu, libc-alpha, Thomas Gleixner, linux-kernel, linux-api,
	Paul E . McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
	Paul Turner, Andrew Morton, Russell King, Ingo Molnar,
	H. Peter Anvin, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds
In-Reply-To: <486623963.509.1551825130539.JavaMail.zimbra@efficios.com>

On Tue, Mar 05, 2019 at 05:32:10PM -0500, Mathieu Desnoyers wrote:
> ----- On Mar 5, 2019, at 4:58 PM, Peter Zijlstra peterz@infradead.org wrote:
> 
> > On Tue, Mar 05, 2019 at 03:18:35PM -0500, Mathieu Desnoyers wrote:
> >> * NUMA node ID in TLS
> >> 
> >> Having the NUMA node ID available in a TLS variable would allow glibc to
> >> perform interesting NUMA performance improvements within its locking
> >> implementation, so I have a patch adding NUMA node ID support to rseq
> >> as a new rseq system call flag.
> > 
> > Details? There's just not much room in the futex word, and futexes
> > themselves are not numa aware.
> 
> It was discussed in this libc-alpha mailing list thread:
> 
> https://public-inbox.org/libc-alpha/CAMe9rOo7i_-keOooa0D+P_wzatVCdKkTRiFiJ-cxpnvi+eApuQ@mail.gmail.com/
> 
> (adding the relevant people in CC)
> 
> I'd like to hear in more details on how they intend to design
> NUMA-aware spinlocks within glibc. All I know is that quick
> access to the node ID would help for this.

Userspace spinlocks are a trainwreck anyway. The only case where they
can possibly work is when there's only a single thread on every cpu.
Pretty much any other scenario is fail; see why we have paravirt
spinlocks.

^ permalink raw reply

* Re: [PATCH for 5.1 0/3] Restartable Sequences updates for 5.1
From: Peter Zijlstra @ 2019-03-06  8:30 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: H.J. Lu, libc-alpha, Thomas Gleixner, linux-kernel, linux-api,
	Paul E . McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
	Paul Turner, Andrew Morton, Russell King, Ingo Molnar,
	H. Peter Anvin, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds
In-Reply-To: <486623963.509.1551825130539.JavaMail.zimbra@efficios.com>

On Tue, Mar 05, 2019 at 05:32:10PM -0500, Mathieu Desnoyers wrote:

> >> * Adaptative mutex improvements
> >> 
> >> I have done a prototype using rseq to implement an adaptative mutex which
> >> can detect preemption using a rseq critical section. This ensures the
> >> thread doesn't continue to busy-loop after it returns from preemption, and
> >> calls sys_futex() instead. This is part of a user-space prototype branch [2],
> >> and does not require any kernel change.
> > 
> > I'm still not convinced that is actually the right way to go about
> > things. The kernel heuristic is spin while the _owner_ runs, and we
> > don't get preempted, obviously.
> > 
> > And the only userspace spinning that makes sense is to cover the cost of
> > the syscall. Now Obviously PTI wrecked everything, but before that
> > syscalls were actually plenty fast and you didn't need many cmpxchg
> > cycles to amortize the syscall itself -- which could then do kernel side
> > adaptive spinning (when required).
> 
> Indeed with PTI the system calls are back to their slow self. ;)
> 
> You point about owner is interesting. Perhaps there is one tweak that I
> should add in there. We could write the owner thread ID in the lock word.

This is already required for PI (and I think robust) futexes. There have
been proposals for FUTEX_LOCK and FUTEX_UNLOCK (!PI) primitives that
require the same.

Waiman had some patches; but I think all went under because 'important'
stuff happened.

> When trying to grab a lock, one of a few situations can happen:
> 
> - It's unlocked, so we grab it by storing our thread ID,
> - It's locked, and we can fetch the CPU number of the thread owning it
>   if we can access its (struct rseq *)->cpu_id through a lookup using its
>   thread ID, We can then check whether it's the same CPU we are running on.

That might just work with threads (private futexes; which are the
majority these these I think), but will obviously not work with regular
(shared) futexes.

>   - If so, we _know_ we should let the owner run, so we call futex right away,
>     no spinning. We can even boost it for priority inheritance mutexes,
>   - If it's owned by a thread which was last running on a different CPU,
>     then it may make sense to actively try to grab the lock by spinning
>     up to a certain number of loops (which can be either fixed or adaptative).
>     After that limit, call futex. If preempted while looping, call futex.
> 
> Do you see this as an improvement over what exists today, or am I
> on the wrong track ?

That's probably better than what they have today. Last time I looked at
libc pthread I got really sad -- arguably that was a long time ago, and
some of that stuff is because POSIX, but still.

Some day we should redesign all that.. futex2 etc.

^ permalink raw reply

* Re: [PATCH 0/3] mincore() and IOCB_NOWAIT adjustments
From: Jiri Kosina @ 2019-03-06 12:11 UTC (permalink / raw)
  To: Vlastimil Babka, Andrew Morton
  Cc: Linus Torvalds, linux-kernel, linux-mm, linux-api, Peter Zijlstra,
	Greg KH, Jann Horn, Andy Lutomirski, Cyril Hrubis, Daniel Gruss,
	Dave Chinner, Dominique Martinet, Kevin Easton,
	Kirill A. Shutemov, Matthew Wilcox, Tejun Heo
In-Reply-To: <20190130124420.1834-1-vbabka@suse.cz>

On Wed, 30 Jan 2019, Vlastimil Babka wrote:

> I've collected the patches from the discussion for formal posting. The first
> two should be settled already, third one is the possible improvement I've
> mentioned earlier, where only in restricted case we resort to existence of page
> table mapping (the original and later reverted approach from Linus) instead of
> faking the result completely. Review and testing welcome.
> 
> The consensus seems to be going through -mm tree for 5.1, unless Linus wants
> them alredy for 5.0.
> 
> Jiri Kosina (2):
>   mm/mincore: make mincore() more conservative
>   mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O
> 
> Vlastimil Babka (1):
>   mm/mincore: provide mapped status when cached status is not allowed

Andrew,

could you please take at least the correct and straightforward fix for 
mincore() before we figure out how to deal with the slightly less 
practical RWF_NOWAIT? Thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: a.out coredumping: fix or delete?
From: Thomas Gleixner @ 2019-03-06 12:25 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Borislav Petkov, Jann Horn, Al Viro, kernel list, linux-fsdevel,
	the arch/x86 maintainers, Linux API
In-Reply-To: <20190305122218.GD13380@bombadil.infradead.org>

On Tue, 5 Mar 2019, Matthew Wilcox wrote:
> On Tue, Mar 05, 2019 at 10:19:04AM +0100, Borislav Petkov wrote:
> > On Sat, Mar 02, 2019 at 12:57:30AM +0100, Jann Horn wrote:
> > > In theory, Linux can dump cores for a.out binaries. In practice, that
> > > code is pretty bitrotten and buggy. Does anyone want that code so much
> > > that they'd like to fix it, or can we just delete it?
> > 
> > I was asking myself the same question while doing
> > 
> >   10970e1b4be9 ("x86/a.out: Clear the dump structure initially")
> > 
> > especially since it is kinda hard to get the toolchain to even create an
> > a.out executable - your script to create one case-in-point.
> 
> It's been 25 years since Linux added support for ELF.  Can we just
> delete the a.out support entirely now?  According to the Linux-ELF HOWTO,
> support was added in 1.1.52 (August 1994).  It's pretty much necromancy
> at this point.

The Kernel-Necrophilia cult members might disagree. :)

But yes, good riddance.

Thanks,

	tglx

^ permalink raw reply

* Re: a.out coredumping: fix or delete?
From: Theodore Y. Ts'o @ 2019-03-06 14:11 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Matthew Wilcox, Borislav Petkov, Jann Horn, Al Viro, kernel list,
	linux-fsdevel, the arch/x86 maintainers, Linux API
In-Reply-To: <alpine.DEB.2.21.1903061323030.1646@nanos.tec.linutronix.de>

On Wed, Mar 06, 2019 at 01:25:17PM +0100, Thomas Gleixner wrote:
> > It's been 25 years since Linux added support for ELF.  Can we just
> > delete the a.out support entirely now?  According to the Linux-ELF HOWTO,
> > support was added in 1.1.52 (August 1994).  It's pretty much necromancy
> > at this point.
> 
> The Kernel-Necrophilia cult members might disagree. :)
> 
> But yes, good riddance.

Doesn't Minix 1.0 use a.out?  It *is* cool to be able to binaries from
run dead operating systems.  :-)

						- Ted

^ permalink raw reply

* [PATCH] ipc: Fix building compat mode without sysvipc
From: Arnd Bergmann @ 2019-03-06 14:29 UTC (permalink / raw)
  To: y2038
  Cc: Arnd Bergmann, Laura Abbott, John Stultz, Thomas Gleixner,
	Dominik Brodowski, linux-api, linux-kernel

As John Stultz noticed, my y2038 syscall series caused a link
failure when CONFIG_SYSVIPC is enabled but CONFIG_COMPAT is
enabled:

arch/arm64/kernel/sys32.o:(.rodata+0x960): undefined reference to `__arm64_compat_sys_old_semctl'
arch/arm64/kernel/sys32.o:(.rodata+0x980): undefined reference to `__arm64_compat_sys_old_msgctl'
arch/arm64/kernel/sys32.o:(.rodata+0x9a0): undefined reference to `__arm64_compat_sys_old_shmctl'

Add the missing entries in kernel/sys_ni.c for the new system
calls.

Cc: Laura Abbott <labbott@redhat.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I'm about to send off my pull requests for arm-soc, so I'd just
send another one with just this common from my y2038 tree.
---
 kernel/sys_ni.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 85e5ccec0955..62a6c8707799 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -202,6 +202,7 @@ COND_SYSCALL(msgget);
 COND_SYSCALL(old_msgctl);
 COND_SYSCALL(msgctl);
 COND_SYSCALL_COMPAT(msgctl);
+COND_SYSCALL_COMPAT(old_msgctl);
 COND_SYSCALL(msgrcv);
 COND_SYSCALL_COMPAT(msgrcv);
 COND_SYSCALL(msgsnd);
@@ -212,6 +213,7 @@ COND_SYSCALL(semget);
 COND_SYSCALL(old_semctl);
 COND_SYSCALL(semctl);
 COND_SYSCALL_COMPAT(semctl);
+COND_SYSCALL_COMPAT(old_semctl);
 COND_SYSCALL(semtimedop);
 COND_SYSCALL(semtimedop_time32);
 COND_SYSCALL(semop);
@@ -221,6 +223,7 @@ COND_SYSCALL(shmget);
 COND_SYSCALL(old_shmctl);
 COND_SYSCALL(shmctl);
 COND_SYSCALL_COMPAT(shmctl);
+COND_SYSCALL_COMPAT(old_shmctl);
 COND_SYSCALL(shmat);
 COND_SYSCALL_COMPAT(shmat);
 COND_SYSCALL(shmdt);
-- 
2.20.0

^ permalink raw reply related

* Re: [PATCH] ipc: Fix building compat mode without sysvipc
From: Laura Abbott @ 2019-03-06 14:44 UTC (permalink / raw)
  To: Arnd Bergmann, y2038
  Cc: John Stultz, Thomas Gleixner, Dominik Brodowski, linux-api,
	linux-kernel
In-Reply-To: <20190306142944.834167-1-arnd@arndb.de>

On 3/6/19 6:29 AM, Arnd Bergmann wrote:
> As John Stultz noticed, my y2038 syscall series caused a link
> failure when CONFIG_SYSVIPC is enabled but CONFIG_COMPAT is
> enabled:
> 

is this supposed to be "CONFIG_SYSVIPC is disabled" to match the
subject?

> arch/arm64/kernel/sys32.o:(.rodata+0x960): undefined reference to `__arm64_compat_sys_old_semctl'
> arch/arm64/kernel/sys32.o:(.rodata+0x980): undefined reference to `__arm64_compat_sys_old_msgctl'
> arch/arm64/kernel/sys32.o:(.rodata+0x9a0): undefined reference to `__arm64_compat_sys_old_shmctl'
> 
> Add the missing entries in kernel/sys_ni.c for the new system
> calls.
> 
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> I'm about to send off my pull requests for arm-soc, so I'd just
> send another one with just this common from my y2038 tree.
> ---
>   kernel/sys_ni.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 85e5ccec0955..62a6c8707799 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -202,6 +202,7 @@ COND_SYSCALL(msgget);
>   COND_SYSCALL(old_msgctl);
>   COND_SYSCALL(msgctl);
>   COND_SYSCALL_COMPAT(msgctl);
> +COND_SYSCALL_COMPAT(old_msgctl);
>   COND_SYSCALL(msgrcv);
>   COND_SYSCALL_COMPAT(msgrcv);
>   COND_SYSCALL(msgsnd);
> @@ -212,6 +213,7 @@ COND_SYSCALL(semget);
>   COND_SYSCALL(old_semctl);
>   COND_SYSCALL(semctl);
>   COND_SYSCALL_COMPAT(semctl);
> +COND_SYSCALL_COMPAT(old_semctl);
>   COND_SYSCALL(semtimedop);
>   COND_SYSCALL(semtimedop_time32);
>   COND_SYSCALL(semop);
> @@ -221,6 +223,7 @@ COND_SYSCALL(shmget);
>   COND_SYSCALL(old_shmctl);
>   COND_SYSCALL(shmctl);
>   COND_SYSCALL_COMPAT(shmctl);
> +COND_SYSCALL_COMPAT(old_shmctl);
>   COND_SYSCALL(shmat);
>   COND_SYSCALL_COMPAT(shmat);
>   COND_SYSCALL(shmdt);
> 

^ permalink raw reply

* Re: [PATCH] ipc: Fix building compat mode without sysvipc
From: Arnd Bergmann @ 2019-03-06 15:00 UTC (permalink / raw)
  To: Laura Abbott
  Cc: y2038 Mailman List, John Stultz, Thomas Gleixner,
	Dominik Brodowski, Linux API, Linux Kernel Mailing List
In-Reply-To: <3614a6b8-0c2f-747c-ce60-b58e79c5cec5@redhat.com>

On Wed, Mar 6, 2019 at 3:44 PM Laura Abbott <labbott@redhat.com> wrote:
>
> On 3/6/19 6:29 AM, Arnd Bergmann wrote:
> > As John Stultz noticed, my y2038 syscall series caused a link
> > failure when CONFIG_SYSVIPC is enabled but CONFIG_COMPAT is
> > enabled:
> >
>
> is this supposed to be "CONFIG_SYSVIPC is disabled" to match the
> subject?

Fixing it now, thanks!

      Arnd

^ permalink raw reply

* Re: [PATCH] x86: Deprecate a.out support
From: Geert Uytterhoeven @ 2019-03-06 15:07 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, Alan Cox, Matthew Wilcox, Jann Horn, Al Viro,
	Thomas Gleixner, kernel list, linux-fsdevel,
	the arch/x86 maintainers, Linux API, Andrew Morton,
	Richard Weinberger, Anton Ivanov, alpha, linux-m68k
In-Reply-To: <20190305181808.GH8256@zn.tnic>

Hi Borislav,

On Tue, Mar 5, 2019 at 8:04 PM Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Mar 05, 2019 at 07:11:38PM +0100, Borislav Petkov wrote:
> > I guess you could Cc arch maintainers with the a.out-core.h removal
> > patch to see if anyone screams.
>
> And they're like two for which we need confirmation:
>
> $ git ls-files | grep a.out-core.h
> arch/alpha/include/asm/a.out-core.h
> arch/m68k/include/asm/a.out-core.h
> arch/um/include/asm/a.out-core.h
> arch/x86/include/asm/a.out-core.h
>
> um and x86 are clear.
>
> Adding alpha and m68k MLs to Cc.

Thanks!

The oldest binaries I still have lying around (an ext2 ramdisk image,
still used from time to time) are ELF, from just after the a.out to ELF
transition on m68k.

I think it's safe to assume no one still runs a.out binaries on m68k.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox