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 D9A43CD3437 for ; Tue, 19 Sep 2023 04:20:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 589A76B0498; Tue, 19 Sep 2023 00:20:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 539A36B049A; Tue, 19 Sep 2023 00:20:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4017A6B049B; Tue, 19 Sep 2023 00:20:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 2DA1F6B0498 for ; Tue, 19 Sep 2023 00:20:32 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 00BA11603F7 for ; Tue, 19 Sep 2023 04:20:31 +0000 (UTC) X-FDA: 81252045504.28.34C6AB5 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf16.hostedemail.com (Postfix) with ESMTP id 1223E18000E for ; Tue, 19 Sep 2023 04:20:29 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=plJiV8FD; dmarc=none; spf=none (imf16.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695097230; 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=rkouqG7ezneLR9fXmWK2z8oVOJc/7J+HE9nJ0wqC52Y=; b=PFJl+sTN7RyFkpOvUtuHtjWfQvOYtiLMRD9Sat0OJAa32RrGvMr5z6LKvLoTS7OjlqeHXx m/zw2BmK26WVjC33jO5zPHTRF4igacuwTiXqq0eU/vFX943T0NV+FaW3/t1AsCJzJaw3EP 07brBu20aRMhfSljVgbDZFcMc3mdTu4= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=plJiV8FD; dmarc=none; spf=none (imf16.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695097230; a=rsa-sha256; cv=none; b=e4afRDKf0XJ8y2E1HgUs6UrzbTLz2xajVBrWzEHnFLEVNqBbl/47KJAz54qwnr3MVJw6I+ 70mjM37EH6X/T+uBMZnNWqsSIcipFCbgygjoCTq1SnqpozGvqEyJ47oRrl+NKejVG+5kYd OUYiEv/cDOV2jeFEiXEzKX7Ht0evne8= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=rkouqG7ezneLR9fXmWK2z8oVOJc/7J+HE9nJ0wqC52Y=; b=plJiV8FDPUDBI4qF+NloBKEGkW sjP88Q+Je4DqE07zwn4QkJTvUIW2BTfWyUsIlTh72QHODMmxjmJhITZPCtOE4E9iLra/UiKRYpdX7 FVDc3aUxetTxkSiJF0WfxpVr7bg1Ki8cFYt6qtZ1OWGKp589GCmw4YBFSaYs7KTT+PkbyytSgdb8s 4QbdhLWcjJxfuLepxUEy+ci9po9r0SAaqWcYRzVVBFaNj6TKS5iPgLFKybZ3cz3THYkWOE58WRv0N J1Ytx/vzXVxEty3t7Eq2ZySvC4nZKI9hI9Yhtj8B0QISswaK7uD6bAGKQHInb1uGnRYWcYJiB2S1D KN0/6UTA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1qiSDh-00F5MH-Ue; Tue, 19 Sep 2023 04:20:18 +0000 Date: Tue, 19 Sep 2023 05:20:17 +0100 From: Matthew Wilcox To: Yury Norov Cc: Mirsad Todorovac , Jan Kara , Philipp Stanner , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, 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: References: <20230918044739.29782-1-mirsad.todorovac@alu.unizg.hr> <20230918094116.2mgquyxhnxcawxfu@quack3> <22ca3ad4-42ef-43bc-51d0-78aaf274977b@alu.unizg.hr> <20230918113840.h3mmnuyer44e5bc5@quack3> <20230918155403.ylhfdbscgw6yek6p@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1223E18000E X-Stat-Signature: ebmxc9mciafcua9aodsy8z3b5xw9koe8 X-Rspam-User: X-HE-Tag: 1695097229-290723 X-HE-Meta: U2FsdGVkX19Ehn9iJ5GKK+XCXwUidbIqXhF8NAO59t1b121O5mSWonZIZdUjhRMCZvVatEChJI42Xoex1ipJ7fT0QKMt/lHp4uijAQWuHi8S+rTitxpKB7aXdmjrix/IoGcDI/T0fXdIDbYqMpJbmLxksBjkK1c0Xl5ei758ePPHBUvmF6PP/jquzjzD7qfjlgUM8dhkCzMEKTdIUGeUOI20zFsad9YkHEtanmuVEVwBHPs5Efj3DWoLGlkUVrMd3YdwpoWcEH9Jm/IccoScqDU3wJsno+kCG4XpsmJydsvGi3+qpT/H0hw8JLFPU17MMi6+a8vml6vVVVRIZ3uH78WMFtlpkMwkrdyAAcUBjc1hfmXlDAw8oHLe9gymGQNATmo8t3QZJAqyl9+PvI9fe3uj1Aot5zrt2Spqmq/thlL9thChah8eUhS1FP0GviZwdnqXO3E0HKqOCT8TpPyDafloQR05syVYtk27rZN072C7NJdylGT6pfg5l8O5PP9g8L4WCq6m4PJhT/qIt6i07U6M/TstvX8xk2ErZ+flzr0M/qc9lcMC1wpe3mT4UW2S6x1LPO8zFWXpf6MroFZKaW4iIVyeKeuD4IQHPCfjadmqgNlmgDHcys6tIGu9lJFSgMKzQeCbzWdehQAMYOdOvjS7zwN7CVCmuL/9PYLeQYxxNJz0qVslYEm8juUJe+5LHyNFGJb9pnYJo6h2sRRNlsv9t3v0+OXnsQsSH7uODAR+mTKRJV8kqVdZArDZe11ttnUmCV50pXdRM8WyWRyp1LxCdGbVtEm+HeySVPkgMX4MJtq8xbdr8hHYKQyOv23r1XvU2EOcUs/DOvGCv7fnxCWGH67Lh8NQe1hhNDWcUNnIBFAptaCxgRJ1KgJS1Sy/CftCpd0e43H5BX7gS22Yp2ZTDFnuFTFi8EvxY/d0gZKrTe/dWJsYCQm/TXd2YFtqYon26gGqJiFYf4gk66E ET12PKFx pV7TYo3/+hOBMnMUVc5z8cHj+4EwJF/XdvOQ/+k49L6AaenvcW9+8rDRVen5bZVPuvV8N2+FvpHGZZleY9yach2lTiDmHzzhKWLE0KMguMnkV2XQBeG7vJSIvDPV8CGYMnEmkhzOQsl5nA4kfgQCJclmO9+Dy3BENKCzmpE/zRITwiW9ZyCJ8nNUHfQcoz7EkuFlFkF9FnXsnPOSxd/AWEJ9GYcx8702+Wn8aeumUeKNwvo+aXOCcTw7XRIlqD+mVVa88 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, Sep 18, 2023 at 11:56:36AM -0700, Yury Norov wrote: > Guys, I lost the track of the conversation. In the other email Mirsad > said: > Which was the basic reason in the first place for all this, because something changed > data from underneath our fingers .. > > It sounds clearly to me that this is a bug in xarray, *revealed* by > find_next_bit() function. But later in discussion you're trying to 'fix' > find_*_bit(), like if find_bit() corrupted the bitmap, but it's not. No, you're really confused. That happens. KCSAN is looking for concurrency bugs. That is, does another thread mutate the data "while" we're reading it. It does that by reading the data, delaying for a few instructions and reading it again. If it changed, clearly there's a race. That does not mean there's a bug! Some races are innocuous. Many races are innocuous! The problem is that compilers sometimes get overly clever and don't do the obvious thing you ask them to do. READ_ONCE() serves two functions here; one is that it tells the compiler not to try anything fancy, and the other is that it tells KCSAN to not bother instrumenting this load; no load-delay-reload. > In previous email Jan said: > for any sane compiler the generated assembly with & without READ_ONCE() > will be exactly the same. > > If the code generated with and without READ_ONCE() is the same, the > behavior would be the same, right? If you see the difference, the code > should differ. Hopefully now you understand why this argument is wrong ... > You say that READ_ONCE() in find_bit() 'fixes' 200 KCSAN BUG warnings. To > me it sounds like hiding the problems instead of fixing. If there's a race > between writing and reading bitmaps, it should be fixed properly by > adding an appropriate serialization mechanism. Shutting Kcsan up with > READ_ONCE() here and there is exactly the opposite path to the right direction. Counterpoint: generally bitmaps are modified with set_bit() which actually is atomic. We define so many bitmap things as being atomic already, it doesn't feel like making find_bit() "must be protected" as a useful use of time. But hey, maybe I'm wrong. Mirsad, can you send Yury the bug reports for find_bit and friends, and Yury can take the time to dig through them and see if there are any real races in that mess? > Every READ_ONCE must be paired with WRITE_ONCE, just like atomic > reads/writes or spin locks/unlocks. Having that in mind, adding > READ_ONCE() in find_bit() requires adding it to every bitmap function > out there. And this is, as I said before, would be an overhead for > most users. I don't believe you. Telling the compiler to stop trying to be clever rarely results in a performance loss.