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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 3480FC35247 for ; Fri, 7 Feb 2020 00:55:43 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D3D8B20838 for ; Fri, 7 Feb 2020 00:55:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lca.pw header.i=@lca.pw header.b="BBGuoH4l" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D3D8B20838 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 647136B0003; Thu, 6 Feb 2020 19:55:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5F7E76B0006; Thu, 6 Feb 2020 19:55:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4E79D6B0007; Thu, 6 Feb 2020 19:55:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0144.hostedemail.com [216.40.44.144]) by kanga.kvack.org (Postfix) with ESMTP id 345786B0003 for ; Thu, 6 Feb 2020 19:55:42 -0500 (EST) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id E4D94181AEF15 for ; Fri, 7 Feb 2020 00:55:41 +0000 (UTC) X-FDA: 76461513282.18.crown95_88ffba7e56860 X-HE-Tag: crown95_88ffba7e56860 X-Filterd-Recvd-Size: 7131 Received: from mail-qk1-f196.google.com (mail-qk1-f196.google.com [209.85.222.196]) by imf50.hostedemail.com (Postfix) with ESMTP for ; Fri, 7 Feb 2020 00:55:41 +0000 (UTC) Received: by mail-qk1-f196.google.com with SMTP id v195so590260qkb.11 for ; Thu, 06 Feb 2020 16:55:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lca.pw; s=google; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=p/R0f3V2QFDfmc86YhIhDY9+JDhyDpcA5H1ziRMRmpw=; b=BBGuoH4ljNf/nsbHXmKFXmeibCtToF0UNbAw3aMRh+69fImEKVZ7WDHIxU2vwAK/rM qRtIQA2bAYT6FF8ZqdcCwDjiUKoQNSw1CAD4bOxrp9icysMpkOzz8lKO56W4yNo6OriH Jt9VtlgGaId+xsdyXSF2zzVf3CAOxStkjWu4y4GBU5bhqH8SZdjgcr6d15n44iGoU0/r yt6Khu+Jf6Ng5yanUKjysg4Etfyzk3QlHJLsgsaD50bX2dQki4v8udeniiVNW21JKm7O W9A8kGUkdbL2L2rlz5GocvQiIiBqRij4BY4mKcl2tKcX5s/h6ZuXnsjQg/8uVzjd38AJ Qr+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=p/R0f3V2QFDfmc86YhIhDY9+JDhyDpcA5H1ziRMRmpw=; b=lAmd9OYYu598d9e0wWF5PzAhygg+GiOZXawt5ftHOg/sbdKiymg3TuxEG1d8Hbzm2u u63LJ3x6xC+kHwhQEKxC8mNcUg58RqEhjrXoW+/lHtc5o/AJjF34H1hITriG24f2VZp1 hykXUZB1SD6DlPZpZZALkrvZBYN660s8M6Xd1+X/mOk33BgQ3SvlUkn4YYN5rFXxbndy eOEsWi9W7t3HXBOhT6KvnQjq4zbUETJa0Ii4NZFf1Q4N1U4iwLooGdkJWwakEIvBlT48 1+PHDnVgqBEp7FSP5141CLgPNhNBrPhWXJ3tMGxZSPXEVOqN5Azc4Sfav6Y4Kx4YVF+d gDWA== X-Gm-Message-State: APjAAAXWky1pIwcAKBKoAaXzby1dNGjl4WUYb+F6iha0ww/uC8ZRsxgM r5loKMldFLzvVebb8kt31VS8RA== X-Google-Smtp-Source: APXvYqylHGR1vme3BY/+qExRUkU3mklEmQvEqAdTdVc9OyvDOG07clXlLFLYLXGmQj8HjiH/lyLAdw== X-Received: by 2002:ae9:f709:: with SMTP id s9mr5073023qkg.463.1581036940798; Thu, 06 Feb 2020 16:55:40 -0800 (PST) Received: from [192.168.1.153] (pool-71-184-117-43.bstnma.fios.verizon.net. [71.184.117.43]) by smtp.gmail.com with ESMTPSA id f32sm540876qtk.89.2020.02.06.16.55.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 06 Feb 2020 16:55:40 -0800 (PST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) Subject: Re: [PATCH] mm: fix a data race in put_page() From: Qian Cai In-Reply-To: <90ab0b09-0f70-fe6d-259e-f529f4ef9174@nvidia.com> Date: Thu, 6 Feb 2020 19:55:38 -0500 Cc: Jan Kara , David Hildenbrand , Andrew Morton , ira.weiny@intel.com, Dan Williams , Marco Elver , Linux Memory Management List , Linux Kernel Mailing List Content-Transfer-Encoding: quoted-printable Message-Id: <1CFC5A47-3334-4696-89FE-CDF01026B12B@lca.pw> References: <20200206145501.GD26114@quack2.suse.cz> <079c4429-8a11-154d-cf5c-473d2698d18d@nvidia.com> <235ACF21-35BE-4EDA-BA64-9553DA53BF12@lca.pw> <90ab0b09-0f70-fe6d-259e-f529f4ef9174@nvidia.com> To: John Hubbard X-Mailer: Apple Mail (2.3608.60.0.2.5) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000174, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: > On Feb 6, 2020, at 7:27 PM, John Hubbard wrote: >=20 > On 2/6/20 4:18 PM, Qian Cai wrote: >>> On Feb 6, 2020, at 6:34 PM, John Hubbard = wrote: >>> On 2/6/20 7:23 AM, Qian Cai wrote: >>>>> On Feb 6, 2020, at 9:55 AM, Jan Kara wrote: >>>>> I don't think the problem is real. The question is how to make = KCSAN happy >>>>> in a way that doesn't silence other possibly useful things it can = find and >>>>> also which makes it most obvious to the reader what's going on... = IMHO >>>>> using READ_ONCE() fulfills these targets nicely - it is free >>>>> performance-wise in this case, it silences the checker without = impacting >>>>> other races on page->flags, its kind of obvious we don't want the = load torn >>>>> in this case so it makes sense to the reader (although a comment = may be >>>>> nice). >>>>=20 >>>> Actually, use the data_race() macro there fulfilling the same = purpose too, i.e, silence the splat here but still keep searching for = other races. >>>>=20 >>>=20 >>> Yes, but both READ_ONCE() and data_race() would be saying untrue = things about this code, >>> and that somewhat offends my sense of perfection... :) >>>=20 >>> * READ_ONCE(): this field need not be restricted to being read only = once, so the >>> name is immediately wrong. We're using side effects of READ_ONCE(). >>>=20 >>> * data_race(): there is no race on the N bits worth of page zone = number data. There >>> is only a perceived race, due to tools that look at word-level = granularity. >>>=20 >>> I'd propose one or both of the following: >>>=20 >>> a) Hope that Marco (I've fixed the typo in his name. --jh) has an = idea to enhance KCSAN so as to support this model of >>> access, and/or >>=20 >> A similar thing was brought up before, i.e., anything compared to = zero is immune to load-tearing >> issues, but it is rather difficult to implement it in the compiler, = so it was settled to use data_race(), >>=20 >> = https://lore.kernel.org/lkml/CANpmjNN8J1oWtLPHTgCwbbtTuU_Js-8HD=3DcozW5cYk= m8h-GTBg@mail.gmail.com/#r >>=20 >=20 >=20 > Thanks for that link to the previous discussion, good context. >=20 >=20 >>>=20 >>> b) Add a new, better-named macro to indicate what's going on. = Initial bikeshed-able >>> candidates: >>>=20 >>> READ_RO_BITS() >>> READ_IMMUTABLE_BITS() >>> ...etc... >>>=20 >>=20 >> Actually, Linus might hate those kinds of complication rather than a = simple data_race() macro, >>=20 >> = https://lore.kernel.org/linux-fsdevel/CAHk-=3Dwg5CkOEF8DTez1Qu0XTEFw_oHhxN= 98bDnFqbY7HL5AB2g@mail.gmail.com/ >>=20 >=20 > Another good link. However, my macros above haven't been proposed yet, = and I'm perfectly=20 > comfortable proposing something that Linus *might* (or might not!) = hate. No point in=20 > guessing about it, IMHO. >=20 > If you want, I'll be happy to put on my flame suit and post a patchset = proposing=20 > READ_IMMUTABLE_BITS() (or a better-named thing, if someone has another = name idea). :) >=20 BTW, the current comment said (note, it is called =E2=80=9Caccess=E2=80=9D= which in this case it does read the whole word rather than those 3 bits, even though it is only those bits are of = interested for us), /* * data_race(): macro to document that accesses in an expression may = conflict with * other concurrent accesses resulting in data races, but the resulting * behaviour is deemed safe regardless. * * This macro *does not* affect normal code generation, but is a hint to = tooling * that data races here should be ignored. */ Macro might have more to say.=