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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1A2C6C433EF for ; Tue, 16 Nov 2021 14:45:18 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9ADDE61C15 for ; Tue, 16 Nov 2021 14:45:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9ADDE61C15 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 154716B0074; Tue, 16 Nov 2021 09:45:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 103EC6B007D; Tue, 16 Nov 2021 09:45:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F0E9F6B007E; Tue, 16 Nov 2021 09:45:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0076.hostedemail.com [216.40.44.76]) by kanga.kvack.org (Postfix) with ESMTP id E46966B0074 for ; Tue, 16 Nov 2021 09:45:16 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 907CD8249980 for ; Tue, 16 Nov 2021 14:45:16 +0000 (UTC) X-FDA: 78815066232.12.322BA81 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by imf12.hostedemail.com (Postfix) with ESMTP id 0275C10003CB for ; Tue, 16 Nov 2021 14:45:09 +0000 (UTC) Received: by mail-wm1-f48.google.com with SMTP id 133so17248186wme.0 for ; Tue, 16 Nov 2021 06:45:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=V9Y9KcPa2lBrD3VwSGoo5xYPKSJ2jtX2bGgZ++EHy0Q=; b=mgCBiXpImkz7mM4cJq4UY8PMvRzsm6lwKouZ+dcvq8QSVa3J7JFej5uyngBzBs3VG9 apP2Z2KnDB5lDZlgAnWWdBCIMaRp/9enlykRrPIMXnoMvjLqtHt0WpZc0g2YyHjeLydk /lQLwnitOv0S5e4LE3MrkKlJFQHSt8KKuyp7xrGkfwqrpHa/ibB3ODZXOTCs18KS7fiG 5Ktnv8UE7n7sDzHh65sX5ke0VLwyVYv435sLeKkgWupMowYh6aN856ah7UXctP7gcCwI 626yN4/2SbwvBKPhStTlohtmgVVNCNpPhrOtB0SuY5XbRiHjBNYUGhGuHafg7zP1SIJh N2Ug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=V9Y9KcPa2lBrD3VwSGoo5xYPKSJ2jtX2bGgZ++EHy0Q=; b=VmZYp6F9z3f0YTh1rTQiO44jSFvsIq+yDBr97bF2ZgHKRVbPpMdz8UHJhLB+2/68bM OhfcqdAISPXrq52LJ7daFwDrxAFos3qFAf/qs2rKGnxmCgTfEPT1GmOp9dxLbYzRdYqQ HBcbdJkD6APYN7WkmX+BeacXG7tegHKHxQmuRCCOP1FDrUNziqkosWHn/9vTcJzAjDcI uZwqT3MX+mTTHJD0VABxGvZoSxGk5NCqdfxD0JPi4OEGQUGNUpC+XjsIVNKMrSdXvWVz 4R21swzN46Qadlh23Kd3C63v1wY4x98CXohDTlNdVB2Gr3RIsf58YmdqNrA7IGdouFDV cpUg== X-Gm-Message-State: AOAM531GFemU2Xqg1ndqH0ng66/Hfs4f3RzePF5ZuvQFOcd0J3GJ+KeY BJHS17iw+sF+fnBDLElWljBoxzefx+lQ4lce X-Google-Smtp-Source: ABdhPJwUc+UlWH9Tc9SEHhXGVwFTVxffKLubOuzcgHdKK5Ik5pUTSDWywCEFBioj4I/hmRhY4Wxy8Q== X-Received: by 2002:a1c:80c5:: with SMTP id b188mr7359307wmd.57.1637064278455; Tue, 16 Nov 2021 04:04:38 -0800 (PST) Received: from elver.google.com ([2a00:79e0:15:13:ee27:74df:199e:beab]) by smtp.gmail.com with ESMTPSA id f7sm3226528wmg.6.2021.11.16.04.04.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Nov 2021 04:04:37 -0800 (PST) Date: Tue, 16 Nov 2021 13:04:32 +0100 From: Marco Elver To: Shakeel Butt Cc: Mina Almasry , paulmck@kernel.org, Mike Kravetz , Muchun Song , Andrew Morton , Shuah Khan , Miaohe Lin , Oscar Salvador , Michal Hocko , David Rientjes , Jue Wang , Yang Yao , Joanna Li , Cannon Matthews , Linux Memory Management List , LKML , kasan-dev@googlegroups.com Subject: Re: [PATCH v6] hugetlb: Add hugetlb.*.numa_stat file Message-ID: References: <20211111015037.4092956-1-almasrymina@google.com> <6887a91a-9ec8-e06e-4507-b2dff701a147@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.5 (2021-01-21) X-Rspamd-Queue-Id: 0275C10003CB X-Stat-Signature: mnwd1ubb5f747h7s4ie5qm9wc3wyaais Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=mgCBiXpI; spf=pass (imf12.hostedemail.com: domain of elver@google.com designates 209.85.128.48 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam03 X-HE-Tag: 1637073909-318003 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, Nov 15, 2021 at 11:59AM -0800, Shakeel Butt wrote: > On Mon, Nov 15, 2021 at 10:55 AM Mina Almasry wrote: [...] > > Sorry I'm still a bit confused. READ_ONCE/WRITE_ONCE isn't documented > > to provide atomicity to the write or read, just prevents the compiler > > from re-ordering them. Is there something I'm missing, or is the > > suggestion to add READ_ONCE/WRITE_ONCE simply to supress the KCSAN > > warnings? It's actually the opposite: READ_ONCE/WRITE_ONCE provide very little ordering (modulo dependencies) guarantees, which includes ordering by compiler, but are supposed to provide atomicity (when used with properly aligned types up to word size [1]; see __READ_ONCE for non-atomic variant). Some more background... The warnings that KCSAN tells you about are "data races", which occur when you have conflicting concurrent accesses, one of which is "plain" and at least one write. I think [2] provides a reasonable summary of data races and why we should care. For Linux, our own memory model (LKMM) documents this [3], and says that as long as concurrent operations are marked (non-plain; e.g. *ONCE), there won't be any data races. There are multiple reasons why data races are undesirable, one of which is to avoid bad compiler transformations [4], because compilers are oblivious to concurrency otherwise. Why do marked operations avoid data races and prevent miscompiles? Among other things, because they should be executed atomically. If they weren't a lot of code would be buggy (there had been cases where the old READ_ONCE could be used on data larger than word size, which certainly weren't atomic, but this is no longer possible). [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/asm-generic/rwonce.h#n35 [2] https://lwn.net/Articles/816850/#Why%20should%20we%20care%20about%20data%20races? [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/memory-model/Documentation/explanation.txt#n1920 [4] https://lwn.net/Articles/793253/ Some rules of thumb when to use which marking: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/memory-model/Documentation/access-marking.txt In an ideal world, we'd have all intentionally concurrent accesses marked. As-is, KCSAN will find: A. Data race, where failure due to current compilers is unlikely (supposedly "benign"); merely marking the accesses appropriately is sufficient. Finding a crash for these will require a miscompilation, but otherwise look "benign" at the C-language level. B. Race-condition bugs where the bug manifests as a data race, too -- simply marking things doesn't fix the problem. These are the types of bugs where a data race would point out a more severe issue. Right now we have way too much of type (A), which means looking for (B) requires patience. > +Paul & Marco > > Let's ask the experts. > > We have a "unsigned long usage" variable that is updated within a lock > (hugetlb_lock) but is read without the lock. > > Q1) I think KCSAN will complain about it and READ_ONCE() in the > unlocked read path should be good enough to silent KCSAN. So, the > question is should we still use WRITE_ONCE() as well for usage within > hugetlb_lock? KCSAN's default config will forgive the lack of WRITE_ONCE(). Technically it's still a data race (which KCSAN can find with a config change), but can be forgiven because compilers are less likely to cause trouble for writes (background: https://lwn.net/Articles/816854/ bit about "Unmarked writes (aligned and up to word size)..."). I would mark both if feasible, as it clearly documents the fact the write can be read concurrently. > Q2) Second question is more about 64 bit archs breaking a 64 bit write > into two 32 bit writes. Is this a real issue? If yes, then the > combination of READ_ONCE()/WRITE_ONCE() are good enough for the given > use-case? Per above, probably unlikely, but allowed. WRITE_ONCE should prevent it, and at least relieve you to not worry about it (and shift the burden to WRITE_ONCE's implementation). Thanks, -- Marco