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 D3175CD37B0 for ; Mon, 18 Sep 2023 15:54:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 57AFC6B03C5; Mon, 18 Sep 2023 11:54:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 52A886B03C7; Mon, 18 Sep 2023 11:54:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4196D6B03C9; Mon, 18 Sep 2023 11:54:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 3360C6B03C5 for ; Mon, 18 Sep 2023 11:54:09 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 10C07160B2A for ; Mon, 18 Sep 2023 15:54:09 +0000 (UTC) X-FDA: 81250164618.22.AE8C44D Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf03.hostedemail.com (Postfix) with ESMTP id DC4DF20004 for ; Mon, 18 Sep 2023 15:54:06 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=hPyVo6xQ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="K/YCBDd4"; dmarc=none; spf=pass (imf03.hostedemail.com: domain of jack@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695052447; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=BO0o7mc51KvupjydrTvNuZTowoiGCihD2MFz1LzYzPM=; b=7t8Vhyl9nuKmVu6isk8wMyf0ew/LJn1r2ulQlRLKhJXifwY6VcusnXCnyMD5eQ6uV0rLiq 1Clr+qv6If/LbFX8vYMYIcoXgn46qE8UOmFBqD7yqRbBXZj6di9yUzn3hQ8olUMfVq7TNL gDvEl8byTSiGO6AB/MElp7wjgzHKZTA= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=hPyVo6xQ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="K/YCBDd4"; dmarc=none; spf=pass (imf03.hostedemail.com: domain of jack@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695052447; a=rsa-sha256; cv=none; b=JbFJcnYIouoKCfsQfgh6+Hkyg215es0YwGbsNpXvTZmT8lMEzc2eNn0snk0RYG5A8XQ0nO oDyv/YTJ4goyjYeBC5oow+w/3EFpc+22C8IbJQLfnbJZzJ1TZ9UZx+O6qr1rFIG3/yb180 7X9lyX7BAhWUr+Y7UdFEp+2DMVip4UM= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id BC00F21E21; Mon, 18 Sep 2023 15:54:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1695052444; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BO0o7mc51KvupjydrTvNuZTowoiGCihD2MFz1LzYzPM=; b=hPyVo6xQ/5JYBEGBWzgWV1gR7wcLXkl8skxnWsiADUcukIRsswrZlRp4H7Q2rp2y0rLPYf hy20KmHGuedlI5a1O1IQkctlT7ysTSghj1lN6N9TCyvl3PQFNq2gUlUhgXbfpMRUYal5O4 vIbr/uLGwth4ZwbOXcZgs+b2058Z0rI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1695052444; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BO0o7mc51KvupjydrTvNuZTowoiGCihD2MFz1LzYzPM=; b=K/YCBDd4H1NBX1EBhbtAbx3l0jAwQCKJS6k+0RRGk+UHsy/dvQVFn9Xz60jhP24bmToRnW /NBs6u4t2V4yr2Bg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id A966A13480; Mon, 18 Sep 2023 15:54:04 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id llpRKZxyCGWsaQAAMHmgww (envelope-from ); Mon, 18 Sep 2023 15:54:04 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id EF583A0759; Mon, 18 Sep 2023 17:54:03 +0200 (CEST) Date: Mon, 18 Sep 2023 17:54:03 +0200 From: Jan Kara To: Yury Norov Cc: Mirsad Todorovac , Jan Kara , Philipp Stanner , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Matthew Wilcox , Chris Mason , Andrew Morton , Josef Bacik , David Sterba , linux-btrfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v1 1/1] xarray: fix the data-race in xas_find_chunk() by using READ_ONCE() Message-ID: <20230918155403.ylhfdbscgw6yek6p@quack3> References: <20230918044739.29782-1-mirsad.todorovac@alu.unizg.hr> <20230918094116.2mgquyxhnxcawxfu@quack3> <22ca3ad4-42ef-43bc-51d0-78aaf274977b@alu.unizg.hr> <20230918113840.h3mmnuyer44e5bc5@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: DC4DF20004 X-Stat-Signature: zpzy4qm6y51f7ozgejej9bpgj95g3zfq X-HE-Tag: 1695052446-435490 X-HE-Meta: U2FsdGVkX19Q1CXFVgxlJjXQQghpDSSGTPbkuDA7sbWGYmKCj/8Mt7naWzqljcrYABPKeJ7nwk3f1yQH3+WhJD5nHUP++Oom0KBcGvA0sd7MxIhtq52yO4l/mhnS+EEtOCISNTzBZtkMcIRirsfCkoVeEBZ63W52jrTwr6YKSBV7vhs6uH+938dfM7DExARTmoIJW5bHFKZzBDJGCwbWO4DNN0VXtR8IJXnDWJ5zqR3KlPQBlp4guAnx+3Q+73jZlXMyGo9B9a0iw7w2OvisJS3RjHDRyMIY1X9O2nCsrZt2u1aCyRF65sj6D88JMyJQwVL7a4t9jRPPNz9sI0lSvyecyw3E/UYABamkBTLfhAWnAri1jFC8xtDfkFsCp311XN0o4nnnRJ3Kw0WJvIkL3w6jf0WOkIQlenuZ2dYRvr7mzhY3E0fJ+kd8krpBaTUdvOfPa5krI2cuvyM+6kjXlmQZmajpe0sBSA/YL7NM1Cgeqra9fyaf3q0qUNSr6NraZbdHmRVdkMkjq/1RQZf7CgidvPJQXpA8Re5eBguy3Vt/HpwrvmGi9Ovcae6JeTsFZ8ubelQFwnoK0ZUcsNNzavE96B0Ja/8+56VtyLthuab0UMKMEnyZoCcCrwi3yb/17PN6cR3xzucxBqPxWQubL9UPCQK8APmdLbVIInCLM9gIS5WftvIHm42t/e1pfdyQZ7rvC2aTuardUb1Gxvk9ZYFglENdCG+R2SfBfGkeXV9r0jZBmx5x5ghRZ6cdMEB7AhKrVKyc3YzW8HqF2VsDLpICaevUYmxmlvoANtjUYbL74mgYXhbozUTpcxPU2rm8IfISpa6o9deUftaFMmmLMJGjCgwDedPNwXzVSLcnQXogIURPPHzzr+oOq5B3PMSuFTjA81nJ2V2MLmRJ8+xs0yZEzc3sj/8l5dWTnW4qD6kGMFk3HNKBM7H5o6Vrv0QGQJsF/TJkTLcvvU6zxnf xXkQzDn0 1TCy4DgE1NwradGlhJSiWgdyXxLdYTXxh7XMlGGIUlDG546QXymeQyy2wMIW8q+2aGsHrWqup3eMdffE35Fb5JR2LBwjVZNMDUkr8BSqIqBlxv6lTxC6CY3uYxK2OdE9LggqZlxSn9N/1ERHA+CGAeIOXD80SRHwjFw1bRWPy8F1m0QePxIWfB0K9tRLOz7zQCOxrEwzXEzfsCXY7E1j3uZVQTAWEy5plQ78ngSfTXbJnRXRL2zGcs0/2B1M4GKcUH9aXPBXnQkJlFJsbc2Tham9L+F4Tj9h9/fAHFaWx6ZT30g5kNUSqADWrRMOWKzkVV3ekuNfDVzSuPQXQz8evuUH9H5k0fZ+egmPDSZiksEESj2SN4XSG0KakHKMUskczpmZBjto22cU3F7UA90dShGvaT1WeH1R9Lj9G 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 Mon 18-09-23 07:59:03, Yury Norov wrote: > On Mon, Sep 18, 2023 at 02:46:02PM +0200, Mirsad Todorovac wrote: > > -------------------------------------------------------- > > lib/find_bit.c | 33 +++++++++++++++++---------------- > > 1 file changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/lib/find_bit.c b/lib/find_bit.c > > index 32f99e9a670e..56244e4f744e 100644 > > --- a/lib/find_bit.c > > +++ b/lib/find_bit.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > /* > > * Common helper for find_bit() function family > > @@ -98,7 +99,7 @@ out: \ > > */ > > unsigned long _find_first_bit(const unsigned long *addr, unsigned long size) > > { > > - return FIND_FIRST_BIT(addr[idx], /* nop */, size); > > + return FIND_FIRST_BIT(READ_ONCE(addr[idx]), /* nop */, size); > > } > > EXPORT_SYMBOL(_find_first_bit); > > #endif > > ... > > That doesn't look correct. READ_ONCE() implies that there's another > thread modifying the bitmap concurrently. This is not the true for > vast majority of bitmap API users, and I expect that forcing > READ_ONCE() would affect performance for them. > > Bitmap functions, with a few rare exceptions like set_bit(), are not > thread-safe and require users to perform locking/synchronization where > needed. Well, for xarray the write side is synchronized with a spinlock but the read side is not (only RCU protected). > If you really need READ_ONCE, I think it's better to implement a new > flavor of the function(s) separately, like: > find_first_bit_read_once() So yes, xarray really needs READ_ONCE(). And I don't think READ_ONCE() imposes any real perfomance overhead in this particular case because for any sane compiler the generated assembly with & without READ_ONCE() will be exactly the same. For example I've checked disassembly of _find_next_bit() using READ_ONCE(). The main loop is: 0xffffffff815a2b6d <+77>: inc %r8 0xffffffff815a2b70 <+80>: add $0x8,%rdx 0xffffffff815a2b74 <+84>: mov %r8,%rcx 0xffffffff815a2b77 <+87>: shl $0x6,%rcx 0xffffffff815a2b7b <+91>: cmp %rcx,%rax 0xffffffff815a2b7e <+94>: jbe 0xffffffff815a2b9b <_find_next_bit+123> 0xffffffff815a2b80 <+96>: mov (%rdx),%rcx 0xffffffff815a2b83 <+99>: test %rcx,%rcx 0xffffffff815a2b86 <+102>: je 0xffffffff815a2b6d <_find_next_bit+77> 0xffffffff815a2b88 <+104>: shl $0x6,%r8 0xffffffff815a2b8c <+108>: tzcnt %rcx,%rcx So you can see the value we work with is copied from the address (rdx) into a register (rcx) and the test and __ffs() happens on a register value and thus READ_ONCE() has no practical effect. It just prevents the compiler from doing some stupid de-optimization. Honza -- Jan Kara SUSE Labs, CR