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=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=ham 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 A5CF3C433DB for ; Fri, 19 Mar 2021 18:20:48 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5308C61981 for ; Fri, 19 Mar 2021 18:20:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5308C61981 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 C242B6B0073; Fri, 19 Mar 2021 14:20:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BFABD6B0078; Fri, 19 Mar 2021 14:20:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A65AD6B007B; Fri, 19 Mar 2021 14:20:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0050.hostedemail.com [216.40.44.50]) by kanga.kvack.org (Postfix) with ESMTP id 84BE56B0073 for ; Fri, 19 Mar 2021 14:20:47 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 428788249980 for ; Fri, 19 Mar 2021 18:20:47 +0000 (UTC) X-FDA: 77937439734.05.BF45F70 Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) by imf27.hostedemail.com (Postfix) with ESMTP id C87F680192D5 for ; Fri, 19 Mar 2021 18:20:44 +0000 (UTC) Received: by mail-lf1-f53.google.com with SMTP id x28so11407524lfu.6 for ; Fri, 19 Mar 2021 11:20:44 -0700 (PDT) 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; bh=Tz1A3bSsj9rkyv4dGud826X0c/D6fAibU1rcdURNzxA=; b=e2CW/V/+A9LjGE8tySVsj6k7xbzbHKqLyuKGKMK14Suap+1kHtnu0Twb29yzjixeCR 85VI6Nz3sh840fxOdohZP6w7DS0gzzeCTGkigI2SWWKv/bidA3QlMY86meg/eNVLQQqD CPNEfQkn+yHZ17r5meKdKTJoEJ5Y6uAqGklYv0e0b12pBI0KrVkyLNI2ovG9LxbRn4d5 9R5rMzUfzFAacvaYyrdjTyMgng185K5kKsbNOJnG3au2hZjCuMvBJmeJI5mvEsY2tWDb lscjrFmxER3OOEuixRLQxKZ1cRz5Y060Z1wJmfXryCJUKF9b+ujDQhqsAVdBcprNsTIG k/YA== 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; bh=Tz1A3bSsj9rkyv4dGud826X0c/D6fAibU1rcdURNzxA=; b=nL2PFW5lSFG+0DiJYz3bJPvSmXKlSx679KkS7CQyVuhsaUZQqNrZDVkIC5ysyhnkla 5jC5Mbru8P0v45SPx4L0uDTbwpQfX+HcFFq4mMaMFM1K1TZL+1mntwNptFldQh4osPOg lGlb4nPKAmz1Xkd2G/9ZB2VBvAwTyMH3jC0teM4AlwNb0M/X1n5pN8Ek6wZ4bp1h0UOM dF6gZwT4hnO535iWPfsFbQGO78j4CtbeIuael9TSpErSiP8QbcNPFcFYCguaWdSHU5OI WfjY3Z+z9Mc1Hvo03RyQ/rpPy5wmWqW1xFz2LaltUuvHej3ZM0yICiUJa96zrp1ymVCi BaAA== X-Gm-Message-State: AOAM530HoDt1fYLcSJAyfn2CwRB4Auouomi+12V1jsF+OHQ/A5dFGFib 39gAk7QOcsK7mOV177KPUSkNWnSauOUtsS2Ohd0KXA== X-Google-Smtp-Source: ABdhPJw5auxNmMxHYh2Wy7/EwwcS9N1z0P4t0mjnwQsWaQuA2PVf+Wn/YVktqSTXqkuRF8PTIKRehoMVUEJWGVxRcKM= X-Received: by 2002:a19:f50e:: with SMTP id j14mr1540575lfb.299.1616178037655; Fri, 19 Mar 2021 11:20:37 -0700 (PDT) MIME-Version: 1.0 References: <20210319054944.50048-1-hannes@cmpxchg.org> <20210319054944.50048-2-hannes@cmpxchg.org> In-Reply-To: From: Shakeel Butt Date: Fri, 19 Mar 2021 11:20:26 -0700 Message-ID: Subject: Re: [PATCH 2/2] mm: memcontrol: deprecate swapaccounting=0 mode To: Johannes Weiner Cc: Andrew Morton , Hugh Dickins , Michal Hocko , Linux MM , Cgroups , LKML , Kernel Team Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: uknfw7cds7drsr1j5qjg9uk36dewfabp X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: C87F680192D5 Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf27; identity=mailfrom; envelope-from=""; helo=mail-lf1-f53.google.com; client-ip=209.85.167.53 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616178044-162894 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, Mar 19, 2021 at 10:36 AM Johannes Weiner wrote: > > On Fri, Mar 19, 2021 at 06:49:55AM -0700, Shakeel Butt wrote: > > On Thu, Mar 18, 2021 at 10:49 PM Johannes Weiner wrote: > > > > > > The swapaccounting= commandline option already does very little > > > today. To close a trivial containment failure case, the swap ownership > > > tracking part of the swap controller has recently become mandatory > > > (see commit 2d1c498072de ("mm: memcontrol: make swap tracking an > > > integral part of memory control") for details), which makes up the > > > majority of the work during swapout, swapin, and the swap slot map. > > > > > > The only thing left under this flag is the page_counter operations and > > > the visibility of the swap control files in the first place, which are > > > rather meager savings. There also aren't many scenarios, if any, where > > > controlling the memory of a cgroup while allowing it unlimited access > > > to a global swap space is a workable resource isolation stragegy. > > > > *strategy > > Will fix :) > > > > On the other hand, there have been several bugs and confusion around > > > the many possible swap controller states (cgroup1 vs cgroup2 behavior, > > > memory accounting without swap accounting, memcg runtime disabled). > > > > > > This puts the maintenance overhead of retaining the toggle above its > > > practical benefits. Deprecate it. > > > > > > Suggested-by: Shakeel Butt > > > Signed-off-by: Johannes Weiner > > [...] > > > > > > static int __init setup_swap_account(char *s) > > > { > > > - if (!strcmp(s, "1")) > > > - cgroup_memory_noswap = false; > > > - else if (!strcmp(s, "0")) > > > - cgroup_memory_noswap = true; > > > - return 1; > > > + pr_warn_once("The swapaccount= commandline option is deprecated. " > > > + "Please report your usecase to linux-mm@kvack.org if you " > > > + "depend on this functionality.\n"); > > > + return 0; > > > > What's the difference between returning 0 or 1 here? > > It signals whether the parameter is "recognized" by the kernel as a > valid thing to pass, or whether it's unknown. If it's unknown, it'll > be passed on to the init system (which ignores it), so this resembles > the behavior we'll have when we remove the __setup in the future. > > If somebody can make an argument why we should go with one over the > other, I'll happily go with that. > > > > __setup("swapaccount=", setup_swap_account); > > > > > > @@ -7291,27 +7287,13 @@ static struct cftype memsw_files[] = { > > > { }, /* terminate */ > > > }; > > > > > > -/* > > > - * If mem_cgroup_swap_init() is implemented as a subsys_initcall() > > > - * instead of a core_initcall(), this could mean cgroup_memory_noswap still > > > - * remains set to false even when memcg is disabled via "cgroup_disable=memory" > > > - * boot parameter. This may result in premature OOPS inside > > > - * mem_cgroup_get_nr_swap_pages() function in corner cases. > > > - */ > > > static int __init mem_cgroup_swap_init(void) > > > { > > > - /* No memory control -> no swap control */ > > > - if (mem_cgroup_disabled()) > > > - cgroup_memory_noswap = true; > > > - > > > - if (cgroup_memory_noswap) > > > - return 0; > > > - > > > > Do we need a mem_cgroup_disabled() check here? > > cgroup_add_cftypes() implies this check from the cgroup side and will > just do nothing and return success. So we don't need it now. > > But it is something we'd have to remember to add if we do add more > code to this function later on. > > Either way works for me. I can add it if people think it's better. > I am fine with either way. For this patch: Reviewed-by: Shakeel Butt