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 X-Spam-Level: X-Spam-Status: No, score=-2.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_2 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CED3AC352A3 for ; Mon, 10 Feb 2020 13:36:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9380420714 for ; Mon, 10 Feb 2020 13:36:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=lca.pw header.i=@lca.pw header.b="hOLmFJuX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9380420714 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lca.pw Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 485286B00F6; Mon, 10 Feb 2020 08:36:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 435E26B00F7; Mon, 10 Feb 2020 08:36:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3724D6B00F8; Mon, 10 Feb 2020 08:36:13 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0052.hostedemail.com [216.40.44.52]) by kanga.kvack.org (Postfix) with ESMTP id 20E7E6B00F6 for ; Mon, 10 Feb 2020 08:36:13 -0500 (EST) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id CD829181AEF00 for ; Mon, 10 Feb 2020 13:36:12 +0000 (UTC) X-FDA: 76474316184.02.home00_705df22027a38 X-HE-Tag: home00_705df22027a38 X-Filterd-Recvd-Size: 6034 Received: from mail-qt1-f194.google.com (mail-qt1-f194.google.com [209.85.160.194]) by imf40.hostedemail.com (Postfix) with ESMTP for ; Mon, 10 Feb 2020 13:36:12 +0000 (UTC) Received: by mail-qt1-f194.google.com with SMTP id n17so5117199qtv.2 for ; Mon, 10 Feb 2020 05:36:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lca.pw; s=google; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=K7xCPnfPDVaR+RixreW4kHYksD72YNNnHkzPeZTukvc=; b=hOLmFJuXSPhNr6hGsvNbQBjooSUc+bvOw+weuVWZ5Oj47eXtyN14KfFin8aMDfNy5e RmJmZt5GHnHXy+AEFUxnaznGVp5oZk9l2PloyqdMT2c0C4bsjUzjVvu58VTesYrASUPI +Fj9ij42AFVr/C2hVYNqaJyTni4jCPd5/cIZ5MlY/nwwREyYbBvTuOQhc1xkG0FdhoK2 37Q5riHmSOGrjnjzkzrFnA226x/CVe15zMTB/bpjcrNZmXt+GGvqPfUtSMLeQ85hWtzv 7oiczKj8+oUlgM2ZuXhrFPs9oI+KdQsYJqvEtPpQ2fnG9hMKOtBWq6w6H8oypBKCJuMv xYvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=K7xCPnfPDVaR+RixreW4kHYksD72YNNnHkzPeZTukvc=; b=owRpvruDeOl1qZPP6de+QIVJM+i6HzRgL6QK4A+DanTamxG7ZTpwz3JemAEzcflz60 cYXHWNtadp6OuTgRB000+pbKeMkjOHsPHbhhIpR/RSBmPsgG8LOE4MoxYZIqKfmk5qc5 vRHm9zix0eDl/7GybAFYIwhSJLcPZFA1a4DNvbDFKGbP7Pqrp4+OHMOuU3h2MuFnQnMK 3chqYs633lAdljjEA9Mq3XfLS7/tk5yF4zMVTxMbq0WmjbDOzKRl1yueYsS7yqTcAori 3F3bbBhxtt/m5n/J/lC/20kgyg9lkRwD5R3hwSyGhhkFzN93rPc1+9XymSSGIm4dL/JA GVkg== X-Gm-Message-State: APjAAAUNL3aykwVTKA/ub/+/iVYfuJr/WXIe7E3Vx1F+2CtFZDNIxXp4 fCVzZcmD0nfEOcRWs4nqFpdl7Q== X-Google-Smtp-Source: APXvYqzfCrSW05EQXUwLqWEr2tZP7aRcMoWF/NyH7uQ5NOu69GP0VRROWubYpavskQBcxywAN8selw== X-Received: by 2002:ac8:163c:: with SMTP id p57mr10090991qtj.106.1581341771026; Mon, 10 Feb 2020 05:36:11 -0800 (PST) Received: from dhcp-41-57.bos.redhat.com (nat-pool-bos-t.redhat.com. [66.187.233.206]) by smtp.gmail.com with ESMTPSA id 205sm144034qkd.61.2020.02.10.05.36.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Feb 2020 05:36:10 -0800 (PST) Message-ID: <1581341769.7365.25.camel@lca.pw> Subject: Re: [PATCH] mm: fix a data race in put_page() From: Qian Cai To: Marco Elver Cc: John Hubbard , Jan Kara , David Hildenbrand , Andrew Morton , ira.weiny@intel.com, Dan Williams , Linux Memory Management List , Linux Kernel Mailing List , "Paul E. McKenney" , kasan-dev Date: Mon, 10 Feb 2020 08:36:09 -0500 In-Reply-To: References: <26B88005-28E6-4A09-B3A7-DC982DABE679@lca.pw> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6 (3.22.6-10.el7) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable 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, 2020-02-10 at 13:58 +0100, Marco Elver wrote: > On Mon, 10 Feb 2020 at 13:16, Qian Cai wrote: > >=20 > >=20 > >=20 > > > On Feb 10, 2020, at 2:48 AM, Marco Elver wrote: > > >=20 > > > Here is an alternative: > > >=20 > > > Let's say KCSAN gives you this: > > > /* ... Assert that the bits set in mask are not written > > > concurrently; they may still be read concurrently. > > > The access that immediately follows is assumed to access those > > > bits and safe w.r.t. data races. > > >=20 > > > For example, this may be used when certain bits of @flags may > > > only be modified when holding the appropriate lock, > > > but other bits may still be modified locklessly. > > > ... > > > */ > > > #define ASSERT_EXCLUSIVE_BITS(flags, mask) .... > > >=20 > > > Then we can write page_zonenum as follows: > > >=20 > > > static inline enum zone_type page_zonenum(const struct page *page) > > > { > > > + ASSERT_EXCLUSIVE_BITS(page->flags, ZONES_MASK << ZONES_PGSH= IFT); > > > return (page->flags >> ZONES_PGSHIFT) & ZONES_MASK; > > > } > > >=20 > > > This will accomplish the following: > > > 1. The current code is not touched, and we do not have to verify th= at > > > the change is correct without KCSAN. > > > 2. We're not introducing a bunch of special macros to read bits in = various ways. > > > 3. KCSAN will assume that the access is safe, and no data race repo= rt > > > is generated. > > > 4. If somebody modifies ZONES bits concurrently, KCSAN will tell yo= u > > > about the race. > > > 5. We're documenting the code. > > >=20 > > > Anything I missed? > >=20 > > I don=E2=80=99t know. Having to write the same line twice does not fe= el me any better than data_race() with commenting occasionally. >=20 > Point 4 above: While data_race() will ignore cause KCSAN to not report > the data race, now you might be missing a real bug: if somebody > concurrently modifies the bits accessed, you want to know about it! > Either way, it's up to you to add the ASSERT_EXCLUSIVE_BITS, but just > remember that if you decide to silence it with data_race(), you need > to be sure there are no concurrent writers to those bits. Right, in this case, there is no concurrent writers to those bits, so I'l= l add a comment should be sufficient. However, I'll keep ASSERT_EXCLUSIVE_BITS() = in mind for other places. >=20 > There is no way to automatically infer all over the kernel which bits > we care about, and the most reliable is to be explicit about it. I > don't see a problem with it per se. >=20 > Thanks, > -- Marco