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 3C68DE95A96 for ; Mon, 9 Oct 2023 10:15:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C10C28D004B; Mon, 9 Oct 2023 06:15:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B9BB78D0031; Mon, 9 Oct 2023 06:15:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A3A848D004B; Mon, 9 Oct 2023 06:15:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 8F1938D0031 for ; Mon, 9 Oct 2023 06:15:55 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5709BB4783 for ; Mon, 9 Oct 2023 10:15:55 +0000 (UTC) X-FDA: 81325517070.11.92DFA5B Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf26.hostedemail.com (Postfix) with ESMTP id 42010140015 for ; Mon, 9 Oct 2023 10:15:52 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=gsQYRTEb; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=qGEvzez2; dmarc=none; spf=pass (imf26.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=1696846553; 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=9aFHFIVYD+PckYoucQFFA7Q682WFJMhzaZvRQ/S9EPY=; b=tVxOw2JQRhis+OfYDW2X+aWEJeVTtUwDAynuk/1YjaSWVC91sO/6sQ2VUfl+ogjr4fGpr9 msIWWYsjURqcU1SkgU57cWLegf5f3yZZmajYRt8g3RlBRu/MLoy9jcuNH7DFl2cmOUWLcY EaMznre5gkMptF+PiIGzrhOnFqI2sYs= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=gsQYRTEb; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=qGEvzez2; dmarc=none; spf=pass (imf26.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=1696846553; a=rsa-sha256; cv=none; b=leFde1xEr/DOAf32qWsbxF63qgZR7cSvWUQ0C4NgsOll3B04rT53XY/omF8IU13Do3vJ5C cHaU5Laum5iEjB1VZ3l2uK3mTrVZOfkRoSZlqN4GTg9XAn/l0DtYavmZiSzMQvdHGXI/H7 +npIPRmt84xZBUPl6RzMFcR0rG74+jo= 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 D093C210EA; Mon, 9 Oct 2023 10:15:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1696846550; 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=9aFHFIVYD+PckYoucQFFA7Q682WFJMhzaZvRQ/S9EPY=; b=gsQYRTEb0njWM9TpyXer3r9WyXV9IEVCnaR6yBLYDXzmPah64iYyEWy0schTQL56j382wU ebeI+dR4WlXBngIzleMTZ6eFCtrOhabfilROoT2ZMSGNUw7hTXLu57wireUa9RtvXxxSBk IXpMHR1y6fiWpE3v0Rh2Thpix3jr7T8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1696846550; 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=9aFHFIVYD+PckYoucQFFA7Q682WFJMhzaZvRQ/S9EPY=; b=qGEvzez2mF/FkU4Ol42KQIlRTDDiii7TYnAWg2gFBLvMprviOvhVfjy0GNDwi4Ha8aMADD fayHZjqOhIiP06DQ== 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 BE1B413905; Mon, 9 Oct 2023 10:15:50 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id b4xZLtbSI2WNeQAAMHmgww (envelope-from ); Mon, 09 Oct 2023 10:15:50 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 29F17A04B2; Mon, 9 Oct 2023 12:15:50 +0200 (CEST) Date: Mon, 9 Oct 2023 12:15:50 +0200 From: Jan Kara To: Mirsad Todorovac Cc: Matthew Wilcox , Yury Norov , 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: <20231009101550.pqnkrp5cp5zbr3lr@quack3> References: <20230918094116.2mgquyxhnxcawxfu@quack3> <22ca3ad4-42ef-43bc-51d0-78aaf274977b@alu.unizg.hr> <20230918113840.h3mmnuyer44e5bc5@quack3> <20230918155403.ylhfdbscgw6yek6p@quack3> <2c7f2acd-ef37-4504-a0e3-f74b66c455ec@alu.unizg.hr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2c7f2acd-ef37-4504-a0e3-f74b66c455ec@alu.unizg.hr> X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 42010140015 X-Stat-Signature: 8hjyh5z9ytfx3yig7b41ea8cz6zrg4e7 X-HE-Tag: 1696846552-684425 X-HE-Meta: U2FsdGVkX1+We7DL/nvTGtBGiqFXpdW2ChQcCfL/ZbiPBRq/XlmArMufiEhfAGnx9+BY0ZrCbqPAzMrzCfszaBU2MMEGl/SeccFfzpeRgU6U2WihAcqEqNb6Nfca1T4FSVllhIP6ifxrvxJh24fdAiD1jm48hTf3f7OU5OwR7TewG1JAe9cmE6eYzhUyl0rQpblbZQtx5C7Q86dSAH3yS7LtsYx11YksaXhww5e/OKPHsC9GEpdJfPe2uBxmiGmOzVBbCQ6lK7uTx/Y/4UWTMDcDWJEAHzxa1IAyCvvkmFG8SLTxwp8YVs/KEircWC4wBF+GOTHTy7FuMQ8BqbqIWSKLo8O0rxzIR97QAuSAQ0JSJpa97bhv91w4x5wtw7lCtgA3uUS4i9xTuN6J5kHMPhbjDRG9CLsvgtmamzyt7ll0vpspNZp1VkNH3It8BUCikoiwfX1GVNT2tjk9VcweRobMBqUAfvH+cHmuSK9A8LM91xBcfJrcUrz2Paqco7WqYl/Hw6DqpiUUjQyYubudhyNcUFeED/FqkKaGgXjsi8p6ozU5JcbKD3C2QNirpafFU8RSnLdYdM4c0goGye/hPI64Arvt0t+j1ZahrFcqtE4+SUn3toeLxfGzKlKefXAXlYkqzuA/Mz9lZ7FGKMfL9lzAE1pWoNXwGaZgaUWs/0metDIKrU0xGNF7TXTr99dMsuwhE7gfkkU4QMsgNMiWACBBC4VIObgtTx0/g3BaUonG4jUWCyejsCmstSZFaITHWVA4M3/WT2ukgAKeOI7fRKCZsNnGQs6oFM6m39k/31x+f4Sc5ig21ib3VEL/3+47u30ZoNkeh5/+sbkft8ODse3lvA+CIkG2VmvgFPzZcVRu8PIWzi6tJrOqARnoTczT5evpyZ5armpHQcBMBQBaIBS16a3zZA3ngVpL7jmZ2ZZe8aWFYFFTSQaVJ3g6RDeBY6VL+dA+5WTYM4bqlBB 9Az2N5hJ etBCYecU1R7oCBbUlec6VN6lq0ULSo75uF442vJ7e4hHTJhSKCuVTMJq/cid1NnFm8+HFPyucG/OVCN0+/JLuuJpxpWDWCIRkXfv4WoyKrR3gHtK3o6x09lS/cd09zMmkxZMRIm9NgGy4ESAmVA6tppYMzj1/i9cAKz24WdaGB7TK68ERJY5VbNtxaYGwWyj0Lc4Kj4JHZd7Xe5dEF1kF+sxvuFJ7eTZrPBWIDgw8stUSEWABrW6xCL8UKhHrSI/NI4C3mK5QThR3PNvpZPQHPEBvIiYC9zsrce5E2qq6O+CptcZnFixf7ByTRrKh3JY2A/2vNqnMjJvTZ/mTFCvZcmaZ/I4F2D7/+8FMbwnjhQN8cI4guCyQoQrR0A== 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 Fri 06-10-23 16:39:54, Mirsad Todorovac wrote: > On 9/19/2023 6:20 AM, Matthew Wilcox wrote: > > 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. > > Hi Mr. Wilcox, > > Do you think we should submit a formal patch for this data-race? So I did some benchmarking with various GCC versions and the truth is that READ_ONCE() does affect code generation a bit (although the original code does not refetch the value from memory). As a result my benchmarks show the bit searching functions are about 2% slower. This is not much but it is stupid to cause a performance regression due to non-issue. I'm trying to get some compiler guys look into this whether we can improve it somehow... Honza -- Jan Kara SUSE Labs, CR