From: Joel Fernandes <joelaf@google.com> To: Daniel Colascione <dancol@google.com> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Peter Zijlstra <peterz@infradead.org>, Paul McKenney <paulmck@linux.vnet.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, Andy Lutomirski <luto@amacapital.net>, davejwatson@fb.com, LKML <linux-kernel@vger.kernel.org>, linux-api@vger.kernel.org, Paul Turner <pjt@google.com>, Andrew Morton <akpm@linux-foundation.org>, linux@arm.linux.org.uk, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, hpa@zytor.com, Andrew Hunter <ahh@google.com>, andi@firstfloor.org, cl@linux.com, bmaurer@fb.com, Steven Rostedt <rostedt@goodmis.org>, Josh Triplett <josh@joshtriplett.org>, torvalds@linux-foundation.org, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, mtk.manpages@gmail.com, longman@redhat.com Subject: Re: [RFC PATCH for 4.18 00/14] Restartable Sequences Date: Thu, 03 May 2018 17:46:55 +0000 [thread overview] Message-ID: <CAJWu+opkzpmwY5jJ0353JKyaOFCqa95AKGhxUdQZ6pZf_omsaQ@mail.gmail.com> (raw) In-Reply-To: <CAKOZueve1K8bmS0pmeCAAburoJtzRA4DeN=OCyW3kHiNo8WO=A@mail.gmail.com> Hi Daniel, Nice to have this healthy discussion about pros/cons. Adding Waiman to the discussion as well. Curious to hear what Waiman and Peter think about all this. Some more comments inline. On Thu, May 3, 2018 at 10:19 AM Daniel Colascione <dancol@google.com> wrote: > On Thu, May 3, 2018 at 9:48 AM Joel Fernandes <joelaf@google.com> wrote: > > > > can skip the manual schedule we were going to perform. > > > By the way, if we eventually find a way to enhance user-space mutexes in > > the > > > fashion you describe here, it would belong to another TLS area, and > would > > > be registered by another system call than rseq. I proposed a more > generic > > Right. Also I still don't see any good reason why optimistic spinning in > > the kernel with FUTEX_LOCK, as Peter described, can't be used instead of > > using the rseq implementation and spinning in userspace, for such a case. > I > > don't really fully buy that we need to design this interface assuming any > > privilege transition level time. > > If privilege level transitions are slow, > > we're going to have bad performance anyway. > That's not the case. There's a large class of program that does useful work > while seldom entering the kernel: just ask the user-space network stack > people. Yes, I am aware of that. I was just saying in general, a system such as an Android embedded system, not an HPC based system does make a lot of system calls. I am not arguing that doing more things in userspace is good or bad here. I am just talking about why do something else for no good reasons (see below) when work has already been done on this area. > It's not wise to design interfaces around system calls being cheap. Even if > system calls are currently cheap enough on some architectures some of the > time, there's no guarantee that they'll stay that way, especially relative > to straight-line user-mode execution. A pure user-space approach, on the > other hand, involves no work in the kernel, and doing nothing is always the > optimal strategy. Besides, there are environments where system calls end up > being more expensive than you might think: consider strace or rr. If the > kernel needs to get involved on some path, it's best that its involvement > be as light as possible. Ofcourse, but I think we shouldn't do a premature optimization here without real data on typical Android devices about the cost of system calls entry/exit, vs spin time. I am not against userspace lock based on rseq if there is data and good reason, before investing significant time on reinventing the wheel. > > we should really stick to using FUTEX_LOCK and > > reuse all the work that went into that area for Android and otherwise (and > > work with Waiman and others on improving that if there are any problems > > with it). > FUTEX_LOCK is a return to the bad old days when systems gave you a fixed > list of synchronization primitives and if you wanted something else, tough. I am not saying we should fix sync. primitives made available to userspace, or anything. I am talking about yours/our usecase and whether another sync primitive interface is needed. For example, have another syscall to register TLS area is a new interface, vs using the existing futex interface. Linus is also against adding new sycalls unnecessarily. > That the latest version of the FUTEX_LOCK patch includes a separate > FUTEX_LOCK_SHARED mode is concerning. The functionality the kernel provides Why? That's just for reader-locks. What's the concern there? I know you had something in mind about efficient userspace rw locks but I am curious either way what you have in mind. > to userspace should be more general-purpose and allow more experimentation > without changes in the kernel. I see no reason to force userspace into 1) > reserving 30 bits of its lockword for a TID and 2) adopting the kernel's Based on our offline chat, this is for only 32-bit only systems though right? Also based on Peter's idea of putting the recursion counter outside, there shouldn't be a space issue? > idea of spin time heuristics and lock stealing when the same basic > functionality can be provided in a generic way while reserving only one > bit. That this mechanism happens to be more efficient as well is a bonus. And also probably easy to get wrong. Heuristics are hard and it would be good to work with community on getting best approach for that and improving existing code. Also about "generic way", that's even more reason in my view to do it in the kernel. > "Mechanism not policy" is still a good design principle. Again, I am not advocating forcing of interfaces anything, but I'm against reinventing the wheel and am all for spending time on improving existing things. thanks! - Joel
WARNING: multiple messages have this Message-ID (diff)
From: Joel Fernandes <joelaf@google.com> To: Daniel Colascione <dancol@google.com> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Peter Zijlstra <peterz@infradead.org>, Paul McKenney <paulmck@linux.vnet.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, Andy Lutomirski <luto@amacapital.net>, davejwatson@fb.com, LKML <linux-kernel@vger.kernel.org>, linux-api@vger.kernel.org, Paul Turner <pjt@google.com>, Andrew Morton <akpm@linux-foundation.org>, linux@arm.linux.org.uk, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, hpa@zytor.com, Andrew Hunter <ahh@google.com>, andi@firstfloor.org, cl@linux.com, bmaurer@fb.com, Steven Rostedt <rostedt@goodmis.org>, Josh Triplett <josh@joshtriplett.org>, torvalds@linux-foundation.org, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, mtk.manpages@gmail.com Subject: Re: [RFC PATCH for 4.18 00/14] Restartable Sequences Date: Thu, 03 May 2018 17:46:55 +0000 [thread overview] Message-ID: <CAJWu+opkzpmwY5jJ0353JKyaOFCqa95AKGhxUdQZ6pZf_omsaQ@mail.gmail.com> (raw) In-Reply-To: <CAKOZueve1K8bmS0pmeCAAburoJtzRA4DeN=OCyW3kHiNo8WO=A@mail.gmail.com> Hi Daniel, Nice to have this healthy discussion about pros/cons. Adding Waiman to the discussion as well. Curious to hear what Waiman and Peter think about all this. Some more comments inline. On Thu, May 3, 2018 at 10:19 AM Daniel Colascione <dancol@google.com> wrote: > On Thu, May 3, 2018 at 9:48 AM Joel Fernandes <joelaf@google.com> wrote: > > > > can skip the manual schedule we were going to perform. > > > By the way, if we eventually find a way to enhance user-space mutexes in > > the > > > fashion you describe here, it would belong to another TLS area, and > would > > > be registered by another system call than rseq. I proposed a more > generic > > Right. Also I still don't see any good reason why optimistic spinning in > > the kernel with FUTEX_LOCK, as Peter described, can't be used instead of > > using the rseq implementation and spinning in userspace, for such a case. > I > > don't really fully buy that we need to design this interface assuming any > > privilege transition level time. > > If privilege level transitions are slow, > > we're going to have bad performance anyway. > That's not the case. There's a large class of program that does useful work > while seldom entering the kernel: just ask the user-space network stack > people. Yes, I am aware of that. I was just saying in general, a system such as an Android embedded system, not an HPC based system does make a lot of system calls. I am not arguing that doing more things in userspace is good or bad here. I am just talking about why do something else for no good reasons (see below) when work has already been done on this area. > It's not wise to design interfaces around system calls being cheap. Even if > system calls are currently cheap enough on some architectures some of the > time, there's no guarantee that they'll stay that way, especially relative > to straight-line user-mode execution. A pure user-space approach, on the > other hand, involves no work in the kernel, and doing nothing is always the > optimal strategy. Besides, there are environments where system calls end up > being more expensive than you might think: consider strace or rr. If the > kernel needs to get involved on some path, it's best that its involvement > be as light as possible. Ofcourse, but I think we shouldn't do a premature optimization here without real data on typical Android devices about the cost of system calls entry/exit, vs spin time. I am not against userspace lock based on rseq if there is data and good reason, before investing significant time on reinventing the wheel. > > we should really stick to using FUTEX_LOCK and > > reuse all the work that went into that area for Android and otherwise (and > > work with Waiman and others on improving that if there are any problems > > with it). > FUTEX_LOCK is a return to the bad old days when systems gave you a fixed > list of synchronization primitives and if you wanted something else, tough. I am not saying we should fix sync. primitives made available to userspace, or anything. I am talking about yours/our usecase and whether another sync primitive interface is needed. For example, have another syscall to register TLS area is a new interface, vs using the existing futex interface. Linus is also against adding new sycalls unnecessarily. > That the latest version of the FUTEX_LOCK patch includes a separate > FUTEX_LOCK_SHARED mode is concerning. The functionality the kernel provides Why? That's just for reader-locks. What's the concern there? I know you had something in mind about efficient userspace rw locks but I am curious either way what you have in mind. > to userspace should be more general-purpose and allow more experimentation > without changes in the kernel. I see no reason to force userspace into 1) > reserving 30 bits of its lockword for a TID and 2) adopting the kernel's Based on our offline chat, this is for only 32-bit only systems though right? Also based on Peter's idea of putting the recursion counter outside, there shouldn't be a space issue? > idea of spin time heuristics and lock stealing when the same basic > functionality can be provided in a generic way while reserving only one > bit. That this mechanism happens to be more efficient as well is a bonus. And also probably easy to get wrong. Heuristics are hard and it would be good to work with community on getting best approach for that and improving existing code. Also about "generic way", that's even more reason in my view to do it in the kernel. > "Mechanism not policy" is still a good design principle. Again, I am not advocating forcing of interfaces anything, but I'm against reinventing the wheel and am all for spending time on improving existing things. thanks! - Joel
next prev parent reply other threads:[~2018-05-03 17:47 UTC|newest] Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-30 22:44 [RFC PATCH for 4.18 00/14] Restartable Sequences Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 01/14] uapi headers: Provide types_32_64.h (v2) Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 02/14] rseq: Introduce restartable sequences system call (v13) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-05-16 16:24 ` Peter Zijlstra 2018-05-16 16:24 ` Peter Zijlstra 2018-05-16 20:18 ` Mathieu Desnoyers 2018-05-16 20:18 ` Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 03/14] arm: Add restartable sequences support Mathieu Desnoyers 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-17 13:32 ` Will Deacon 2018-05-17 13:32 ` Will Deacon 2018-05-17 15:30 ` Mathieu Desnoyers 2018-05-17 15:30 ` Mathieu Desnoyers 2018-05-22 18:19 ` Mathieu Desnoyers 2018-05-22 18:19 ` Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 04/14] arm: Wire up restartable sequences system call Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 05/14] x86: Add support for restartable sequences (v2) Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 06/14] x86: Wire up restartable sequence system call Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 07/14] powerpc: Add support for restartable sequences Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-17 1:19 ` Boqun Feng 2018-05-17 1:19 ` Boqun Feng 2018-05-17 1:19 ` Boqun Feng 2018-05-17 7:43 ` Peter Zijlstra 2018-05-17 7:43 ` Peter Zijlstra 2018-05-17 15:28 ` Mathieu Desnoyers 2018-05-17 15:28 ` Mathieu Desnoyers 2018-05-17 23:50 ` Boqun Feng 2018-05-17 23:50 ` Boqun Feng 2018-05-18 18:17 ` Mathieu Desnoyers 2018-05-18 18:17 ` Mathieu Desnoyers 2018-05-20 14:08 ` Boqun Feng 2018-05-20 14:08 ` Boqun Feng 2018-05-20 14:08 ` Boqun Feng 2018-05-23 20:14 ` Mathieu Desnoyers 2018-05-23 20:14 ` Mathieu Desnoyers 2018-05-23 20:46 ` Paul E. McKenney 2018-05-23 20:46 ` Paul E. McKenney 2018-05-23 21:29 ` Mathieu Desnoyers 2018-05-23 21:29 ` Mathieu Desnoyers 2018-05-24 1:03 ` Michael Ellerman 2018-05-24 1:03 ` Michael Ellerman 2018-05-28 7:00 ` Mathieu Desnoyers 2018-05-28 7:00 ` Mathieu Desnoyers 2018-05-18 12:38 ` Michael Ellerman 2018-05-18 12:38 ` Michael Ellerman 2018-04-30 22:44 ` [PATCH 08/14] powerpc: Wire up restartable sequences system call Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 09/14] selftests: lib.mk: Introduce OVERRIDE_TARGETS Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 10/14] rseq: selftests: Provide rseq library (v5) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 11/14] rseq: selftests: Provide basic test Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 12/14] rseq: selftests: Provide basic percpu ops test (v2) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 13/14] rseq: selftests: Provide parametrized tests (v2) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 14/14] rseq: selftests: Provide Makefile, scripts, gitignore (v2) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-05-02 3:53 ` [RFC PATCH for 4.18 00/14] Restartable Sequences Daniel Colascione 2018-05-02 8:43 ` Peter Zijlstra 2018-05-02 16:03 ` Mathieu Desnoyers 2018-05-02 16:03 ` Mathieu Desnoyers 2018-05-02 16:07 ` Daniel Colascione 2018-05-02 16:42 ` Steven Rostedt 2018-05-02 16:55 ` Daniel Colascione 2018-05-03 16:12 ` Mathieu Desnoyers 2018-05-03 16:12 ` Mathieu Desnoyers 2018-05-03 16:22 ` Daniel Colascione 2018-05-03 18:04 ` Mathieu Desnoyers 2018-05-03 18:04 ` Mathieu Desnoyers 2018-05-03 16:48 ` Joel Fernandes 2018-05-03 17:18 ` Daniel Colascione 2018-05-03 17:46 ` Joel Fernandes [this message] 2018-05-03 17:46 ` Joel Fernandes 2018-05-04 22:17 ` Ben Maurer 2018-05-04 22:17 ` Ben Maurer 2018-05-02 17:22 ` Peter Zijlstra 2018-05-02 18:27 ` Daniel Colascione 2018-05-02 20:22 ` Peter Zijlstra 2018-05-02 20:37 ` Daniel Colascione 2018-05-03 1:15 ` Steven Rostedt 2018-05-03 8:49 ` Peter Zijlstra 2018-05-06 10:03 ` Thomas Gleixner
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=CAJWu+opkzpmwY5jJ0353JKyaOFCqa95AKGhxUdQZ6pZf_omsaQ@mail.gmail.com \ --to=joelaf@google.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=dancol@google.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=longman@redhat.com \ --cc=luto@amacapital.net \ --cc=mathieu.desnoyers@efficios.com \ --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=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.