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 D7E16CD13CF for ; Sun, 17 Sep 2023 19:09:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C2CDA6B01A9; Sun, 17 Sep 2023 15:09:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BDD506B01AA; Sun, 17 Sep 2023 15:09:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AA3F76B01AB; Sun, 17 Sep 2023 15:09:23 -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 9AEA46B01A9 for ; Sun, 17 Sep 2023 15:09:23 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 5FD0B1CA44A for ; Sun, 17 Sep 2023 19:09:23 +0000 (UTC) X-FDA: 81247027806.29.935C45B Received: from domac.alu.hr (domac.alu.unizg.hr [161.53.235.3]) by imf16.hostedemail.com (Postfix) with ESMTP id CD33A180011 for ; Sun, 17 Sep 2023 19:09:20 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=alu.unizg.hr header.s=mail header.b="s2xc7/yL"; dkim=pass header.d=alu.unizg.hr header.s=mail header.b=xfiFVRW8; dmarc=pass (policy=none) header.from=alu.unizg.hr; spf=pass (imf16.hostedemail.com: domain of mirsad.todorovac@alu.unizg.hr designates 161.53.235.3 as permitted sender) smtp.mailfrom=mirsad.todorovac@alu.unizg.hr ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694977761; a=rsa-sha256; cv=none; b=5OUj306ROw8uyHqEiH6WkDGXQB+nO1dHnUr6GcSO0Yy6617G3QobE7Q907/TCCitMbEz6v 37GW7wrXkoVxGUgTei4vp/IUwiSr9QzzneyWnJxNLjW7HtihQJQBlbdK9Rs5B4wi+6pwkw tLTp12DrkLoLFb0jGmDDbi87a95nnVE= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=alu.unizg.hr header.s=mail header.b="s2xc7/yL"; dkim=pass header.d=alu.unizg.hr header.s=mail header.b=xfiFVRW8; dmarc=pass (policy=none) header.from=alu.unizg.hr; spf=pass (imf16.hostedemail.com: domain of mirsad.todorovac@alu.unizg.hr designates 161.53.235.3 as permitted sender) smtp.mailfrom=mirsad.todorovac@alu.unizg.hr ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694977761; 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=QILXUHinr4L72s4mJEKeMkUGzr5tYJrB6XoN0Ox2q6c=; b=18cb3MsEC/q77XiFS5v3R3fxqULoXZ/Yf+3+rJbBCb5Kr5L0WBlMhkFeuNZyjpkBOuSrtw LSyCOB2uQ3D/uCZDOvUsDqyC64dFRCR4P1p5AfZF+xsuY3sjxfyEhzMErbiIHzKQGRAw4m 20JhnaJEbHOGNv4wWbOP2gHGjVL5zxo= Received: from localhost (localhost [127.0.0.1]) by domac.alu.hr (Postfix) with ESMTP id 1D88260157; Sun, 17 Sep 2023 21:09:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=alu.unizg.hr; s=mail; t=1694977758; bh=J2GzxI4nKONyIeyCnRvL4IDtSLiLqiX3VYZ/pESlvbQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=s2xc7/yLlQO2ZxByvSifmy9yaLhlJ8SwoB9jisWE0fKUpdVFjwIHwmhkah+9oi0Wa gPZPMCiXMP2ztNWyELrplUSj8sQoBTyfqpFXsFcaPnI5Dz6zkiVflTEkNwJr1BiNjd +AALeFSpSmxYai0U42HHADWHfCe5ohb5Cfpb94X1VMtragTN8vs36fmjVVN444HVzf zDQDFCUx6OhompZW3kq8Y9zd2EHagKyU+UvoDi67uLY/pbg37+M7XGpR1qRxrpzHNK doA9fr/u+SxArIn57IgEamqbnROkeipugMfa4l9fi0zdtc6D6qiPkHkWQLnwPRnssG W6nZb2F0qClGQ== X-Virus-Scanned: Debian amavisd-new at domac.alu.hr Received: from domac.alu.hr ([127.0.0.1]) by localhost (domac.alu.hr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IwPoN2A_Z6Mm; Sun, 17 Sep 2023 21:09:15 +0200 (CEST) Received: from [192.168.1.6] (78-2-88-58.adsl.net.t-com.hr [78.2.88.58]) by domac.alu.hr (Postfix) with ESMTPSA id ED53D60155; Sun, 17 Sep 2023 21:09:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=alu.unizg.hr; s=mail; t=1694977755; bh=J2GzxI4nKONyIeyCnRvL4IDtSLiLqiX3VYZ/pESlvbQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=xfiFVRW8iwNRMZrcXU8lHie0TaHNu0mMI2+3PSwseHYoyIqEs/045aDmzesmk08X8 +kU1E1J09Kw//yEYUeOEIPBEzTmRMy37GaxUSby/VYsDhhMGzlIYQolUNYtRycj/Xg jtk24u58a/0QyD7cm4f5uh6c4LFxu/xQkflKUKiS1ddpcj5CuTsXQ9lrb2cj22+Dg8 XF/nrwYTLyplaVdCz2vPvcKNH2vVgpvf+rsyjMH61rSyUVDZ9EbqIUzawuNtFbYQGP Js/OxDZmvxhxG3Rvn0nl+eobwgkQVsCfs3zDyMWktCjqPFA+ehBhcCpGz4KSEmPD4O rcSfkuqvbbyKQ== Message-ID: <3cfe5345-66a0-bb3b-a1d4-02ff2b3b098b@alu.unizg.hr> Date: Sun, 17 Sep 2023 21:09:10 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [BUG] KCSAN: data-race in xas_clear_mark / xas_find_marked To: Jan Kara , Matthew Wilcox Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Andrew Morton , linux-mm@kvack.org, Chris Mason , Josef Bacik , David Sterba , linux-btrfs@vger.kernel.org References: <06645d2b-a964-1c4c-15cf-42ccc6c6e19b@alu.unizg.hr> <20230914080811.465zw662sus4uznq@quack3> Content-Language: en-US From: Mirsad Todorovac In-Reply-To: <20230914080811.465zw662sus4uznq@quack3> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: CD33A180011 X-Stat-Signature: iuhb16jre77n5hdcnx953hguadnxffym X-HE-Tag: 1694977760-100937 X-HE-Meta: U2FsdGVkX1/8+kM7jN+j85Dea3A8QO88pdWQB1ZgLIRCyb4rdsLISOhsmM7VLqfpQGybBWKXKowDZ22CmuNeUWHwbZWkwWSttVDivpL1DDSxdFokI4WbCTygjGUcIUf+wcHMZ8xKhxk8ViAtbXqtBlWq2w/yUYYQjDtaHB0PaeepuZEUrYRBsXimKbiE/44aDlU3kS8EqQFAGHBO+nEhj1FUfY7DQ7DHGgDFTpOsPXsiekvqlzyJc/UcCfwYVUDfJNNJmmw8j+O5QwGRfXMaU2ONdXwaorUUe37jAXNwdrJuhEjPBDYxfCBDJ/SQc03LGPzaamBUm2SYxBobo78LFGqXdBiLD9hxVuYCe1i+qngGJdmd71YKHhXk1RmT5mAO6Z6Z+dF6Aq0D8ZeQ5t7T05Am6TZZ38vVFlw9AuhlqzcUECp0ily8jHhCWKaifyIIUWPn8f6oWq38PXxVjmove7tN8omQg3R6lHip9IVUwNtOLSVXXiB/bqDfySesAdhvlASuRIwU0awz2YpBAaE+rjiEeKqvD/quvKLLxgFKnJ+i9eeOEGKH4VIe2BW5cb6m0uuSGpZo3LL8KgE4V5xt493boIL7ANE+PGyiWuRYDi+MaRDUVX3OLd+PdwHneDfG4jpgB28ccdys+wuHqi6B3QXbenfzCwZiO6QUE+H89SrBf60l48vmYWJsX4BCfeAhBXpKhLuRZCaFrLwcBvrbC6x3nF33DnOSK2YRUZIC2FbAuM4d/nemkXCeopmzQQ1Yn9jhUqpvfES9fjA7xOpsCK2leTCkUe3J9zzwtT53xj73l/zFQUqap4m61xKXlEQBB1Sfx+AuM6/+V97kg8V5Ga7O+Shu+TxYOKy4yiV8lQ9+hY2ACo8zDJtI671Y/Ly8F9XID8GUnSEJ8A2oLubiPab09WYBC1/KR0X60wUmWOgOVTqyTHcQe+dSYWm2FWfg4odjvrJY0qAq5uPWGbY KCGvZCqQ os+mflRqPOBcoepmi9EvaM/V9XuJwOfqSgZjf5EQrOoBDHPgUFly07xQTIdMUbnxvlG6HIVDrtc3AFzLHZQeTDH+fUSj2jiU7OoREHoihPJ6+uQ16IFKIkG6O2zDSZaeBSMi98zCZreO21bZBiuGSa8OKq3sUjKbxcvzN1mz4Zyr1X0fQSlAKEB85vB5/LXWEpDIOQFmnkQmzxjVRmnIF9kIV18nwxPp49pDy 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 9/14/23 10:08, Jan Kara wrote: > On Fri 18-08-23 13:21:17, Matthew Wilcox wrote: >> On Fri, Aug 18, 2023 at 10:01:32AM +0200, Mirsad Todorovac wrote: >>> [ 206.510010] ================================================================== >>> [ 206.510035] BUG: KCSAN: data-race in xas_clear_mark / xas_find_marked >>> >>> [ 206.510067] write to 0xffff963df6a90fe0 of 8 bytes by interrupt on cpu 22: >>> [ 206.510081] xas_clear_mark+0xd5/0x180 >>> [ 206.510097] __xa_clear_mark+0xd1/0x100 >>> [ 206.510114] __folio_end_writeback+0x293/0x5a0 >>> [ 206.520722] read to 0xffff963df6a90fe0 of 8 bytes by task 2793 on cpu 6: >>> [ 206.520735] xas_find_marked+0xe5/0x600 >>> [ 206.520750] filemap_get_folios_tag+0xf9/0x3d0 >> Also, before submitting this kind of report, you should run the >> trace through scripts/decode_stacktrace.sh to give us line numbers >> instead of hex offsets, which are useless to anyone who doesn't have >> your exact kernel build. >> >>> [ 206.510010] ================================================================== >>> [ 206.510035] BUG: KCSAN: data-race in xas_clear_mark / xas_find_marked >>> >>> [ 206.510067] write to 0xffff963df6a90fe0 of 8 bytes by interrupt on cpu 22: >>> [ 206.510081] xas_clear_mark (./arch/x86/include/asm/bitops.h:178 ./include/asm-generic/bitops/instrumented-non-atomic.h:115 lib/xarray.c:102 lib/xarray.c:914) >>> [ 206.510097] __xa_clear_mark (lib/xarray.c:1923) >>> [ 206.510114] __folio_end_writeback (mm/page-writeback.c:2981) >> >> This path is properly using xa_lock_irqsave() before calling >> __xa_clear_mark(). >> >>> [ 206.520722] read to 0xffff963df6a90fe0 of 8 bytes by task 2793 on cpu 6: >>> [ 206.520735] xas_find_marked (./include/linux/xarray.h:1706 lib/xarray.c:1354) >>> [ 206.520750] filemap_get_folios_tag (mm/filemap.c:1975 mm/filemap.c:2273) >> >> This takes the RCU read lock before calling xas_find_marked() as it's >> supposed to. >> >> What garbage do I have to write to tell KCSAN it's wrong? The line >> that's probably triggering it is currently: >> >> unsigned long data = *addr & (~0UL << offset); > > I don't think it is actually wrong in this case. You're accessing xarray > only with RCU protection so it can be changing under your hands. For > example the code in xas_find_chunk(): > > unsigned long data = *addr & (~0UL << offset); > if (data) > return __ffs(data); > > is prone to the compiler refetching 'data' from *addr after checking for > data != 0 and getting 0 the second time which would trigger undefined > behavior of __ffs(). So that code should definitely use READ_ONCE() to make > things safe. > > BTW, find_next_bit() seems to need a similar treatment and in fact I'm not > sure why xas_find_chunk() has a special case for XA_CHUNK_SIZE == > BITS_PER_LONG because find_next_bit() checks for that and handles that in a > fast path in the same way. > > Honza Hi, Thank you for your insight on the matter. I guess you meant something like implementing this: include/linux/xarray.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/xarray.h b/include/linux/xarray.h index cb571dfcf4b1..1715fd322d62 100644 --- a/include/linux/xarray.h +++ b/include/linux/xarray.h @@ -1720,7 +1720,7 @@ static inline unsigned int xas_find_chunk(struct xa_state *xas, bool advance, offset++; if (XA_CHUNK_SIZE == BITS_PER_LONG) { if (offset < XA_CHUNK_SIZE) { - unsigned long data = *addr & (~0UL << offset); + unsigned long data = READ_ONCE(*addr) & (~0UL << offset); if (data) return __ffs(data); } This apparently clears the KCSAN xas_find_marked() warning, so this might have been a data race after all. Do you think we should escalate this to a formal patch? Best regards, Mirsad Todorovac