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=-5.9 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_DBL_ABUSE_MALW,USER_IN_DEF_DKIM_WL 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 243CAC33C9E for ; Thu, 30 Jan 2020 12:35:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DA1D220702 for ; Thu, 30 Jan 2020 12:35:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="YUsBAZ71" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DA1D220702 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 5EFEF6B0345; Thu, 30 Jan 2020 07:35:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 59FAF6B0346; Thu, 30 Jan 2020 07:35:31 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 490056B0347; Thu, 30 Jan 2020 07:35:31 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0135.hostedemail.com [216.40.44.135]) by kanga.kvack.org (Postfix) with ESMTP id 32EDB6B0345 for ; Thu, 30 Jan 2020 07:35:31 -0500 (EST) Received: from smtpin11.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id D6760181AEF10 for ; Thu, 30 Jan 2020 12:35:30 +0000 (UTC) X-FDA: 76434246420.11.ball82_6a4c16f4efb2f X-HE-Tag: ball82_6a4c16f4efb2f X-Filterd-Recvd-Size: 4755 Received: from mail-ot1-f65.google.com (mail-ot1-f65.google.com [209.85.210.65]) by imf15.hostedemail.com (Postfix) with ESMTP for ; Thu, 30 Jan 2020 12:35:30 +0000 (UTC) Received: by mail-ot1-f65.google.com with SMTP id i6so2937262otr.7 for ; Thu, 30 Jan 2020 04:35:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=JChikV2eDuxHeLtfwZM5oT2t0AhKhE72FILijwgC87M=; b=YUsBAZ71sBC1TVR1hKRZFi1hmpwB9zhsnlhdWL3ICTqBooSAMFFMXZbSBFSKgdRiRs 2xsfHXBBEj7yGw0MZ0CGqRBIfoJlZVnnBAVwixr97b3sdPC7zE3bdgnIZiMsmjnl4kta 7otz7uWSguPylhanlVvJmGvb9wkkGJn83byq0JK2qzqW2QxovIam5kWVILEYi++eLZgC 3Bjd4uumjplegcu8AiADFuzxLn05QfbOtY/PhGncK+3qM6wwUm/c/CpY0o8suWm3UiMl 3zkdHX1df6YOWpCuAAolzgzgxy3/itRkFubaSIbf2WSQVtTZWmGp5EfW8hnz0rWRvHCY /HnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=JChikV2eDuxHeLtfwZM5oT2t0AhKhE72FILijwgC87M=; b=KZYB2lsrJN6CT9LemKs4dn8TQjlh1MFXD96rcZXO0dE07KGSSvxNi9P8+4a9c7jhkW rvNfBQFJ8epHcMQuuPscWd1og4ZqYB7jsHWFH1gNNzMRB43ZSfzZrAo22QkXDrETku3a SVh/YDoRCU2+co9kqXzC5w5zxbp6snLmuO8ZMOZ4cryOmUx39Zth+dH2ORBTpUA7h8yn mm2AfMR4+5hKrKpsxVC8Dsf6FCcu+JjLtaWqY+Y4S1UPX3gEF5VkvuhZT9N+dy/RZ3ZF yvoN4pzokUTkENEPkkNOy2z1tXf+lZzpUo9PkNntf5SpVjT77w4lx/561damU1Va50mY PaZg== X-Gm-Message-State: APjAAAVKKFBysuOKvoOr6e2+4M4plBfQq0S1Q7D2IH0cXNXfQmuCkOZD /bBOQ6Wx/p13d/OGLJCdE3BD+kKCi4fcOZ0jnOKwvA== X-Google-Smtp-Source: APXvYqxlMiItmIGzWpPjTehVgqmQudgnyMa5XuMeysm+8SBqlIDbsQDGlMs2wwGLilvOTimeqi4YyHBwdOczbyzWLKQ= X-Received: by 2002:a9d:4711:: with SMTP id a17mr3422842otf.233.1580387729374; Thu, 30 Jan 2020 04:35:29 -0800 (PST) MIME-Version: 1.0 References: <20200130042011.GI6615@bombadil.infradead.org> <1135BD67-4CCB-4700-8150-44E7E323D385@lca.pw> In-Reply-To: <1135BD67-4CCB-4700-8150-44E7E323D385@lca.pw> From: Marco Elver Date: Thu, 30 Jan 2020 13:35:18 +0100 Message-ID: Subject: Re: [PATCH] mm/util: fix a data race in __vm_enough_memory() To: Qian Cai Cc: Matthew Wilcox , Andrew Morton , dennis@kernel.org, tj@kernel.org, Christoph Lameter , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" 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 Thu, 30 Jan 2020 at 12:50, Qian Cai wrote: > > > On Jan 29, 2020, at 11:20 PM, Matthew Wilcox wrot= e: > > > > I'm really not a fan of exposing the internals of a percpu_counter outs= ide > > the percpu_counter.h file. Why shouldn't this be fixed by putting the > > READ_ONCE() inside percpu_counter_read()? > > It is because not all places suffer from a data race. For example, in __w= b_update_bandwidth(), it was protected by a lock. I was a bit worry about b= lindly adding READ_ONCE() inside percpu_counter_read() might has unexpected= side-effect. For example, it is unnecessary to have READ_ONCE() for a vola= tile variable. So, I thought just to keep the change minimal with a trade o= ff by exposing a bit internal details as you mentioned. > > However, I had also copied the percpu maintainers to see if they have any= preferences? I would not add READ_ONCE to percpu_counter_read(), given the writes (increments) are not atomic either, so not much is gained. Notice that this is inside a WARN_ONCE, so you may argue that a data race here doesn't matter to the correct behaviour of the system (except if you have panic_on_warn on). For the warning to trigger, vm_committed_as must decrease. Assume that a data race (assuming bad compiler optimizations) can somehow accomplish this, then the load or write must cause a transient value to somehow be less than a stable value. My hypothesis is this is very unlikely. Given the fact this is a WARN_ONCE, and the fact that a transient decrease in the value is unlikely, you may consider 'VM_WARN_ONCE(data_race(percpu_counter_read(&vm_committed_as)) < ...)'. That way you won't modify percpu_counter_read and still catch unintended races elsewhere. [ Note that the 'data_race()' macro is still only in -next, -tip, and -rcu.= ] Thanks, -- Marco