From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> To: Ben Maurer <bmaurer@fb.com> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, Peter Zijlstra <peterz@infradead.org>, Paul Turner <pjt@google.com>, Andrew Hunter <ahh@google.com>, Andy Lutomirski <luto@amacapital.net>, Dave Watson <davejwatson@fb.com>, Josh Triplett <josh@joshtriplett.org>, Will Deacon <will.deacon@arm.com>, linux-kernel <linux-kernel@vger.kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, rostedt <rostedt@goodmis.org>, Linus Torvalds <torvalds@linux-foundation.org>, Andrew Morton <akpm@linux-foundation.org>, Russell King <linux@arm.linux.org.uk>, Catalin Marinas <catalin.marinas@arm.com>, Michael Kerrisk <mtk.manpages@gmail.com>, Alexander Viro <viro@zeniv.linux.org.uk>, linux-api <linux-api@vger.kernel.org> Subject: Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call Date: Mon, 23 Oct 2017 20:44:01 +0000 (UTC) [thread overview] Message-ID: <439398759.47028.1508791441765.JavaMail.zimbra@efficios.com> (raw) In-Reply-To: <CY4PR15MB16888F91F41A4A1D322C102CCF460@CY4PR15MB1688.namprd15.prod.outlook.com> ----- On Oct 23, 2017, at 7:30 PM, Ben Maurer bmaurer@fb.com wrote: >> if (!((long)ip - (long)start_ip <= (long)post_commit_offset)) >> return 1; > >> This introduces an issue here: if "ip" is lower than "start_ip", we >> can incorrectly think we are in a critical section, when we are in >> fact not. > > This shouldn't be an issue if we used unsigned numbers. Eg if start_ip is X and > post_commit_offset is L, then (ip - X <= L) means that if ip is less than X ip > - X will be signed, which will become a large unsigned value. > >> or to the kernel to set it back to NULL if it finds out that it is >> preempting/delivering a signal over an instruction pointer outside >> of the current rseq_cs start_ip/post_commit_ip range (lazy clear). > > I see, lazy clear makes sense. Still, if during most execution periods the user > code enters some rseq section (likely if rseq is used for something like > malloc) on every context switch this code will have to be run. > >> Moreover, this modification would add a subtraction on the common case >> (ip - start_ip), and makes the ABI slightly uglier. > > We could benchmark it but the subtraction should be similar in cost to the extra > comparison but reducing the number of branches seems like it will help as well. > FWIW GCC attempts to translate this kind of sequence to a subtract and compare: > https://godbolt.org/g/5DGLvo. > > I agree the ABI is uglier, but since we're mucking with every context switch I > thought I'd point it out. Thanks for following up on this. I did not initially realize the importance of doing the unsigned comparison. I've pushed a commit in my private dev branch implementing your suggestion. https://github.com/compudj/linux-percpu-dev/commit/4cf8e9104636b51741c0118f2c88519e3acab7aa Thanks! Mathieu > >> If I understand well, you are proposing to speed up .so load time by >> means of removing relocations of pointers within rseq_cs, done by >> making those relative to the rseq_cs address. > > Yeah, I think this may be overkill as optimization. -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> To: Ben Maurer <bmaurer-b10kYP2dOMg@public.gmane.org> Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>, Boqun Feng <boqun.feng-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>, Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Andrew Hunter <ahh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>, Dave Watson <davejwatson-b10kYP2dOMg@public.gmane.org>, Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>, linux-kernel <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>, Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org>, Chris Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>, Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>, rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org>, Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>, Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>, Michael Subject: Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call Date: Mon, 23 Oct 2017 20:44:01 +0000 (UTC) [thread overview] Message-ID: <439398759.47028.1508791441765.JavaMail.zimbra@efficios.com> (raw) In-Reply-To: <CY4PR15MB16888F91F41A4A1D322C102CCF460-ZVJ2su15u+xeX4ZvlgGe+Yd3EbNNOtPMvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> ----- On Oct 23, 2017, at 7:30 PM, Ben Maurer bmaurer-b10kYP2dOMg@public.gmane.org wrote: >> if (!((long)ip - (long)start_ip <= (long)post_commit_offset)) >> return 1; > >> This introduces an issue here: if "ip" is lower than "start_ip", we >> can incorrectly think we are in a critical section, when we are in >> fact not. > > This shouldn't be an issue if we used unsigned numbers. Eg if start_ip is X and > post_commit_offset is L, then (ip - X <= L) means that if ip is less than X ip > - X will be signed, which will become a large unsigned value. > >> or to the kernel to set it back to NULL if it finds out that it is >> preempting/delivering a signal over an instruction pointer outside >> of the current rseq_cs start_ip/post_commit_ip range (lazy clear). > > I see, lazy clear makes sense. Still, if during most execution periods the user > code enters some rseq section (likely if rseq is used for something like > malloc) on every context switch this code will have to be run. > >> Moreover, this modification would add a subtraction on the common case >> (ip - start_ip), and makes the ABI slightly uglier. > > We could benchmark it but the subtraction should be similar in cost to the extra > comparison but reducing the number of branches seems like it will help as well. > FWIW GCC attempts to translate this kind of sequence to a subtract and compare: > https://godbolt.org/g/5DGLvo. > > I agree the ABI is uglier, but since we're mucking with every context switch I > thought I'd point it out. Thanks for following up on this. I did not initially realize the importance of doing the unsigned comparison. I've pushed a commit in my private dev branch implementing your suggestion. https://github.com/compudj/linux-percpu-dev/commit/4cf8e9104636b51741c0118f2c88519e3acab7aa Thanks! Mathieu > >> If I understand well, you are proposing to speed up .so load time by >> means of removing relocations of pointers within rseq_cs, done by >> making those relative to the rseq_cs address. > > Yeah, I think this may be overkill as optimization. -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
next prev parent reply other threads:[~2017-10-23 20:41 UTC|newest] Thread overview: 113+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-12 23:03 [RFC PATCH v9 for 4.15 00/14] Restartable sequences and CPU op vector system calls Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call Mathieu Desnoyers 2017-10-13 0:36 ` Linus Torvalds 2017-10-13 0:36 ` Linus Torvalds 2017-10-13 9:35 ` Ben Maurer 2017-10-13 9:35 ` Ben Maurer 2017-10-13 18:30 ` Linus Torvalds 2017-10-13 18:30 ` Linus Torvalds 2017-10-13 20:54 ` Paul E. McKenney 2017-10-13 20:54 ` Paul E. McKenney 2017-10-13 21:05 ` Linus Torvalds 2017-10-13 21:05 ` Linus Torvalds 2017-10-13 21:21 ` Paul E. McKenney 2017-10-13 21:21 ` Paul E. McKenney 2017-10-13 21:36 ` Mathieu Desnoyers 2017-10-13 21:36 ` Mathieu Desnoyers 2017-10-16 16:04 ` Carlos O'Donell 2017-10-16 16:04 ` Carlos O'Donell 2017-10-16 16:46 ` Andi Kleen 2017-10-16 16:46 ` Andi Kleen 2017-10-16 22:17 ` Mathieu Desnoyers 2017-10-16 22:17 ` Mathieu Desnoyers 2017-10-17 16:19 ` Ben Maurer 2017-10-17 16:19 ` Ben Maurer 2017-10-17 16:33 ` Mathieu Desnoyers 2017-10-17 16:33 ` Mathieu Desnoyers 2017-10-17 16:41 ` Ben Maurer 2017-10-17 16:41 ` Ben Maurer 2017-10-17 17:48 ` Mathieu Desnoyers 2017-10-17 17:48 ` Mathieu Desnoyers 2017-10-18 6:22 ` Greg KH 2017-10-18 6:22 ` Greg KH 2017-10-18 16:28 ` Mathieu Desnoyers 2017-10-18 16:28 ` Mathieu Desnoyers 2017-10-14 3:01 ` Andi Kleen 2017-10-14 3:01 ` Andi Kleen 2017-10-14 4:05 ` Linus Torvalds 2017-10-14 4:05 ` Linus Torvalds 2017-10-14 11:37 ` Mathieu Desnoyers 2017-10-14 11:37 ` Mathieu Desnoyers 2017-10-13 12:50 ` Florian Weimer 2017-10-13 13:40 ` Mathieu Desnoyers 2017-10-13 13:40 ` Mathieu Desnoyers 2017-10-13 13:56 ` Florian Weimer 2017-10-13 13:56 ` Florian Weimer 2017-10-13 14:27 ` Mathieu Desnoyers 2017-10-13 14:27 ` Mathieu Desnoyers 2017-10-13 17:24 ` Andy Lutomirski 2017-10-13 17:24 ` Andy Lutomirski 2017-10-13 17:53 ` Florian Weimer 2017-10-13 17:53 ` Florian Weimer 2017-10-13 18:17 ` Andy Lutomirski 2017-10-13 18:17 ` Andy Lutomirski 2017-10-14 11:53 ` Mathieu Desnoyers 2017-10-14 11:53 ` Mathieu Desnoyers 2017-10-18 16:41 ` Ben Maurer 2017-10-18 18:11 ` Mathieu Desnoyers 2017-10-18 18:11 ` Mathieu Desnoyers 2017-10-19 11:35 ` Mathieu Desnoyers 2017-10-19 11:35 ` Mathieu Desnoyers 2017-10-19 17:01 ` Florian Weimer 2017-10-19 17:01 ` Florian Weimer 2017-10-23 17:30 ` Ben Maurer 2017-10-23 17:30 ` Ben Maurer 2017-10-23 20:44 ` Mathieu Desnoyers [this message] 2017-10-23 20:44 ` Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 02/14] tracing: instrument restartable sequences Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 03/14] Restartable sequences: ARM 32 architecture support Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 04/14] Restartable sequences: wire up ARM 32 system call Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 05/14] Restartable sequences: x86 32/64 architecture support Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 06/14] Restartable sequences: wire up x86 32/64 system call Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 07/14] Restartable sequences: powerpc architecture support Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 08/14] Restartable sequences: Wire up powerpc system call Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 09/14] Provide cpu_opv " Mathieu Desnoyers 2017-10-13 13:57 ` Alan Cox 2017-10-13 13:57 ` Alan Cox 2017-10-13 14:50 ` Mathieu Desnoyers 2017-10-13 14:50 ` Mathieu Desnoyers 2017-10-14 14:22 ` Mathieu Desnoyers 2017-10-14 14:22 ` Mathieu Desnoyers 2017-10-13 17:20 ` Andy Lutomirski 2017-10-13 17:20 ` Andy Lutomirski 2017-10-14 2:50 ` Andi Kleen 2017-10-14 2:50 ` Andi Kleen 2017-10-14 13:35 ` Mathieu Desnoyers 2017-10-14 13:35 ` Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 10/14] cpu_opv: Wire up x86 32/64 " Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 11/14] cpu_opv: Wire up powerpc " Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 12/14] cpu_opv: Wire up ARM32 " Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 13/14] cpu_opv: Implement selftests Mathieu Desnoyers 2017-10-12 23:03 ` [RFC PATCH for 4.15 14/14] Restartable sequences: Provide self-tests Mathieu Desnoyers 2017-10-16 2:51 ` Michael Ellerman 2017-10-16 2:51 ` Michael Ellerman 2017-10-16 14:23 ` Mathieu Desnoyers 2017-10-16 14:23 ` Mathieu Desnoyers 2017-10-17 10:38 ` Michael Ellerman 2017-10-17 10:38 ` Michael Ellerman 2017-10-17 13:50 ` Mathieu Desnoyers 2017-10-17 13:50 ` Mathieu Desnoyers 2017-10-16 18:50 ` Mathieu Desnoyers 2017-10-16 18:50 ` Mathieu Desnoyers 2017-10-17 10:36 ` Michael Ellerman 2017-10-17 10:36 ` Michael Ellerman 2017-10-17 13:50 ` Mathieu Desnoyers 2017-10-17 13:50 ` Mathieu Desnoyers 2017-10-18 5:45 ` Michael Ellerman 2017-10-18 5:45 ` Michael Ellerman 2017-10-16 3:00 ` Michael Ellerman 2017-10-16 3:00 ` Michael Ellerman 2017-10-16 3:48 ` Boqun Feng 2017-10-16 3:48 ` Boqun Feng 2017-10-16 11:48 ` Michael Ellerman 2017-10-16 11:48 ` Michael Ellerman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=439398759.47028.1508791441765.JavaMail.zimbra@efficios.com \ --to=mathieu.desnoyers@efficios.com \ --cc=ahh@google.com \ --cc=akpm@linux-foundation.org \ --cc=andi@firstfloor.org \ --cc=bmaurer@fb.com \ --cc=boqun.feng@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=cl@linux.com \ --cc=davejwatson@fb.com \ --cc=hpa@zytor.com \ --cc=josh@joshtriplett.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=luto@amacapital.net \ --cc=mingo@redhat.com \ --cc=mtk.manpages@gmail.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=peterz@infradead.org \ --cc=pjt@google.com \ --cc=rostedt@goodmis.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=viro@zeniv.linux.org.uk \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.