Message ID | 20231108105639.70088-1-haifeng.xu@shopee.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:aa0b:0:b0:403:3b70:6f57 with SMTP id k11csp830229vqo; Wed, 8 Nov 2023 02:57:50 -0800 (PST) X-Google-Smtp-Source: AGHT+IGo3PBxEbDw1Fr4ipbeL602miPnciamc825sDP3BtFjq1MDI8GLxl78sWqcGQumuQBIfGmP X-Received: by 2002:a17:902:f547:b0:1cc:32ae:4afd with SMTP id h7-20020a170902f54700b001cc32ae4afdmr1834797plf.46.1699441070138; Wed, 08 Nov 2023 02:57:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699441070; cv=none; d=google.com; s=arc-20160816; b=L0+BUJoo4EW/ItF81YjTEJL//w/iOLmSrq6KZxMvritSYLg199jDQ1Fd8alzOULCTK XJMQrLv2tlS1o/yGtwCuAukmNAwLz91zI7BsohIJZJC6GrSEZnTT5kgyi1fVOnbAlUno EJ+mrLko3/Ffdv/kGgpJYC0sJso6aWySSZ7jDnhxJaJL2PdgxSyHgERydlXIOnGUX3vc YnvgcDdA8lp3LN4CAzXzaShaX1HqiQrmtl8vVLWXYOLfIOimvCLXfKNg7udtrNtgtG7d fuD+Epr6dOM31405aClNT3GH1/+j5XBhWLd/wyp1+aTVuBxkD20ntnEKT94C46go88JA Er6A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=Xn2IA5P26vaovRSBo2ciBaT5wWZFGHvMT0WOwKqY8ag=; fh=rLxKPkQox0gGEOCIEnDaHKd9Ac18/c+nEjxOp/aZaPQ=; b=ROE0nfhAcHpqtx/8rgaLQ2CuSoAhXU7HEqk5nqLjXj0cyc++gWslhBtYY5HCePtwvc 3NUQAFBwk+iH78lkr2N/1nfi/0l9th6m3juVej6UrZqmAcM/N7PQEuq/rg8rNWBIEC1X NckWMxoxJMFBUj2/0WQ2LkwifkS86kdY30iEnxbTbPIAL9kGl+16aPtpo6gAoyAwFu7J ldJ5rTTTMY+S++SS0zyBm9ncg3jMcQyBZhjUJI4j3WyTe1V8aVoK7CmvnTQPUslJXBpN 8qwuAhQ7rmZE/YK8TthEaYlCn7lsxoEZy71nC/kdM8LVFq7e0luN6Awp4w4fTUqqHfw3 rD5w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@shopee.com header.s=shopee.com header.b=kl4br25R; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=shopee.com Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id i2-20020a170902c94200b001b8904eadb8si2152524pla.460.2023.11.08.02.57.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Nov 2023 02:57:50 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@shopee.com header.s=shopee.com header.b=kl4br25R; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=shopee.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 2183C81C50B7; Wed, 8 Nov 2023 02:57:47 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344385AbjKHK5c (ORCPT <rfc822;jaysivo@gmail.com> + 32 others); Wed, 8 Nov 2023 05:57:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54500 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344528AbjKHK5W (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 8 Nov 2023 05:57:22 -0500 Received: from mail-pl1-x62d.google.com (mail-pl1-x62d.google.com [IPv6:2607:f8b0:4864:20::62d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A2B8B19BA for <linux-kernel@vger.kernel.org>; Wed, 8 Nov 2023 02:57:20 -0800 (PST) Received: by mail-pl1-x62d.google.com with SMTP id d9443c01a7336-1cc316ccc38so54938985ad.1 for <linux-kernel@vger.kernel.org>; Wed, 08 Nov 2023 02:57:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shopee.com; s=shopee.com; t=1699441040; x=1700045840; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Xn2IA5P26vaovRSBo2ciBaT5wWZFGHvMT0WOwKqY8ag=; b=kl4br25R6LOG23ScjAqRq3bCG0JzK68uySVKr7cuJcf+vuTTU+CKPZOuPjBHeYU2O7 mUWI8C0EXBP0cQF5bfndLpdkURamngqmvfpVmbRZzSetRlVxk5PFYSobjLorDzA+SXHG cQeW3vwP9J3I5uXXoE91NJciUezQJfq9PHQqUSYFcAA5WFwdzcNPkeTFsjEdveiwnpRq 8oQN/F1myWSTdOQ2uoW38MqlT+TlMOKEmH5KfoShPGFhT73x6XEWEuerm6yOex6NAHbt UjiDxBTAiXEX8YuPRv5lDcimQW9LY2u8grbxg+RayT9I204b5EeZ/ff9gME87VQItPpW vlzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699441040; x=1700045840; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Xn2IA5P26vaovRSBo2ciBaT5wWZFGHvMT0WOwKqY8ag=; b=YmsTg0TsoQqR4A+UUq+NoxSNLBKdVSTzEWWH42Fp0ULpNUBaoMUqGNzZd2mnI8HXTF dUYtqgjX8+lsj6qocB58JWagvw4Pcym8+qnZ0ey94db1TChQQTXEe/hODK5nZpPCxZpo RD5IhJQPz0KZdSDbAuideVzU44Ci/M1F7QhHBshz9jCh8hSAfrN/EUxoetnA+Ofvue2f o7XJpJOWQ1NmOndNScOdo3mQH3nRTBpQzGz8RmXH1apNPv6Wx9IyZlPe7NfUJmQE1s/P yDU6YCiCcHSz5/IGL2TydoYVhL5n6Vij7QvsejB9+djEaHBGyzDkL3VHXi4HbInxSUgU +JMg== X-Gm-Message-State: AOJu0YwqaRe+J/Ws+WLagVC6Fi6KdzvjMV5rzXDvg7/ysj2EJpetIbRr ozKFMxg5eUICn/HwdHpbDXiqRw== X-Received: by 2002:a17:903:11ce:b0:1cc:b09a:b811 with SMTP id q14-20020a17090311ce00b001ccb09ab811mr1864075plh.14.1699441040125; Wed, 08 Nov 2023 02:57:20 -0800 (PST) Received: from ubuntu-hf2.default.svc.cluster.local ([101.127.248.173]) by smtp.gmail.com with ESMTPSA id 12-20020a170902c14c00b001cc79f3c60csm1468546plj.31.2023.11.08.02.57.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Nov 2023 02:57:19 -0800 (PST) From: Haifeng Xu <haifeng.xu@shopee.com> To: longman@redhat.com Cc: peterz@infradead.org, mingo@redhat.com, will@kernel.org, boqun.feng@gmail.com, linux-kernel@vger.kernel.org, Haifeng Xu <haifeng.xu@shopee.com> Subject: [PATCH] locking/rwsem: Remove unnessary check in rwsem_down_read_slowpath() Date: Wed, 8 Nov 2023 10:56:39 +0000 Message-Id: <20231108105639.70088-1-haifeng.xu@shopee.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Wed, 08 Nov 2023 02:57:47 -0800 (PST) X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781993119554856221 X-GMAIL-MSGID: 1781993119554856221 |
Series |
locking/rwsem: Remove unnessary check in rwsem_down_read_slowpath()
|
|
Commit Message
Haifeng Xu
Nov. 8, 2023, 10:56 a.m. UTC
When the owner of rw_semaphore is reader, the count can't be
RWSEM_WRITER_LOCKED, so there is no need to check it.
Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
kernel/locking/rwsem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Comments
On 11/8/23 05:56, Haifeng Xu wrote: > When the owner of rw_semaphore is reader, the count can't be > RWSEM_WRITER_LOCKED, so there is no need to check it. > > Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com> > --- > kernel/locking/rwsem.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c > index 2340b6d90ec6..7a4d8a9ebd9c 100644 > --- a/kernel/locking/rwsem.c > +++ b/kernel/locking/rwsem.c > @@ -1005,8 +1005,7 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat > * waiter, don't attempt optimistic lock stealing if the lock is > * currently owned by readers. > */ > - if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && > - (rcnt > 1) && !(count & RWSEM_WRITER_LOCKED)) > + if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && (rcnt > 1)) > goto queue; > > /* Unlike RWSEM_WRITER_LOCKED bit in count, the RWSEM_READER_OWNED bit in owner is just a hint, not an authoritative state of the rwsem. So it is possible that both the RWSEM_READER_OWNED bit can be set in owner and RWSEM_WRITER_LOCKED bit set in count in a transition period right after RWSEM_WRITER_LOCKED bit is set. So the RWSEM_WRITER_LOCKED check can still provide some value. We should probably update the comment to reflect that. Cheers, Longman
On 2023/11/8 22:04, Waiman Long wrote: > On 11/8/23 05:56, Haifeng Xu wrote: >> When the owner of rw_semaphore is reader, the count can't be >> RWSEM_WRITER_LOCKED, so there is no need to check it. >> >> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com> >> --- >> kernel/locking/rwsem.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c >> index 2340b6d90ec6..7a4d8a9ebd9c 100644 >> --- a/kernel/locking/rwsem.c >> +++ b/kernel/locking/rwsem.c >> @@ -1005,8 +1005,7 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat >> * waiter, don't attempt optimistic lock stealing if the lock is >> * currently owned by readers. >> */ >> - if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && >> - (rcnt > 1) && !(count & RWSEM_WRITER_LOCKED)) >> + if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && (rcnt > 1)) >> goto queue; >> /* > > Unlike RWSEM_WRITER_LOCKED bit in count, the RWSEM_READER_OWNED bit in owner is just a hint, not an authoritative state of the rwsem. So it is possible that both the RWSEM_READER_OWNED bit can be set in owner and RWSEM_WRITER_LOCKED bit set in count in a transition period right after RWSEM_WRITER_LOCKED bit is set. reader writer reader acquire release rwsem_write_trylock set RWSEM_WRITER_LOCKED rwsem_down_read_slowpath set owner If prev lock holder is a reader, when it releases the lock, the owner isn't cleared(CONFIG_DEBUG_RWSEMS isn't enabled). A writer comes and can set the RWSEM_WRITER_LOCKED bit succsessfully, then a new reader run into slow path, before the writer set the owner, the new reader will see that both the RWSEM_READER_OWNED bit and RWSEM_WRITER_LOCKED bit are set. So the above sequence could be the case, right? So the RWSEM_WRITER_LOCKED check can still provide some value. We should probably update the comment to reflect that. > > Cheers, > Longman >
On Thu, Nov 9, 2023 at 11:17 AM Haifeng Xu <haifeng.xu@shopee.com> wrote: > > reader writer reader > > acquire > release > rwsem_write_trylock > set RWSEM_WRITER_LOCKED > rwsem_down_read_slowpath > set owner > > If prev lock holder is a reader, when it releases the lock, the owner isn't cleared(CONFIG_DEBUG_RWSEMS isn't enabled). > A writer comes and can set the RWSEM_WRITER_LOCKED bit succsessfully, then a new reader run into slow path, before > the writer set the owner, the new reader will see that both the RWSEM_READER_OWNED bit and RWSEM_WRITER_LOCKED bit are > set. > For the above example, it won't cause a problem. When the writer successfully sets RWSEM_WRITER_LOCKED, the reader, when reading rcnt through rwsem_down_read_slowpath(), will see that rcnt is 0 and will jump to the queue label. Thanks, Tang
On 2023/11/10 14:54, Tang Yizhou wrote: > On Thu, Nov 9, 2023 at 11:17 AM Haifeng Xu <haifeng.xu@shopee.com> wrote: >> >> reader writer reader >> >> acquire >> release >> rwsem_write_trylock >> set RWSEM_WRITER_LOCKED >> rwsem_down_read_slowpath >> set owner >> >> If prev lock holder is a reader, when it releases the lock, the owner isn't cleared(CONFIG_DEBUG_RWSEMS isn't enabled). >> A writer comes and can set the RWSEM_WRITER_LOCKED bit succsessfully, then a new reader run into slow path, before >> the writer set the owner, the new reader will see that both the RWSEM_READER_OWNED bit and RWSEM_WRITER_LOCKED bit are >> set. >> > > For the above example, it won't cause a problem. When the writer > successfully sets RWSEM_WRITER_LOCKED, the reader, when reading rcnt > through rwsem_down_read_slowpath(), will see that rcnt is 0 and will > jump to the queue label. > > Thanks, > Tang Yes, so if rcnt > 1, the RWSEM_WRITER_LOCKED bit couldn't be set?
On 2023/11/10 14:54, Tang Yizhou wrote: > On Thu, Nov 9, 2023 at 11:17 AM Haifeng Xu <haifeng.xu@shopee.com> wrote: >> >> reader writer reader >> >> acquire >> release >> rwsem_write_trylock >> set RWSEM_WRITER_LOCKED >> rwsem_down_read_slowpath >> set owner >> >> If prev lock holder is a reader, when it releases the lock, the owner isn't cleared(CONFIG_DEBUG_RWSEMS isn't enabled). >> A writer comes and can set the RWSEM_WRITER_LOCKED bit succsessfully, then a new reader run into slow path, before >> the writer set the owner, the new reader will see that both the RWSEM_READER_OWNED bit and RWSEM_WRITER_LOCKED bit are >> set. >> > > For the above example, it won't cause a problem. When the writer > successfully sets RWSEM_WRITER_LOCKED, the reader, when reading rcnt > through rwsem_down_read_slowpath(), will see that rcnt is 0 and will > jump to the queue label. > > Thanks, > Tang In this case, rcnt is not 0, it's 1, because rwsem_read_trylock() has add RWSEM_READER_BIAS, so if more than one new reader comes, it could be the case. reader writer reader reader acquire release rwsem_write_trylock set RWSEM_WRITER_LOCKED rwsem_down_read_slowpath rwsem_down_read_slowpath ... check RWSEM_WRITER_LOCKED bit(rcnt=2) count = atomic_long_add_return(adjustment, &sem->count); set owner
On 11/10/23 05:29, Haifeng Xu wrote: > > On 2023/11/10 14:54, Tang Yizhou wrote: >> On Thu, Nov 9, 2023 at 11:17 AM Haifeng Xu <haifeng.xu@shopee.com> wrote: >>> reader writer reader >>> >>> acquire >>> release >>> rwsem_write_trylock >>> set RWSEM_WRITER_LOCKED >>> rwsem_down_read_slowpath >>> set owner >>> >>> If prev lock holder is a reader, when it releases the lock, the owner isn't cleared(CONFIG_DEBUG_RWSEMS isn't enabled). >>> A writer comes and can set the RWSEM_WRITER_LOCKED bit succsessfully, then a new reader run into slow path, before >>> the writer set the owner, the new reader will see that both the RWSEM_READER_OWNED bit and RWSEM_WRITER_LOCKED bit are >>> set. >>> >> For the above example, it won't cause a problem. When the writer >> successfully sets RWSEM_WRITER_LOCKED, the reader, when reading rcnt >> through rwsem_down_read_slowpath(), will see that rcnt is 0 and will >> jump to the queue label. >> >> Thanks, >> Tang > Yes, so if rcnt > 1, the RWSEM_WRITER_LOCKED bit couldn't be set? No. The way readers acquire the lock is via atomic_long_add_return_acquire() without looking at current state of the rwsem (write-locked or not). So rcnt can be greater than 0 and the rwsem is still writer-owned. Because of that atomic_long_add_return_acquire() primitive, rcnt includes its reader count. The lock may be read-locked if only if there is at least one other reader present. So rcnt must be bigger than 1. Cheers, Longman
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c index 2340b6d90ec6..7a4d8a9ebd9c 100644 --- a/kernel/locking/rwsem.c +++ b/kernel/locking/rwsem.c @@ -1005,8 +1005,7 @@ rwsem_down_read_slowpath(struct rw_semaphore *sem, long count, unsigned int stat * waiter, don't attempt optimistic lock stealing if the lock is * currently owned by readers. */ - if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && - (rcnt > 1) && !(count & RWSEM_WRITER_LOCKED)) + if ((atomic_long_read(&sem->owner) & RWSEM_READER_OWNED) && (rcnt > 1)) goto queue; /*