From mboxrd@z Thu Jan 1 00:00:00 1970 From: Waiman Long Subject: Re: [PATCH v6 02/11] locking/rwsem: Implement a new locking scheme Date: Wed, 11 Oct 2017 14:58:02 -0400 Message-ID: <29f33a68-bf01-e0f0-158f-3bae3a680e80@redhat.com> References: <1507744922-15196-1-git-send-email-longman@redhat.com> <1507744922-15196-3-git-send-email-longman@redhat.com> <20171011184038.pilwo7sjf72nwaxk@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20171011184038.pilwo7sjf72nwaxk@hirez.programming.kicks-ass.net> Content-Language: en-US Sender: linux-alpha-owner@vger.kernel.org To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org, x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, Davidlohr Bueso , Dave Chinner List-Id: linux-arch.vger.kernel.org On 10/11/2017 02:40 PM, Peter Zijlstra wrote: > On Wed, Oct 11, 2017 at 02:01:53PM -0400, Waiman Long wrote: >> +/* >> + * The definition of the atomic counter in the semaphore: >> + * >> + * Bit 0 - writer locked bit >> + * Bit 1 - waiters present bit >> + * Bits 2-7 - reserved >> + * Bits 8-31 - 24-bit reader count >> + * >> + * atomic_fetch_add() is used to obtain reader lock, whereas atomic_cmpxchg() >> + * will be used to obtain writer lock. >> + */ >> +#define RWSEM_WRITER_LOCKED 0X00000001 >> +#define RWSEM_FLAG_WAITERS 0X00000002 >> +#define RWSEM_READER_BIAS 0x00000100 >> +#define RWSEM_READER_SHIFT 8 >> +#define RWSEM_READER_MASK (~((1U << RWSEM_READER_SHIFT) - 1)) >> +#define RWSEM_LOCK_MASK (RWSEM_WRITER_LOCKED|RWSEM_READER_MASK) >> +#define RWSEM_READ_FAILED_MASK (RWSEM_WRITER_LOCKED|RWSEM_FLAG_WAITERS) >> + >> +#define RWSEM_COUNT_IS_LOCKED(c) ((c) & RWSEM_LOCK_MASK) >> + >> +/* >> + * lock for reading >> + */ >> +static inline void __down_read(struct rw_semaphore *sem) >> +{ >> + if (unlikely(atomic_fetch_add_acquire(RWSEM_READER_BIAS, &sem->count) >> + & RWSEM_READ_FAILED_MASK)) >> + rwsem_down_read_failed(sem); >> +} > So I implemented rwsem-mutex (also qrwlock based) that puts > > (unsigned long)current | RWSEM_WRITER > > in the atomic_long_t rw_semaphore::owner field. The down-side is that > you can't do fetch_add based __down_read, because that would clobber the > pointer. The up-side is that we have a stable owner pointer (which is > what I needed for PI like things). Without fetch_add for readers, it could lead to reduced performance for reader heavy workloads. Are you trying to do a PI version of rwsem? It can work when the lock is writer owned, but not when it is reader owned. Cheers, Longman From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:54462 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752096AbdJKS6D (ORCPT ); Wed, 11 Oct 2017 14:58:03 -0400 Subject: Re: [PATCH v6 02/11] locking/rwsem: Implement a new locking scheme References: <1507744922-15196-1-git-send-email-longman@redhat.com> <1507744922-15196-3-git-send-email-longman@redhat.com> <20171011184038.pilwo7sjf72nwaxk@hirez.programming.kicks-ass.net> From: Waiman Long Message-ID: <29f33a68-bf01-e0f0-158f-3bae3a680e80@redhat.com> Date: Wed, 11 Oct 2017 14:58:02 -0400 MIME-Version: 1.0 In-Reply-To: <20171011184038.pilwo7sjf72nwaxk@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Content-Language: en-US Sender: linux-arch-owner@vger.kernel.org List-ID: To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org, x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, Davidlohr Bueso , Dave Chinner Message-ID: <20171011185802.H6yBI-ftS4JCIfwxxHXgWLbwd7bolHWJteqlblEK83Q@z> On 10/11/2017 02:40 PM, Peter Zijlstra wrote: > On Wed, Oct 11, 2017 at 02:01:53PM -0400, Waiman Long wrote: >> +/* >> + * The definition of the atomic counter in the semaphore: >> + * >> + * Bit 0 - writer locked bit >> + * Bit 1 - waiters present bit >> + * Bits 2-7 - reserved >> + * Bits 8-31 - 24-bit reader count >> + * >> + * atomic_fetch_add() is used to obtain reader lock, whereas atomic_cmpxchg() >> + * will be used to obtain writer lock. >> + */ >> +#define RWSEM_WRITER_LOCKED 0X00000001 >> +#define RWSEM_FLAG_WAITERS 0X00000002 >> +#define RWSEM_READER_BIAS 0x00000100 >> +#define RWSEM_READER_SHIFT 8 >> +#define RWSEM_READER_MASK (~((1U << RWSEM_READER_SHIFT) - 1)) >> +#define RWSEM_LOCK_MASK (RWSEM_WRITER_LOCKED|RWSEM_READER_MASK) >> +#define RWSEM_READ_FAILED_MASK (RWSEM_WRITER_LOCKED|RWSEM_FLAG_WAITERS) >> + >> +#define RWSEM_COUNT_IS_LOCKED(c) ((c) & RWSEM_LOCK_MASK) >> + >> +/* >> + * lock for reading >> + */ >> +static inline void __down_read(struct rw_semaphore *sem) >> +{ >> + if (unlikely(atomic_fetch_add_acquire(RWSEM_READER_BIAS, &sem->count) >> + & RWSEM_READ_FAILED_MASK)) >> + rwsem_down_read_failed(sem); >> +} > So I implemented rwsem-mutex (also qrwlock based) that puts > > (unsigned long)current | RWSEM_WRITER > > in the atomic_long_t rw_semaphore::owner field. The down-side is that > you can't do fetch_add based __down_read, because that would clobber the > pointer. The up-side is that we have a stable owner pointer (which is > what I needed for PI like things). Without fetch_add for readers, it could lead to reduced performance for reader heavy workloads. Are you trying to do a PI version of rwsem? It can work when the lock is writer owned, but not when it is reader owned. Cheers, Longman