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 195A6E7717F for ; Mon, 16 Dec 2024 10:11:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 405966B007B; Mon, 16 Dec 2024 05:11:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3B5656B0082; Mon, 16 Dec 2024 05:11:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 27C756B0085; Mon, 16 Dec 2024 05:11:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 0A92E6B007B for ; Mon, 16 Dec 2024 05:11:42 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 8D8CA140B36 for ; Mon, 16 Dec 2024 10:11:41 +0000 (UTC) X-FDA: 82900404762.26.4CC0B2D Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by imf12.hostedemail.com (Postfix) with ESMTP id DA0D84000E for ; Mon, 16 Dec 2024 10:11:26 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=elCrCJ0h; spf=pass (imf12.hostedemail.com: domain of akiyks@gmail.com designates 209.85.210.169 as permitted sender) smtp.mailfrom=akiyks@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734343885; 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=l+ixtMUrgX7I9rD5qvImPgp6IqifPuxLwfcX3hM//qc=; b=ekJjLxPM4lKp/zgy0ZsNhNaeFqzzTEywRf2JFram1dt9pXIYowx6gQijg+y5xZ+y8R7bGS SF9i/QwXiFbYjCALIOOqau+xgRrWaMSH68RQgfJ3SU3MUcMbhLc3M8PClKDjtVA5msmPJG hBYS+PFceNN1arXw+JiX0orHEfofrZM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734343885; a=rsa-sha256; cv=none; b=NNpL/F1/NTveTW6k6xT9WpXn0jDbnr3zjJ4LtLTPPX5YWxp+R5t5HFgIiMHaG+HFqMj/5C qJPB70awqziFur1sUMp5vZVDBo1Y7sxdPpTMkuGGvNwLuwzo/hQ4iizxMI9KQYMlH/Hhhh bsAd00/m8293CrPz9UTHieW2OvCXUEs= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=elCrCJ0h; spf=pass (imf12.hostedemail.com: domain of akiyks@gmail.com designates 209.85.210.169 as permitted sender) smtp.mailfrom=akiyks@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-728d1a2f180so2820148b3a.1 for ; Mon, 16 Dec 2024 02:11:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734343898; x=1734948698; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=l+ixtMUrgX7I9rD5qvImPgp6IqifPuxLwfcX3hM//qc=; b=elCrCJ0hrwDqOHhedaehZpaadVG5KRRZ7s+ichcKmS+8Ud22cq889LXJp4MVtBfTvy 4lwVb4kC/fbV0rZXe2s1A1NIv6QybTq4U2eRVT08rRHFYgpZYDz6nsynv8vy1//VceDN ZtiChRy7CwHT8+zvIj47eac3OVexjwaYGHbfTRQwyLqda7GnWumxwaRLeTbYYkfdMTkF fWdSaVgAWodAfgseNFTDCa6Jgu6RFXbWWXI9dMaM9KHXNYwAB3o/sTkvlUO4OZRayV45 bz5Bmb7bYNWBtPhtw3tU/28KJsGoaW7Nax5eUB+x6hDgkDyqvbra+6Tn4gxUl9jIT8K3 fuig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734343898; x=1734948698; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=l+ixtMUrgX7I9rD5qvImPgp6IqifPuxLwfcX3hM//qc=; b=br6+FuTF8Sz/5uB5KswPz2n9LyXxOAArCMBi5yxpUNAYZtgRhPCrzVYhxVk7QD/8v8 I2eLNIendLXODttNrSjh6aUcOC0imCCKG3s70qOA0cuJQEC1Jwgcs7lEjifu9Lj57pri LNSRhoRlqHZCMJuuuNawaEsalRDCEFAlj+ImqS++C/BXYr7H3DEyFbvMCZhEDaF+yUvV IP9OOAXCNgzQqokp/KxL0RfNNvlAiacpkJItCh+iowk9e0B30rFxhG+vnmUpXA94f9I6 PChB/IKwQJtgD64WBNivWpj7dN8w1CW1OtPQ2PLtmO+KsO+5l3OKUw3ar7yt7j+tbZU0 nLJg== X-Forwarded-Encrypted: i=1; AJvYcCULjjI3TLAFEmyBD7gcLhTtQzhS77bge2n6nl8TrCDqnpSCzFZmO8PK1UCwHuXBviqu/JY4y82jQg==@kvack.org X-Gm-Message-State: AOJu0YwFHu7F83n4NbjvFOH6IzgokCP+BxbY7xK7eIrzvVToSKVPIryY E7nOtGL8HZdKAyVCIHWft085RiX7xK3lv+eta60d58usjuDKYMVB X-Gm-Gg: ASbGncukRZAl8a98et2aXcI6UaaGCwVsMOSg+mGSgaTGR02AtRv+E4TIueLZ1z6zUFL tQ8lWfjpycWuPRie4DapD5xWOAE6n8LKrI0S8YGFfSSFl7v20ZHbW2tBIubTR8/Xtm7dzlEuJJj SUVR4oDEOjlQdMLfoIrWnY/Jj+lHKEpUuZfN07T/+b9j22+fqHCRSWzeqqt3F45ig4/M95yhUdl MGnKS4tFeriCcrTpUG5097o2H8wiRu9j3gbE507wQgE7p7hFojq+vFjHrtYo3CDeDuIZf7Vqxje WVZki0Dj8ppHw6ewmjuHVh8= X-Google-Smtp-Source: AGHT+IGkvlAjtALOBMooHfO7cK9L9lvwDBwHCDIhwn8y88sRHLlwLffLXlgmsZ5Mm6dr710N1QQFRg== X-Received: by 2002:a05:6a20:a11c:b0:1e1:e1c0:1c05 with SMTP id adf61e73a8af0-1e1e1c0202amr16720966637.9.1734343898208; Mon, 16 Dec 2024 02:11:38 -0800 (PST) Received: from [10.0.2.15] (KD106167137155.ppp-bb.dion.ne.jp. [106.167.137.155]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-801d5c3a513sm3824066a12.72.2024.12.16.02.11.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Dec 2024 02:11:37 -0800 (PST) Message-ID: <554ff96b-5be5-46b0-ac8b-f178394856f3@gmail.com> Date: Mon, 16 Dec 2024 19:11:33 +0900 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 07/10] netfs: Fix missing barriers by using clear_and_wake_up_bit() To: David Howells , "Paul E. McKenney" Cc: Christian Brauner , Max Kellermann , Ilya Dryomov , Xiubo Li , Trond Myklebust , Jeff Layton , Matthew Wilcox , netfs@lists.linux.dev, linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org, v9fs@lists.linux.dev, linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Zilin Guan , Akira Yokosawa References: <27fff669-bec4-4255-ba2f-4b154b474d97@gmail.com> <20241213135013.2964079-1-dhowells@redhat.com> <20241213135013.2964079-8-dhowells@redhat.com> <3332016.1734183881@warthog.procyon.org.uk> Content-Language: en-US From: Akira Yokosawa In-Reply-To: <3332016.1734183881@warthog.procyon.org.uk> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: DA0D84000E X-Stat-Signature: f4zggcc7zxbgieffma6ecfr3b3bstwat X-Rspam-User: X-HE-Tag: 1734343886-206972 X-HE-Meta: U2FsdGVkX18yuTi3VJXsyEmIiLnRVEwZuufyUbbI5ITdffmjH6/P/vmO71Fs28UusrUHz07gM7MH5OhOyRhe3mTmz7K66cAtM5g56ZVRsRfdr0JBJzkenWyzrR0iFUmPIweQJ+MpXTJztwWM/YfSsHVXd9z/7iGeAZ4FlSgvxbDpNHXf4bftL0rxAyWxRnxmye1ndl/WjOufw2sNSFZPHFwKgZbQE3XWgKVyuOJnbtd1aL9daSjUxKcyobhkiDtvZCG7xjxhB+TKTQQW4eV4gGIDtiW3oRHCdUik2AONPBDddnku9ssZVkkq9Ojkc1wUH60KXHKvNCBEPBjmpmlRPOVKmVFpPnSYdWJkumzYh0LPi+9dF1BxRn2SQiLOBN31N06QBF/x8tqNYx4jhbpT26W6PvBFr/qdWWCc33zui3yvpvclLoLi6cq/Dot3ahnYpF4ZvZhlj4ZHKDVba1p2hmtp56YoVCi2q8QX0A/sq46MqtwmOZMthR1t1D+ittP2g4TCvYDlCY6y6drCMXq8hLMFxD8/082yCecLC3JwLwjzu8KMk/pB12J7d78xxsunFqMneDN4xGEpkUOWzqFfOpxmMZU/FeZwmeen64bn7osUFPY+ST2zCNkJnYMfvaROWZJu58+4Yp+U+tqjuiB2ifDdWjUEII/jSICl0+gSc0smArNfrv3BldMWchz2w/j81E2aSnmSHlXVYXZuhIdLe1LhN8UugdslSyPQP+zMf8B/qFYO5yJZ3fNbOLCd69MO4yYmoiVi2TyIlXp/dr94mf/46xcSmOeiYLV2HugLrwDm0Bb4TjTZBzVnfX2CLkMm+BY9cghpvwjU/Q3pp2E3mINYy6szdOFRt6PNOgNUQchtxDkxFo4pteWZx0iHNZ/J25/OmeDCzX5kgOYFBxrlHvdotNQNt9OWJHqjjDZcaoYfCzdkcT9/Gs8ma9PwVnctfvNJ16htIBxldqTjh+R 665LOHtf 65328LnH8dBscgCn/tjNsBbGNXPUJZuZ1K+khJrR0cTZBsl3DGFifoohzP1sJCgTJcj2831j3HjmcV0QKVT3hbVx/D1Fl7L1xRQQwVKMSKWHfHzcFKPv2lKYMdbYWGOX7m/PF2oTAGOZmkGC2BCV9hJ8I7g8nr/U5Xp/mKKGeO63IkVdweRtwrUJhOuevyyI3xsb4mE1AxvVronNJg2rMidfEe8BIJJZRbmhDAoMFhJevILrlMueUntLkFfhGN4qVk/mhgz0r+yV5Da5xUmovEsupMPjy0OcxqmUM1A/2lBrF4bY2h9M86z51gw3E5GNM3PyjAZDnBWUAE6dgRBTCYsrn2yHmF55zWmrZFure6ReQwu9eBTUotKY0v6dRMg33vxmFEEWUDNxrlSS9aJN9ap5n4zY/q6sbDv89WdjktUx6lLT+YfzngesvZWKvnqgFhQ04GEwwyhBMzDEc7VKiqGrKoFvKhyUcdqi+Y7kgpaJmBZxoaQBwyF8hADupQgDBzkfzHGb5gB08wDmvqxl8vd52dH1RJDdJGAGJtcXsBLlC8HY= X-Bogosity: Ham, tests=bogofilter, spamicity=0.066411, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: David Howells wrote: > [Adding Paul McKenney as he's the expert.] > > Akira Yokosawa wrote: > >> David Howells wrote: >>> Use clear_and_wake_up_bit() rather than something like: >>> >>> clear_bit_unlock(NETFS_RREQ_IN_PROGRESS, &rreq->flags); >>> wake_up_bit(&rreq->flags, NETFS_RREQ_IN_PROGRESS); >>> >>> as there needs to be a barrier inserted between which is present in >>> clear_and_wake_up_bit(). >> >> If I am reading the kernel-doc comment of clear_bit_unlock() [1, 2]: >> >> This operation is atomic and provides release barrier semantics. >> >> correctly, there already seems to be a barrier which should be >> good enough. >> >> [1]: https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#c.clear_bit_unlock >> [2]: include/asm-generic/bitops/instrumented-lock.h >> >>> >>> Fixes: 288ace2f57c9 ("netfs: New writeback implementation") >>> Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading") >> >> So I'm not sure this fixes anything. >> >> What am I missing? > > We may need two barriers. You have three things to synchronise: > > (1) The stuff you did before unlocking. > > (2) The lock bit. > > (3) The task state. > > clear_bit_unlock() interposes a release barrier between (1) and (2). > > Neither clear_bit_unlock() nor wake_up_bit(), however, necessarily interpose a > barrier between (2) and (3). Got it! I was confused because I compared kernel-doc comments of clear_bit_unlock() and clear_and_wake_up_bit() only, without looking at latter's code. clear_and_wake_up_bit() has this description in its kernel-doc: * The designated bit is cleared and any tasks waiting in wait_on_bit() * or similar will be woken. This call has RELEASE semantics so that * any changes to memory made before this call are guaranteed to be visible * after the corresponding wait_on_bit() completes. , without any mention of additional full barrier at your (3) above. It might be worth mentioning it there. Thoughts? FWIW, Reviewed-by: Akira Yokosawa > I'm not sure it entirely matters, but it seems > that since we have a function that combines the two, we should probably use > it - though, granted, it might not actually be a fix. Looks like it should matter where smp_mb__after_atomic() is stronger than a plain barrier(). Akira