From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757631AbdJKS6G convert rfc822-to-8bit (ORCPT ); Wed, 11 Oct 2017 14:58:06 -0400 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 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B0555C058EC0 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=longman@redhat.com Subject: Re: [PATCH v6 02/11] locking/rwsem: Implement a new locking scheme 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 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 Organization: Red Hat Message-ID: <29f33a68-bf01-e0f0-158f-3bae3a680e80@redhat.com> Date: Wed, 11 Oct 2017 14:58:02 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 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 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 11 Oct 2017 18:58:03 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@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