Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-20 Thread Waiman Long
On 05/18/2016 09:37 PM, Dave Chinner wrote: On Fri, May 06, 2016 at 08:20:24PM -0400, Waiman Long wrote: Currently, it is not possible to determine for sure if a reader owns a rwsem by looking at the content of the rwsem data structure. This patch adds a new state RWSEM_READER_OWNED to the owner

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-19 Thread Paul E. McKenney
On Thu, May 19, 2016 at 11:00:13AM +0200, Peter Zijlstra wrote: > On Wed, May 18, 2016 at 10:26:06AM -0700, Paul E. McKenney wrote: > > On Wed, May 18, 2016 at 01:05:55PM +0200, Peter Zijlstra wrote: > > > > Alternatively, could we try and talk to our GCC friends to make sure GCC > > > doesn't tea

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-19 Thread Peter Zijlstra
On Wed, May 18, 2016 at 10:26:06AM -0700, Paul E. McKenney wrote: > On Wed, May 18, 2016 at 01:05:55PM +0200, Peter Zijlstra wrote: > > Alternatively, could we try and talk to our GCC friends to make sure GCC > > doesn't tear loads/stores irrespective of what the C language spec > > allows? > > I

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-19 Thread Peter Zijlstra
On Thu, May 19, 2016 at 11:37:53AM +1000, Dave Chinner wrote: > On Fri, May 06, 2016 at 08:20:24PM -0400, Waiman Long wrote: > > Currently, it is not possible to determine for sure if a reader > > owns a rwsem by looking at the content of the rwsem data structure. > > This patch adds a new state RW

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-18 Thread Dave Chinner
On Fri, May 06, 2016 at 08:20:24PM -0400, Waiman Long wrote: > Currently, it is not possible to determine for sure if a reader > owns a rwsem by looking at the content of the rwsem data structure. > This patch adds a new state RWSEM_READER_OWNED to the owner field > to indicate that readers current

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-18 Thread Paul E. McKenney
On Wed, May 18, 2016 at 11:56:39AM -0400, Waiman Long wrote: > On 05/18/2016 07:05 AM, Peter Zijlstra wrote: > >On Tue, May 17, 2016 at 12:46:07PM -0700, Paul E. McKenney wrote: > >>Actually, if you show a case where this makes a visible system-wide > >>difference, you could create a set of primiti

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-18 Thread Paul E. McKenney
On Wed, May 18, 2016 at 01:05:55PM +0200, Peter Zijlstra wrote: > On Tue, May 17, 2016 at 12:46:07PM -0700, Paul E. McKenney wrote: > > Actually, if you show a case where this makes a visible system-wide > > difference, you could create a set of primitives for #1 below. Have > > a compiler version

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-18 Thread Waiman Long
On 05/18/2016 07:05 AM, Peter Zijlstra wrote: On Tue, May 17, 2016 at 12:46:07PM -0700, Paul E. McKenney wrote: Actually, if you show a case where this makes a visible system-wide difference, you could create a set of primitives for #1 below. Have a compiler version check, and if it is an old c

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-18 Thread Peter Zijlstra
On Tue, May 17, 2016 at 12:46:07PM -0700, Paul E. McKenney wrote: > Actually, if you show a case where this makes a visible system-wide > difference, you could create a set of primitives for #1 below. Have > a compiler version check, and if it is an old compiler, map them to > READ_ONCE() and WRIT

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-17 Thread Peter Hurley
Hi Paul, You can disregard this as I think we're talking about the same things with the other email thread. Regards, Peter Hurley On 05/17/2016 12:46 PM, Peter Hurley wrote: > On 05/16/2016 10:22 AM, Paul E. McKenney wrote: >> On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote: >>> On

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-17 Thread Peter Hurley
On 05/16/2016 10:22 AM, Paul E. McKenney wrote: > On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote: >> On 05/16/2016 05:17 AM, Paul E. McKenney wrote: >>> On Mon, May 16, 2016 at 01:09:48PM +0200, Peter Zijlstra wrote: On Fri, May 13, 2016 at 10:58:05AM -0700, Peter Hurley wrote: >

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-17 Thread Paul E. McKenney
On Tue, May 17, 2016 at 12:15:20PM -0700, Peter Hurley wrote: > On 05/16/2016 10:50 AM, Peter Zijlstra wrote: > > On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote: > > > Correct; which is why we should always use {READ,WRITE}_ONCE() for > anything that is used locklessly. > >

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-17 Thread Peter Hurley
On 05/16/2016 10:50 AM, Peter Zijlstra wrote: > On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote: > Correct; which is why we should always use {READ,WRITE}_ONCE() for anything that is used locklessly. >>> >>> Completely agreed. Improve readability of code by flagging lockles

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-16 Thread Peter Zijlstra
On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote: > >> Correct; which is why we should always use {READ,WRITE}_ONCE() for > >> anything that is used locklessly. > > > > Completely agreed. Improve readability of code by flagging lockless > > shared-memory accesses, help checkers bette

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-16 Thread Paul E. McKenney
On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote: > On 05/16/2016 05:17 AM, Paul E. McKenney wrote: > > On Mon, May 16, 2016 at 01:09:48PM +0200, Peter Zijlstra wrote: > >> On Fri, May 13, 2016 at 10:58:05AM -0700, Peter Hurley wrote: > Note that barrier() and READ_ONCE() have over

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-16 Thread Peter Hurley
On 05/16/2016 05:17 AM, Paul E. McKenney wrote: > On Mon, May 16, 2016 at 01:09:48PM +0200, Peter Zijlstra wrote: >> On Fri, May 13, 2016 at 10:58:05AM -0700, Peter Hurley wrote: Note that barrier() and READ_ONCE() have overlapping but not identical results and the combined use actually m

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-16 Thread Paul E. McKenney
On Mon, May 16, 2016 at 01:09:48PM +0200, Peter Zijlstra wrote: > On Fri, May 13, 2016 at 10:58:05AM -0700, Peter Hurley wrote: > > > Note that barrier() and READ_ONCE() have overlapping but not identical > > > results and the combined use actually makes sense here. > > > > > > Yes, a barrier() an

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-16 Thread Peter Zijlstra
On Fri, May 13, 2016 at 10:58:05AM -0700, Peter Hurley wrote: > > Note that barrier() and READ_ONCE() have overlapping but not identical > > results and the combined use actually makes sense here. > > > > Yes, a barrier() anywhere in the loop will force a reload of the > > variable, _however_ it d

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-15 Thread Waiman Long
On 05/13/2016 01:58 PM, Peter Hurley wrote: On 05/13/2016 08:07 AM, Peter Zijlstra wrote: On Wed, May 11, 2016 at 03:04:20PM -0700, Peter Hurley wrote: + return !rwsem_is_reader_owned(READ_ONCE(sem->owner)); It doesn't make sense to force reload sem->owner here; if sem->owner is not bein

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-13 Thread Peter Hurley
On 05/13/2016 08:07 AM, Peter Zijlstra wrote: > On Wed, May 11, 2016 at 03:04:20PM -0700, Peter Hurley wrote: >>> + return !rwsem_is_reader_owned(READ_ONCE(sem->owner)); >> >> It doesn't make sense to force reload sem->owner here; if sem->owner >> is not being reloaded then the loop above will ex

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-13 Thread Peter Zijlstra
On Wed, May 11, 2016 at 03:04:20PM -0700, Peter Hurley wrote: > > + return !rwsem_is_reader_owned(READ_ONCE(sem->owner)); > > It doesn't make sense to force reload sem->owner here; if sem->owner > is not being reloaded then the loop above will execute forever. > > Arguably, this check should be

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-12 Thread Waiman Long
On 05/12/2016 05:27 PM, Peter Hurley wrote: Arguably, this check should be bumped out to the optimistic spin and reload/check the owner there? Or better yet; don't pass the owner in as a parameter at all, but instead snapshot the owner and check its ownership on entry. That will make the main o

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-12 Thread Peter Hurley
On 05/12/2016 01:15 PM, Waiman Long wrote: > On 05/11/2016 06:04 PM, Peter Hurley wrote: [...] >> >>> @@ -328,8 +329,6 @@ done: >>> static noinline >>> bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct >>> *owner) >>> { >>> -long count; >>> - >>> rcu_read_lock

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-12 Thread Waiman Long
On 05/11/2016 06:04 PM, Peter Hurley wrote: /* Grant an infinite number of read locks to the readers at the front @@ -306,16 +312,11 @@ static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem) rcu_read_lock(); owner = READ_ONCE(sem->owner); - if (!owner

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-11 Thread Peter Hurley
On 05/06/2016 05:20 PM, Waiman Long wrote: > Currently, it is not possible to determine for sure if a reader > owns a rwsem by looking at the content of the rwsem data structure. > This patch adds a new state RWSEM_READER_OWNED to the owner field > to indicate that readers currently own the lock. T

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-10 Thread Davidlohr Bueso
On Fri, 06 May 2016, Waiman Long wrote: Currently, it is not possible to determine for sure if a reader owns a rwsem by looking at the content of the rwsem data structure. This patch adds a new state RWSEM_READER_OWNED to the owner field to indicate that readers currently own the lock. This enab

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-10 Thread Peter Zijlstra
On Mon, May 09, 2016 at 10:24:16PM -0400, Waiman Long wrote: > >>+static inline void rwsem_set_reader_owned(struct rw_semaphore *sem) > >>+{ > >>+ /* > >>+* We check the owner value first to make sure that we will only > >>+* do a write to the rwsem cacheline when it is really necessary >

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-09 Thread Waiman Long
On 05/09/2016 04:27 AM, Peter Zijlstra wrote: On Fri, May 06, 2016 at 08:20:24PM -0400, Waiman Long wrote: @@ -391,9 +386,11 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) * When there's no owner, we might have preempted between the * owner acqui

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-09 Thread Jason Low
On Fri, 2016-05-06 at 20:20 -0400, Waiman Long wrote: > Currently, it is not possible to determine for sure if a reader > owns a rwsem by looking at the content of the rwsem data structure. > This patch adds a new state RWSEM_READER_OWNED to the owner field > to indicate that readers currently own

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-09 Thread Peter Zijlstra
On Fri, May 06, 2016 at 08:20:24PM -0400, Waiman Long wrote: > @@ -391,9 +386,11 @@ static bool rwsem_optimistic_spin(struct rw_semaphore > *sem) >* When there's no owner, we might have preempted between the >* owner acquiring the lock and setting the owner field. I

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-07 Thread Waiman Long
On 05/07/2016 12:56 AM, Ingo Molnar wrote: * Waiman Long wrote: On a 4-socket Haswell machine running on a 4.6-rc1 based kernel, the fio test with multithreaded randrw and randwrite tests on the same file on a XFS partition on top of a NVDIMM were run, the aggregated bandwidths before and afte

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-06 Thread Ingo Molnar
* Waiman Long wrote: > On a 4-socket Haswell machine running on a 4.6-rc1 based kernel, the > fio test with multithreaded randrw and randwrite tests on the same > file on a XFS partition on top of a NVDIMM were run, the aggregated > bandwidths before and after the patch were as follows: > > T

[PATCH v2] locking/rwsem: Add reader-owned state to the owner field

2016-05-06 Thread Waiman Long
Currently, it is not possible to determine for sure if a reader owns a rwsem by looking at the content of the rwsem data structure. This patch adds a new state RWSEM_READER_OWNED to the owner field to indicate that readers currently own the lock. This enables us to address the following 2 issues in