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 1BF1EC433F5 for ; Mon, 1 Nov 2021 14:31:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A2DC5610A0 for ; Mon, 1 Nov 2021 14:31:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A2DC5610A0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=soleen.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 0F02D940016; Mon, 1 Nov 2021 10:31:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0A07794000F; Mon, 1 Nov 2021 10:31:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EA9E4940016; Mon, 1 Nov 2021 10:31:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0132.hostedemail.com [216.40.44.132]) by kanga.kvack.org (Postfix) with ESMTP id D7EFA94000F for ; Mon, 1 Nov 2021 10:31:18 -0400 (EDT) Received: from smtpin11.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 9296F183F1382 for ; Mon, 1 Nov 2021 14:31:18 +0000 (UTC) X-FDA: 78760599036.11.0E5EAD1 Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by imf27.hostedemail.com (Postfix) with ESMTP id 2741B70000A9 for ; Mon, 1 Nov 2021 14:31:18 +0000 (UTC) Received: by mail-lj1-f182.google.com with SMTP id 17so26582747ljq.0 for ; Mon, 01 Nov 2021 07:31:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=soleen.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=kJLhZg8fgyUrWR28/t5VvMjVzdgGdzLZ7WEsafpviKg=; b=IuCM/zkA9w+zgj7vM1G+9lF2cqdeAOg3vv9y1Byl7DrCiV23k+GnQ4JGGsr7QyATI+ 1StNazJxafHRYdl260iMQFo4AmqjwPrsOVEsobM+Knsyjgd2xz83jHHujz9TzyOA8wji V73MWziifVDeKIR1KLYWw6A1KkjeD/laXqi0j0nHmTXPqILXVUIcV1nMJ7V+TvaaF7Kf MBzaKrCwZfGd1Xi8JjBRFwmOns5L79zNRXTGNHDtTZDW78CsruZn8e9uVFnOA/UWdbUS 8iidVKw4qfj2b6ds/SwYhli/fuofO8cLzOOEDshbVNx5goTKMqWCYPNEQO1RvjSaSpMu vu+A== 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; bh=kJLhZg8fgyUrWR28/t5VvMjVzdgGdzLZ7WEsafpviKg=; b=L6QlaMuhYBpVxsfIQ95roiDgTm2WaL/ZqJbRO1XKS1UIZ6wc1beMQNxCoYUPuYHV80 KsAp9fJrJhW6sXBxNDDOL+d2Gpy8k95sOr4eEC4LKcimuv6zFfTrrgoYx50CJZAXSMHh tCmJGZOp3mkyyv/wbA1ZN3PMkRzc2iG7fxBPPJxsnqPhY/Szkw8TVkCY05xQF+dkp4M2 T7ql2TTrUp6O0LcxPusM7A8giOKcKBET01B9qPqGqBTqm9f3A4dPogl6deIq0Rp0oRni U1BiL9Vh7kCEBFgquul2xuieWnWXPrzP109DOxP/nVX88Uzz2GEtdeBGyFtM6nu4u6IT vLhA== X-Gm-Message-State: AOAM530cavSZgICASMOjYRnUjWEimIkW7Ve7fZ4Ah1ns8aLO5XP64LBN 3FZqMts2E4WtEjxacMqjsFPuYc+JSyfxhbG+eF1LLQ== X-Google-Smtp-Source: ABdhPJxoZb9NdURfvUT8GcH1I9TuGJTGoo8r4lq3ScBZPmN1Z72mGEA5Knv8Ic2/fcV/g5pp3DguNSmkdxt3rLfc/jU= X-Received: by 2002:a05:651c:114b:: with SMTP id h11mr5220050ljo.41.1635777076656; Mon, 01 Nov 2021 07:31:16 -0700 (PDT) MIME-Version: 1.0 References: <20211026173822.502506-1-pasha.tatashin@soleen.com> <20211026173822.502506-4-pasha.tatashin@soleen.com> <7b131cb1-68d8-6746-f9c1-2b01d4838869@nvidia.com> <19d16b40-355f-3f79-dcba-e1d8d2216d33@nvidia.com> <27b7177c-71be-9ff2-716e-caaa5035d451@nvidia.com> In-Reply-To: <27b7177c-71be-9ff2-716e-caaa5035d451@nvidia.com> From: Pasha Tatashin Date: Mon, 1 Nov 2021 10:30:41 -0400 Message-ID: Subject: Re: [RFC 3/8] mm: Avoid using set_page_count() in set_page_recounted() To: John Hubbard Cc: LKML , linux-mm , linux-m68k@lists.linux-m68k.org, Anshuman Khandual , Matthew Wilcox , Andrew Morton , william.kucharski@oracle.com, Mike Kravetz , Vlastimil Babka , Geert Uytterhoeven , schmitzmic@gmail.com, Steven Rostedt , Ingo Molnar , Johannes Weiner , Roman Gushchin , Muchun Song , weixugc@google.com, Greg Thelen Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 2741B70000A9 X-Stat-Signature: r4g9aagxwnpmc7xfejpb4tkzp1wqsyzw Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=soleen.com header.s=google header.b="IuCM/zkA"; spf=pass (imf27.hostedemail.com: domain of pasha.tatashin@soleen.com designates 209.85.208.182 as permitted sender) smtp.mailfrom=pasha.tatashin@soleen.com; dmarc=none X-HE-Tag: 1635777078-149780 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 Wed, Oct 27, 2021 at 9:35 PM John Hubbard wrote: > > On 10/27/21 18:20, John Hubbard wrote: > >>> But it's still not good to have this function name doing something completely > >>> different than its name indicates. > >> > >> I see, I can rename it to: 'set_page_recounted/get_page_recounted' ? > >> > > > > What? No, that's not where I was going at all. The function is already > > named set_page_refcounted(), and one of the problems I see is that your > > changes turn it into something that most certainly does not > > set_page_refounted(). Instead, this patch *increments* the refcount. > > That is not the same thing. > > > > And then it uses a .config-sensitive assertion to "prevent" problems. > > And by that I mean, the wording throughout this series seems to equate > > VM_BUG_ON_PAGE() assertions with real assertions. They are only active, > > however, in CONFIG_DEBUG_VM configurations, and provide no protection at > > all for normal (most distros) users. That's something that the wording, > > comments, and even design should be tweaked to account for. > > ...and to clarify a bit more, maybe this also helps: > > These patches are attempting to improve debugging, and that is fine, as They are attempting to catch potentioal race conditions where _refcount is changed between the time we verified what it was and we set it to something else. They also attempt to prevent overflows and underflows bugs which are not all tested today, but can be tested with this patch set at least on kernels where DEBUG_VM is enabled. > far as debugging goes. However, a point that seems to be slightly > misunderstood is: incrementing a bad refcount value is not actually any > better than overwriting it, from a recovery point of view. Maybe (?) > it's better from a debugging point of view. It is better for debugging as well: if one is tracing the page _refcount history, knowing that the _refcount can only be incremented/decremented/frozen/unfrozen provides a contiguous history of refcount that can be tracked. In case when we set refcount in some places as we do today, the contigous history is lost, as we do not know the actual _refcount value at the time of the set operation. > > That's because the problem occurred before this code, and its debug-only > assertions, ran. Once here, the code cannot actually recover: there is > no automatic way to recover from a refcount that it 1, -1, 2, or 706, > when it was supposed to be zero. Incrementing it is, again, not really > necessarily better than setting: setting it might actually make the > broken system appear to run--and in some cases, even avoid symptoms. > Whereas incrementing doesn't cover anything up. The only thing you can > really does is just panic() or BUG(), really. This is what my patch series attempt to do, I chose to use VM_BUG() instead of BUG() because this is VM code, and avoid potential performance regressions for those who chose performance over possible security implications. > > Don't get me wrong, I don't want bugs covered up. But the claim that > incrementing is somehow better deserves some actual thinking about it. I think it does, I described my points above, if you still disagree please let me know. Thank you for providing your thoughts on this RFC, I will send out a new version, and we can continue discussion in the new thread. Pasha