From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57AD1EC875E for ; Fri, 8 Sep 2023 00:44:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 821138E0001; Thu, 7 Sep 2023 20:44:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D1086B0075; Thu, 7 Sep 2023 20:44:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 64A918E0001; Thu, 7 Sep 2023 20:44:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 51E056B0074 for ; Thu, 7 Sep 2023 20:44:19 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 28B3580351 for ; Fri, 8 Sep 2023 00:44:19 +0000 (UTC) X-FDA: 81211583838.11.8FFCCF2 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) by imf23.hostedemail.com (Postfix) with ESMTP id 2F661140008 for ; Fri, 8 Sep 2023 00:44:16 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=fromorbit-com.20230601.gappssmtp.com header.s=20230601 header.b="eui2pn/C"; spf=pass (imf23.hostedemail.com: domain of david@fromorbit.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=david@fromorbit.com; dmarc=pass (policy=quarantine) header.from=fromorbit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694133857; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=7BcrfYnyRxSF9PP3U2fLWJK1bEQTLfrZGPuCDzVXSzs=; b=QvEMrc+tc+jQ11z9+lBB8igdMfH/xKx5A/IpsfOhPSohmEX+r3vL1XTbNaQmghIzArjAaM m/zJ8nXPrQp5oxN/9zE2h7jcu0FVdJDTesfyLn6a4FvmORzc64u1rEW7vKuB0Ok/iffD5z m3xT8VyEt5YzbwaURVJP/bO3fOQKrV0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694133857; a=rsa-sha256; cv=none; b=j90284/zuOnvjOVgQS6B87uWx/AzD1aOUuVenLEktZcjsxZRYe53tKG+MeghVdTy+beGGB IJw2VFWgtNpKv77nzFk6yWdDoA0zRUz0o7XM+4aU0/VoxK++BFpxJ0pG/BfbLilf4OZA4a NkHkvdGT6XvXtIWol/yn7eWdxJEaRLw= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=fromorbit-com.20230601.gappssmtp.com header.s=20230601 header.b="eui2pn/C"; spf=pass (imf23.hostedemail.com: domain of david@fromorbit.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=david@fromorbit.com; dmarc=pass (policy=quarantine) header.from=fromorbit.com Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-1bd9b4f8e0eso11457675ad.1 for ; Thu, 07 Sep 2023 17:44:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1694133856; x=1694738656; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=7BcrfYnyRxSF9PP3U2fLWJK1bEQTLfrZGPuCDzVXSzs=; b=eui2pn/Cc8tDhzXF/T8gvpq08BWtlMb9mEqayQ0NaKECECoJsZy6kVLdN8VWLbh+Sv 9VzfF3hSmhoBxQB7JtpTvSsftwNfchq0tefLp77HtcHa1RCVgz2hRNdD4Ds7JDEYMUh7 RiWxbzLuJgLV+UgwUQmx87n58WT4cbJ2i993Xuc3JxP7+xOu8oJBJ4uknAt9NFnUMKVY n4b0mTT3rb8CDSWK7ZzjYzxjeO+ZAZvx/n5jXtsh0fPfROQXkxkvys9RxqnJjY2I1gf5 b4x13JvFKQtbb2f+chmSFHWQ9Jg4KgZNtt9iKp0PSb3JOKedSahG9wi0KXaQkdSgx85t MSjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694133856; x=1694738656; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=7BcrfYnyRxSF9PP3U2fLWJK1bEQTLfrZGPuCDzVXSzs=; b=DxmPRMb/nLJhGExNonOb7b/3cAHAIBLPAmCjFgntbGgHBWnvxm/av4F14Q6z3gzyFq VU+GqA3NqoY4xPifQfrMAhvqNx2JT0UzXNjoXQrUPbszgbl11ZrdIujrAp6oE7Aj9kJI A/v3lZJz46GxMO7o96tIJqwZitRZ7WcUzJjIfbDpZdRPWsOJu1Ygbf91u+RrVxMHMMHe k/5YEoRyhLSSdsYh9K21r8t1GrfNBppUz7at4kAQ8+fWNSBkw3tGVzTAArK7/oNV1vSX MPQz3ytYS7qp2SduL3cKZFCMayW6ub9LK/h2HANewiu0+exhFVix6PrCFqvn+PDv/v3t OJlQ== X-Gm-Message-State: AOJu0YwC7l9p359YkkKqOzZ2EaUCNTXWjywrff5uwjbi4LPBTq7kR/xf dcEwvA2fn0rSSLQ8GGWtAUtoag== X-Google-Smtp-Source: AGHT+IHF33zguZ89K0iOoy7lHBVYfHc84YSxC26LhbI49eHjFrQwo1x6osxcaKsS9pDqLYWTJEya+A== X-Received: by 2002:a17:902:db0e:b0:1c3:39f8:3e67 with SMTP id m14-20020a170902db0e00b001c339f83e67mr1370255plx.20.1694133855901; Thu, 07 Sep 2023 17:44:15 -0700 (PDT) Received: from dread.disaster.area (pa49-195-66-88.pa.nsw.optusnet.com.au. [49.195.66.88]) by smtp.gmail.com with ESMTPSA id x1-20020a1709029a4100b001befac3b3cbsm320319plv.290.2023.09.07.17.44.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Sep 2023 17:44:15 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.96) (envelope-from ) id 1qePbZ-00CFhM-0H; Fri, 08 Sep 2023 10:44:13 +1000 Date: Fri, 8 Sep 2023 10:44:13 +1000 From: Dave Chinner To: Waiman Long Cc: Matthew Wilcox , Peter Zijlstra , Ingo Molnar , Will Deacon , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Chandan Babu R , "Darrick J . Wong" , linux-xfs@vger.kernel.org Subject: Re: [PATCH 1/5] locking: Add rwsem_is_write_locked() Message-ID: References: <20230907174705.2976191-1-willy@infradead.org> <20230907174705.2976191-2-willy@infradead.org> <2cd975ec-f868-f180-350f-b1b704118777@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 2F661140008 X-Rspam-User: X-Stat-Signature: 4gef3qwehpis8gjuj1867utowcscxtxm X-Rspamd-Server: rspam03 X-HE-Tag: 1694133856-139001 X-HE-Meta: U2FsdGVkX18KWxZkOjzsWpFAhF8rt0BXOL3s7A6rFd9w2ugWV0tkpTAERilso5Ok8YStZWea2HmvW9/YNIoHw6CnV5pvyhsF4OO2fRdEHJD3zNY/5kAVMQ/pbkyLfnSHwfsXPFpmYPUw68ZgKglKMZM8fe7fZnhrJBFVKb3iLutwmSeIzmsdNH69E57OlJmy+Og0ZH4b0UQX8ffx63xrBoZ7+CeAEZIcJK9wP6EplF5ls7DZWea0yfPIANuWQ5jW8k++E+foawCBdbJXhBPPaSjPqMj2/+xMoy4TdM2J7BVXprxqJfO7T8urCNMnXcxAKDgCKnqBkoz+ZLYvFCVP0YoPhAaYLD73IofVHEkqg7+8VKhn/tcV121R26rtzjEDlOVsRQQMMKrQqmSU8AjsbWTzEQNceaFa0+Xi5iZieqBC2JRwInkipZTkVW5g+1lt8MXm1AT0W0/TM8eWNV2893nYrA/7pD3GMCT0AvP2PRPRzPAGAjLkHbhyXx/QqcLxiWNIFU77a/Tr+P9zWmRRslM+EN4cwMAiNLxzjMaSI+2zSSpbQgc9l9g51rU33eQnN8qZobWAORc9y9Bu3Bja+XFJJKDX0zDChLiv3AwuSN/GHLfAD4lIs6t7eEuGcJlyIfIAyWqWCz3KA1xiaDBnVOL/LPQZTOZ5E5s3P265fmutvAV6eYrIsrWvsqKaaatf4Aj6oqlXtNgQruGLjSg0Fjr3837hm/8dWbEgapcbMumAt+dSFYW11tPFGzPty82B38nHc8M/YxEMOAg8CEIajeLCfpc39m5+61guzeT51Vx1on5jtjGHbUp6ZPm7RpMfg5kIIv96M6OA4CBMxHU7N4vCBi3kyhE0Dzg/U4YeCaMnkK9CSxOaGN2Z5RgvdtTb1hFkwG6MKQtdPIkBlveCM98PPx1zNVXhfBFJEV0KW/smkZOz9ZRiK1xC6dYaDad2tS43a87SYaR4/taaV4g csng3GEC rk8YCNQZgYIn9x8wPjh1ltFJsD0yjnXM6TSnIkqb7TjAiu8iLg+SeT3vRLiOaIDtmmSnWb2F100fpyRBJjvyNmyVO9hgBguaCRfYa65NOj0O0wd7iweCcq903KAXQ2SKDs2GLks8Ljc+pZnv3rh2wVNpNdUgoowEsTjwAaX1YpMo6UBSdWhfwhsovARGrdt4lT//KTUm5fdpKIaziPN2L6WsoBhm+kLRdA+Fn1tpNSgJWa045gJqPCRyTn/QE33e/gds9lcAxb7rNzKajXhBYtIRSDTq3xugX7IsjhMLsu6yEf97eTBjeuxygTR/CUnbKOvQXK5vqxgzS7mgQDEZzW80u3EB9wBA5nje/svQdEecl+YvU86stRhyHDSAbexIvzknT8cvo6kDUV5yPt1NpQi+VhHsYP6uxEWTO2jExv+/Ort3bbBeCwzn7CfRQvc98/kMbmHpIdfalTPWl0+hUmMCi6IpnNqbUVeZsVnUzf4IIMhUTDfD6XIDMXA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Sep 07, 2023 at 07:47:31PM -0400, Waiman Long wrote: > > On 9/7/23 17:06, Waiman Long wrote: > > > > On 9/7/23 15:33, Matthew Wilcox wrote: > > > On Thu, Sep 07, 2023 at 02:05:54PM -0400, Waiman Long wrote: > > > > On 9/7/23 13:47, Matthew Wilcox (Oracle) wrote: > > > > > +static inline int rwsem_is_write_locked(struct rw_semaphore *sem) > > > > > +{ > > > > > +    return atomic_long_read(&sem->count) & 1 /* > > > > > RWSEM_WRITER_LOCKED */; > > > > > +} > > > > I would prefer you move the various RWSEM_* count bit macros from > > > > kernel/locking/rwsem.c to under the !PREEMPT_RT block and directly use > > > > RWSEM_WRITER_LOCKED instead of hardcoding a value of 1. > > > Just to be clear, you want the ~50 lines from: > > > > > > /* > > >   * On 64-bit architectures, the bit definitions of the count are: > > > ... > > > #define RWSEM_READ_FAILED_MASK (RWSEM_WRITER_MASK|RWSEM_FLAG_WAITERS|\ > > > RWSEM_FLAG_HANDOFF|RWSEM_FLAG_READFAIL) > > > > > > moved from rwsem.c to rwsem.h? > > > > > > Or just these four lines: > > > > > > #define RWSEM_WRITER_LOCKED     (1UL << 0) > > > #define RWSEM_FLAG_WAITERS      (1UL << 1) > > > #define RWSEM_FLAG_HANDOFF      (1UL << 2) > > > #define RWSEM_FLAG_READFAIL     (1UL << (BITS_PER_LONG - 1)) > > > > I think just the first 3 lines will be enough. Maybe a bit of comment > > about these bit flags in the count atomic_long value. > > Actually, the old rwsem implementation won't allow you to reliably determine > if a rwsem is write locked because the xadd instruction is used for write > locking and the code had to back out the WRITER_BIAS if the attempt failed. > Maybe that is why XFS has its own code to check if a rwsem is write locked > which is needed with the old rwsem implementation. mrlocks pre-date rwsems entirely on Linux. mrlocks were introduced to XFS as part of the port from Irix back in 2000. This originally had a 'ismrlocked()' function for checking lock state. In 2003, this was expanded to allow explicit lock type checks via 'mrislocked_access() and 'mrislocked_update()' wrappers that checked internal counters to determine how it was locked. In 2004, the mrlock was converted to use the generic kernel rwsem implementation, and because that couldn't be used to track writers, the mrlock included a mr_writer boolean field to indicate it was write locked for the purpose of implementing the existing debug checks. Hence the mrlock debug code has always had reliable differentiation of read vs write state, whereas we couldn't do that natively with rwsems for a real long time. The mrlocks have essentially remained unchanged since 2004 - this long predates lockdep, and it lives on because gives us something lockdep doesn't: zero overhead locking validation. > The new implementation makes this check reliable. Still it is not easy to > check if a rwsem is read locked as the check will be rather complicated and > probably racy. You can't look at these locking checks in isolation. These checks are done in code paths where we expect the caller to have already locked the rwsem in the manner required. Hence there should be no races with rwsem state changes at all. If we see a locking assert to fire, it means we either screwed up the XFS locking completely (no races necessary), or there's a bug in the rwsem implementation. The latter case has occurred several times; the rwsem locking checks in XFS have uncovered more than one rwsem implementation bug in the past... IOWs, the explicit lock state checks and asserts provide a lock implemetnation validation mechanism that lockdep doesn't.... Cheers, Dave. -- Dave Chinner david@fromorbit.com