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 42D6DC433F5 for ; Tue, 16 Nov 2021 21:48:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E568163223 for ; Tue, 16 Nov 2021 21:48:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E568163223 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 851B16B0071; Tue, 16 Nov 2021 16:48:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D8C16B0073; Tue, 16 Nov 2021 16:48:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 652736B0074; Tue, 16 Nov 2021 16:48:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0218.hostedemail.com [216.40.44.218]) by kanga.kvack.org (Postfix) with ESMTP id 537C86B0071 for ; Tue, 16 Nov 2021 16:48:09 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 253D6181616B7 for ; Tue, 16 Nov 2021 21:47:59 +0000 (UTC) X-FDA: 78816131478.21.064A352 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by imf06.hostedemail.com (Postfix) with ESMTP id ABDEB801AB0C for ; Tue, 16 Nov 2021 21:47:58 +0000 (UTC) Received: by mail-wm1-f45.google.com with SMTP id 77-20020a1c0450000000b0033123de3425so3171674wme.0 for ; Tue, 16 Nov 2021 13:47:58 -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=CH+wOU8IkpDGfrPMJt+XhHiyZDF6T70EhzybBc8lVmM=; b=qEwFgJ4U7LTjWdRCTYx0yi6+SIjDlyTtX2cbtRLGNQrkc13D/DB/O5DJw9Y9of0/LO pXATjSXWlIB1hiT+NIkT6g2XxmSKx8Z0CwdEP4aMWyB0FmTcJUSYFcMrGNGQ+y6FluvS mzZ1tQE0pk8WAqb1b0tRiFOEzN8AJsFZrgVd2O7mQxozfBz0aT703TZUQHVxLwBGrhT7 5Ag2Cwtm+bBIQQVOYoj5n+RUNZ3o2VQgsOQa1qyiXoUhhJYtZfK24vMZHQ5SaBP2q+3C JZQ0Bt1u/kSixXsHaW6xsRvvOBtjyj+b8i9TN/CcdSWOuY2nzdQ+WpaoJPVXLYjCkwwv uQEA== 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=CH+wOU8IkpDGfrPMJt+XhHiyZDF6T70EhzybBc8lVmM=; b=mOj5rfWQXQz0keBapqxkJpE9xJXQ1R3EfXc9hwAp/a1ufS/vMt56RynZiFcGXv7v4F 2CHHvgyxyn7W+bgOvwpKVEl92UAg9YQTS0kZaqt952jz5sbxnT0l7CVlqGuyf664IcUi 48wGqe5/lEahkojqVO58e4Lz+ORh5ZxlmpW/6T0IybrM3okYv9G/ntvnfd7RCBY6zgtS 74V555mIjwvgPcrDoIRa3n+pMZjA9Cg1w2648uTfhwWKnql1PfG/jQP9A10jCZC769FJ rVNZuLMWqkp+IaPAk0KjmH8QpjfkVszFQSZpVAKBDUyIwjk2D6w9fnlaXhbFD1lmMcW0 jVHA== X-Gm-Message-State: AOAM531t/nLhbJqIO6BrHzeYTQ3qAfmNsP7AvxF1WqzQivV6H6z40Eiz ZHujjP+WSqTL6Ke0loUCJVTt0Q== X-Google-Smtp-Source: ABdhPJz/gXu7UjAE8Pg+B9OjTYttzkEf3y6nVZxkFEaWbbvKwFMnOuHZAAj6ceBKrZZsh1t2dH7tvA== X-Received: by 2002:a7b:c4c4:: with SMTP id g4mr11223073wmk.93.1637099277010; Tue, 16 Nov 2021 13:47:57 -0800 (PST) Received: from elver.google.com ([2a00:79e0:15:13:ee27:74df:199e:beab]) by smtp.gmail.com with ESMTPSA id v7sm18040254wrq.25.2021.11.16.13.47.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Nov 2021 13:47:56 -0800 (PST) Date: Tue, 16 Nov 2021 22:47:50 +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: <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-Server: rspam07 X-Rspamd-Queue-Id: ABDEB801AB0C X-Stat-Signature: a3i1pq6zett3yzj9mqgmw1khojqbxre1 Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=qEwFgJ4U; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf06.hostedemail.com: domain of elver@google.com designates 209.85.128.45 as permitted sender) smtp.mailfrom=elver@google.com X-HE-Tag: 1637099278-259041 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 Tue, Nov 16, 2021 at 12:59PM -0800, Shakeel Butt wrote: > On Tue, Nov 16, 2021 at 12:48 PM Mina Almasry wrote: [...] > > > 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). > > > > > > > Thank you very much for the detailed response. I can add READ_ONCE() > > at the no-lock read site, that is no issue. > > > > However, for the writes that happen while holding the lock, the write > > is like so: > > + h_cg->nodeinfo[page_to_nid(page)]->usage[idx] += nr_pages; > > > > And like so: > > + h_cg->nodeinfo[page_to_nid(page)]->usage[idx] -= nr_pages; > > > > I.e. they are increments/decrements. Sorry if I missed it but I can't > > find an INC_ONCE(), and it seems wrong to me to do something like: > > > > + WRITE_ONCE(h_cg->nodeinfo[page_to_nid(page)]->usage[idx], > > + > > h_cg->nodeinfo[page_to_nid(page)] + nr_pages); >From what I gather there are no concurrent writers, right? WRITE_ONCE(a, a + X) is perfectly fine. What it says is that you can have concurrent readers here, but no concurrent writers (and KCSAN will still check that). Maybe we need a more convenient macro for this idiom one day.. Though I think for something like h_cg->nodeinfo[page_to_nid(page)]->usage[idx] += nr_pages; it seems there wants to be an temporary long* so that you could write WRITE_ONCE(*usage, *usage + nr_pages) or something. > > I know we're holding a lock anyway so there is no race, but to the > > casual reader this looks wrong as there is a race between the fetch of > > the value and the WRITE_ONCE(). What to do here? Seems to me the most > > reasonable thing to do is just READ_ONCE() and leave the write plain? > > > > > > How about atomic_long_t? That would work of course; if this is very hot path code it might be excessive if you don't have concurrent writers. Looking at the patch in more detail, the counter is a stat counter that can be read from a stat file, correct? Hypothetically, what would happen if the reader of 'usage' reads approximate values? If the answer is "nothing", then this could classify as an entirely "benign" data race and you could only use data_race() on the reader and leave the writers unmarked using normal +=/-=. Check if it fits "Data-Racy Reads for Approximate Diagnostics" [1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/memory-model/Documentation/access-marking.txt#n74