Re: [PATCH v2 0/9] Remove spin_unlock_wait()

Hi Alan, On 07/08/2017 06:21 PM, Alan Stern wrote: Pardon me for barging in, but I found this whole interchange extremely confusing... On Sat, 8 Jul 2017, Ingo Molnar wrote: * Paul E. McKenney wrote: On Sat, Jul 08, 2017 at 10:35:43AM +0200, Ingo Molnar wrote:

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

Pardon me for barging in, but I found this whole interchange extremely confusing... On Sat, 8 Jul 2017, Ingo Molnar wrote: > * Paul E. McKenney wrote: > > > On Sat, Jul 08, 2017 at 10:35:43AM +0200, Ingo Molnar wrote: > > > > > > * Manfred Spraul

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Sat, Jul 08, 2017 at 02:30:19PM +0200, Ingo Molnar wrote: > > * Paul E. McKenney wrote: > > > On Sat, Jul 08, 2017 at 10:35:43AM +0200, Ingo Molnar wrote: > > > > > > * Manfred Spraul wrote: > > > > > > > Hi Ingo, > > > > > > > > On

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Paul E. McKenney wrote: > On Sat, Jul 08, 2017 at 10:35:43AM +0200, Ingo Molnar wrote: > > > > * Manfred Spraul wrote: > > > > > Hi Ingo, > > > > > > On 07/07/2017 10:31 AM, Ingo Molnar wrote: > > > > > > > > There's another, probably

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Sat, Jul 08, 2017 at 10:43:24AM +0200, Ingo Molnar wrote: > > * Paul E. McKenney wrote: > > > On Fri, Jul 07, 2017 at 10:31:28AM +0200, Ingo Molnar wrote: > > > > [ . . . ] > > > > > In fact I'd argue that any future high performance spin_unlock_wait() > > >

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Sat, Jul 08, 2017 at 10:35:43AM +0200, Ingo Molnar wrote: > > * Manfred Spraul wrote: > > > Hi Ingo, > > > > On 07/07/2017 10:31 AM, Ingo Molnar wrote: > > > > > > There's another, probably just as significant advantage: > > > queued_spin_unlock_wait() > > > is

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Paul E. McKenney wrote: > On Fri, Jul 07, 2017 at 10:31:28AM +0200, Ingo Molnar wrote: > > [ . . . ] > > > In fact I'd argue that any future high performance spin_unlock_wait() user > > is > > probably better off open coding the unlock-wait poll loop (and

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Manfred Spraul wrote: > Hi Ingo, > > On 07/07/2017 10:31 AM, Ingo Molnar wrote: > > > > There's another, probably just as significant advantage: > > queued_spin_unlock_wait() > > is 'read-only', while spin_lock()+spin_unlock() dirties the lock cache > > line. On

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

