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 1EA41C433F5 for ; Sat, 13 Nov 2021 14:48:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A745860F55 for ; Sat, 13 Nov 2021 14:48:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A745860F55 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 E0A5D6B0075; Sat, 13 Nov 2021 09:48:49 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DBA626B0078; Sat, 13 Nov 2021 09:48:49 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C81136B007B; Sat, 13 Nov 2021 09:48:49 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0028.hostedemail.com [216.40.44.28]) by kanga.kvack.org (Postfix) with ESMTP id B5C2A6B0075 for ; Sat, 13 Nov 2021 09:48:49 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 6FF14181CE299 for ; Sat, 13 Nov 2021 14:48:49 +0000 (UTC) X-FDA: 78804188778.12.7291F1D Received: from mail-il1-f169.google.com (mail-il1-f169.google.com [209.85.166.169]) by imf12.hostedemail.com (Postfix) with ESMTP id E8D0210000A3 for ; Sat, 13 Nov 2021 14:48:48 +0000 (UTC) Received: by mail-il1-f169.google.com with SMTP id j28so11910389ila.1 for ; Sat, 13 Nov 2021 06:48:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=mT2wqWAbGXye/oOAJu7JszLtZ2k9E4RwfAiqc/hqR6s=; b=bKzsmtg8WGNurTIyAuveG62Gvo6HVFivXIQTq6AvIcD9voJK5p0+nHRx/2HS1sY9EN PCJR4whf6loIXngAWLEVCfYaeBxdZvQ2jMblyOnkITsjzjvJ3cRyDlvo4nGbdn/8L6Mt TGQ+aW0Ss9cDpJk80JURiWkK4MGgrpzQwkpPihtU6EpeCsgcqS6wPAs05a8Bhq6tYaHp AUX8xJNNAEZpEd9Bl2F/skHCwo1ZVt+qmtbTIxxiN6uKpRBKUHAjBedsjujxbFF/PHTJ ijjaOV6KzQ/0FOtmDbu3CxVd44LQewg1ADvdNuwglpuZi21rhMwD1EqG4kVujZOy2bLV C8kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=mT2wqWAbGXye/oOAJu7JszLtZ2k9E4RwfAiqc/hqR6s=; b=YQgMF0Q7Ws7NtCeB6yTVvnQ7cCcK8Y1kihGeu1Y1NbxmSZigT06AYwaPsIrIR0ZfGz KBeV9cue5DISOw2Kqivv8FHusvVoibxG75WGyboQI0iLPYEFZQ61H1oY0KSn39QIUENS LhXtG9UBVtel/uUAuhH5IvQTbD7cuz+RNbHfYjt3sy8oDe0SFXZ88u+wKj7mugrO8gud ptWR6oJRVJXgENLAkGE3NYwhcNRXfPCDWiOsB4yueMv9mWIVI4JdM8gAZ3bpYSN8ZBvo dmUSMK67Uxd8bLP65L3khFxl+RQd871qixhxudFsR3v2DVOATm/a3D95qDOvKPtajU1g 11Uw== X-Gm-Message-State: AOAM532yI0HHR+3ZddqyLsaJimRvxI8B4zcO5m9QQnPI+nxZ0Zx2oFjR i+RCy+gKoxoefzSLi24d4diUKVH9SCpyvgARcsNTVg== X-Google-Smtp-Source: ABdhPJxMgSs399iXe+1qXlJc43lDzixP1TRp4F/RUxvdwOET1jBQYUIjsUjent6vHHJLGucIW7LQ1VFRsjHX10geASo= X-Received: by 2002:a92:c5a1:: with SMTP id r1mr13480189ilt.228.1636814928145; Sat, 13 Nov 2021 06:48:48 -0800 (PST) MIME-Version: 1.0 References: <20211111015037.4092956-1-almasrymina@google.com> In-Reply-To: From: Mina Almasry Date: Sat, 13 Nov 2021 06:48:37 -0800 Message-ID: Subject: Re: [PATCH v6] hugetlb: Add hugetlb.*.numa_stat file To: Muchun Song Cc: Mike Kravetz , Andrew Morton , Shuah Khan , Miaohe Lin , Oscar Salvador , Michal Hocko , David Rientjes , Shakeel Butt , Jue Wang , Yang Yao , Joanna Li , Cannon Matthews , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: E8D0210000A3 X-Stat-Signature: hfmhdrei9n5u6txjnyzmmkag14gq9a8e Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=bKzsmtg8; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf12.hostedemail.com: domain of almasrymina@google.com designates 209.85.166.169 as permitted sender) smtp.mailfrom=almasrymina@google.com X-HE-Tag: 1636814928-954508 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 Fri, Nov 12, 2021 at 6:45 PM Muchun Song wrot= e: > > On Sat, Nov 13, 2021 at 7:36 AM Mike Kravetz wr= ote: > > > > Subject: Re: [PATCH v6] hugetlb: Add hugetlb.*.numa_stat file > > > > To: Muchun Song , Mina Almasry > > > > Cc: Andrew Morton , Shuah Khan , Miaohe Lin , Oscar Salvador , Michal Hocko , David Rientjes , Shakeel Butt , Jue Wang , Yang Y= ao , Joanna Li , Cannon Matthews , Linux Memory Management List = , LKML > > > > Bcc: > > > > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D# Don't remove this line #=3D-=3D-= =3D-=3D-=3D-=3D-=3D-=3D-=3D- > > > > On 11/10/21 6:36 PM, Muchun Song wrote: > > > > > On Thu, Nov 11, 2021 at 9:50 AM Mina Almasry = wrote: > > > > >> > > > > >> +struct hugetlb_cgroup_per_node { > > > > >> + /* hugetlb usage in pages over all hstates. */ > > > > >> + atomic_long_t usage[HUGE_MAX_HSTATE]; > > > > > > > > > > Why do you use atomic? IIUC, 'usage' is always > > > > > increased/decreased under hugetlb_lock except > > > > > hugetlb_cgroup_read_numa_stat() which is always > > > > > reading it. So I think WRITE_ONCE/READ_ONCE > > > > > is enough. > > > > > > > > Thanks for continuing to work this, I was traveling and unable to > > > > comment. > > Have a good time. > > > > > > > > > Unless I am missing something, I do not see a reason for WRITE_ONCE/REA= D_ONCE > > Because __hugetlb_cgroup_commit_charge and > hugetlb_cgroup_read_numa_stat can run parallely, > which meets the definition of data race. I believe > KCSAN could report this race. I'm not strongly > suggest using WRITE/READ_ONCE() here. But > in theory it should be like this. Right? > My understanding is that the (only) potential problem here is read_numa_stat() reading an intermediate garbage value while commit_charge() is happening concurrently. This will only happen on archs where the writes to an unsigned long aren't atomic. On archs where writes to an unsigned long are atomic, there is no race, because read_numa_stat() will only ever read the value before the concurrent write or after the concurrent write, both of which are valid. To cater to archs where the writes to unsigned long aren't atomic, we need to use an atomic data type. I'm not too familiar but my understanding from reading the documentation is that WRITE_ONCE/READ_ONCE don't contribute anything meaningful here: /* * Prevent the compiler from merging or refetching reads or writes. The * compiler is also forbidden from reordering successive instances of * READ_ONCE and WRITE_ONCE, but only when the compiler is aware of some * particular ordering. One way to make the compiler aware of ordering is to * put the two invocations of READ_ONCE or WRITE_ONCE in different C * statements. ... I can't see a reason why we care about the compiler merging or refetching reads or writes here. As far as I can tell the problem is atomicy of the write. > Thanks. > > > > > and would suggest going back to the way this code was in v5. > > > > -- > > > > Mike Kravetz > >