Hi Ingo, On 07/07/2017 10:31 AM, Ingo Molnar wrote: There's another, probably just as significant advantage: queued_spin_unlock_wait() is 'read-only', while spin_lock()+spin_unlock() dirties the lock cache line. On any bigger system this should make a very measurable difference - if

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Fri, Jul 07, 2017 at 10:31:28AM +0200, Ingo Molnar wrote: [ . . . ] > In fact I'd argue that any future high performance spin_unlock_wait() user is > probably better off open coding the unlock-wait poll loop (and possibly > thinking > hard about eliminating it altogether). If such patterns

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Fri, Jul 07, 2017 at 12:33:49PM +0200, Ingo Molnar wrote: > [1997/04] v2.1.36: > > the spin_unlock_wait() primitive gets introduced as part of release() Whee, that goes _way_ further back than I thought it did :-) > [2017/07] v4.12: > > wait_task_inactive() is still alive

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Peter Zijlstra wrote: > You missed the one in do_exit(), which I thought was the original one. Indeed, it's raw_spin_unlock_wait() which my git grep pattern missed. But it's not the original spin_unlock_wait(): the pi_lock and priority inheritance is a newfangled

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Ingo Molnar wrote: > > * Peter Zijlstra wrote: > > > > It might even be that this is the defined semantics of spin_unlock_wait(). > > > > As is, spin_unlock_wait() is somewhat ill defined. IIRC it grew from an > > optimization by Oleg and

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Fri, Jul 07, 2017 at 10:31:28AM +0200, Ingo Molnar wrote: > Here's a quick list of all the use cases: > > net/netfilter/nf_conntrack_core.c: > >- This is I believe the 'original', historic spin_unlock_wait() usecase > that > still exists in the kernel. spin_unlock_wait() is only

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 09:20:24AM -0700, Paul E. McKenney wrote: > > On Thu, Jul 06, 2017 at 06:05:55PM +0200, Peter Zijlstra wrote: > > > On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > > > > From: Paul E. McKenney > > > > [ . .

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

* Peter Zijlstra wrote: > > It might even be that this is the defined semantics of spin_unlock_wait(). > > As is, spin_unlock_wait() is somewhat ill defined. IIRC it grew from an > optimization by Oleg and subsequently got used elsewhere. And it being the > subtle

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, 6 Jul 2017, Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 12:49:12PM -0400, Alan Stern wrote: > > On Thu, 6 Jul 2017, Paul E. McKenney wrote: > > > > > On Thu, Jul 06, 2017 at 06:10:47PM +0200, Peter Zijlstra wrote: > > > > On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 06:08:50PM +0100, Will Deacon wrote: > On Thu, Jul 06, 2017 at 06:50:36PM +0200, Peter Zijlstra wrote: > > On Thu, Jul 06, 2017 at 09:20:24AM -0700, Paul E. McKenney wrote: > > > On Thu, Jul 06, 2017 at 06:05:55PM +0200, Peter Zijlstra wrote: > > > > On Thu, Jul 06, 2017 at

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 06:50:36PM +0200, Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 09:20:24AM -0700, Paul E. McKenney wrote: > > On Thu, Jul 06, 2017 at 06:05:55PM +0200, Peter Zijlstra wrote: > > > On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > > > > From: Paul E. McKenney

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 06:50:36PM +0200, Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 09:20:24AM -0700, Paul E. McKenney wrote: > > On Thu, Jul 06, 2017 at 06:05:55PM +0200, Peter Zijlstra wrote: > > > On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > > > > From: Paul E. McKenney

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 06:41:34PM +0200, Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 09:24:12AM -0700, Paul E. McKenney wrote: > > On Thu, Jul 06, 2017 at 06:10:47PM +0200, Peter Zijlstra wrote: > > > On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney wrote: > > > > And yes, there are

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 12:49:12PM -0400, Alan Stern wrote: > On Thu, 6 Jul 2017, Paul E. McKenney wrote: > > > On Thu, Jul 06, 2017 at 06:10:47PM +0200, Peter Zijlstra wrote: > > > On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney wrote: > > > > And yes, there are architecture-specific

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 09:20:24AM -0700, Paul E. McKenney wrote: > On Thu, Jul 06, 2017 at 06:05:55PM +0200, Peter Zijlstra wrote: > > On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > > > From: Paul E. McKenney > > [ . . . ] > > > Now on the one hand I feel like Oleg that it

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, 6 Jul 2017, Paul E. McKenney wrote: > On Thu, Jul 06, 2017 at 06:10:47PM +0200, Peter Zijlstra wrote: > > On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney wrote: > > > And yes, there are architecture-specific optimizations for an > > > empty spin_lock()/spin_unlock() critical

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 09:24:12AM -0700, Paul E. McKenney wrote: > On Thu, Jul 06, 2017 at 06:10:47PM +0200, Peter Zijlstra wrote: > > On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney wrote: > > > And yes, there are architecture-specific optimizations for an > > > empty

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 06:10:47PM +0200, Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney wrote: > > And yes, there are architecture-specific optimizations for an > > empty spin_lock()/spin_unlock() critical section, and the current > > arch_spin_unlock_wait()

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 06:05:55PM +0200, Peter Zijlstra wrote: > On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > > From: Paul E. McKenney [ . . . ] > Now on the one hand I feel like Oleg that it would be a shame to loose > the optimization, OTOH this thing is really really

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 08:21:10AM -0700, Paul E. McKenney wrote: > And yes, there are architecture-specific optimizations for an > empty spin_lock()/spin_unlock() critical section, and the current > arch_spin_unlock_wait() implementations show some of these optimizations. > But I expect that

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > From: Paul E. McKenney > > Sent: 06 July 2017 00:30 > > There is no agreed-upon definition of spin_unlock_wait()'s semantics, > > and it appears that all callers could do just as well with a lock/unlock > > pair. This series

Re: [PATCH v2 0/9] Remove spin_unlock_wait()

On Thu, Jul 06, 2017 at 02:12:24PM +, David Laight wrote: > From: Paul E. McKenney > > Sent: 06 July 2017 00:30 > > There is no agreed-upon definition of spin_unlock_wait()'s semantics, > > and it appears that all callers could do just as well with a lock/unlock > > pair. This series

RE: [PATCH v2 0/9] Remove spin_unlock_wait()

From: Paul E. McKenney > Sent: 06 July 2017 00:30 > There is no agreed-upon definition of spin_unlock_wait()'s semantics, > and it appears that all callers could do just as well with a lock/unlock > pair. This series therefore removes spin_unlock_wait() and changes > its users to instead use a

[PATCH v2 0/9] Remove spin_unlock_wait()

Hello! There is no agreed-upon definition of spin_unlock_wait()'s semantics, and it appears that all callers could do just as well with a lock/unlock pair. This series therefore removes spin_unlock_wait() and changes its users to instead use a lock/unlock pair. The commits are as follows, in