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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 98B01C433C1 for ; Fri, 19 Mar 2021 17:43:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2142960238 for ; Fri, 19 Mar 2021 17:43:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2142960238 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8CBE16B0075; Fri, 19 Mar 2021 13:43:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A4126B0078; Fri, 19 Mar 2021 13:43:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 76B2C6B007B; Fri, 19 Mar 2021 13:43:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0022.hostedemail.com [216.40.44.22]) by kanga.kvack.org (Postfix) with ESMTP id 5DCC36B0075 for ; Fri, 19 Mar 2021 13:43:52 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 22D1C1802A373 for ; Fri, 19 Mar 2021 17:43:52 +0000 (UTC) X-FDA: 77937346704.01.630F755 Received: from mail-qt1-f180.google.com (mail-qt1-f180.google.com [209.85.160.180]) by imf15.hostedemail.com (Postfix) with ESMTP id 42430A000293 for ; Fri, 19 Mar 2021 17:43:50 +0000 (UTC) Received: by mail-qt1-f180.google.com with SMTP id a11so7383249qto.2 for ; Fri, 19 Mar 2021 10:43:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=tfsvGpLK+CuDrVdmeV4WHbj8Vk5nJ4Roy68oEY4gfyA=; b=WTDYAcQY8XXEtUVTWUYj1UZ9W4ojknZfX8LZVQvAAN+HkIuawO0/AttbRhKWjPhcCC GJSpxDkjmrAIKSM3t+ZMvrAIiU8XS5eUyfJMf3pjpDY4fNEw9o1/6DipQNLjxUjLYkr/ Y/x0nBvtvz8+0b3N/n1DzXM3jPy+2DXKL7zO/jDkb8uFzpMo32zJc4twjzrEt8VdXu0Y qHwMWr+03PLl/retLPKsVjOxWpAhEEQGQWd7LiZGnEU6QWGDez8ZSzCRTW+ovn46V8Sp QDv/j+Ip75kO9C8B63mcq6JqS/ZvfLW0X1H8uvyZXvCAb3EYJImlDVMkEgQfvGgt5m3r YxqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tfsvGpLK+CuDrVdmeV4WHbj8Vk5nJ4Roy68oEY4gfyA=; b=RkMH3XDq7xXAPRo/2kdBelKxwz/W4LT0vdRiPWQn/fR4jbgVGz324xz8euz8ZS5pSC 9MMySJq22SBhUEhXdzTmaa1bYaul5ZdushqB+4w9oR+n0hLsB8OY7lXJq9Vxzxw+2Npj zHh2irSJza1QjChbbeNKhywbOE1B7vdKrRGQ6AMDpP44SjWYbe0ilDxdBjHAD3RQm8Vf +j62RV/wMtUtZrOIxVOyOLA5olfo9o7wu0XXbr/IoMUGnfXOLuZ7KMeYo6di/Hgrjlt2 f54hX5HW2Io6sAgnMjq3xqPwMdmypDcMzLdG0eiDpozhclc0aUdRIZXYJf4+a7lFwgUQ Ph6A== X-Gm-Message-State: AOAM532iYYa6mP6k+/mVw0ZtKjW9Q4L30ts9WRj8Vmf1CRpUxaTJROFU nnGta4rf+LhgqtYNlf2D+BaftVP2GzUFwA== X-Google-Smtp-Source: ABdhPJztEQ6+2EmkeqiPV4c/LJI6bVpVMS3Y2j1tG0aYG7IDJ7uN/f76YXpplQssD8vVmn+tsRisJA== X-Received: by 2002:ac8:1009:: with SMTP id z9mr9201531qti.128.1616175378314; Fri, 19 Mar 2021 10:36:18 -0700 (PDT) Received: from localhost ([2620:10d:c091:480::1:18e3]) by smtp.gmail.com with ESMTPSA id h12sm5128779qko.29.2021.03.19.10.36.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Mar 2021 10:36:17 -0700 (PDT) Date: Fri, 19 Mar 2021 13:36:15 -0400 From: Johannes Weiner To: Shakeel Butt Cc: Andrew Morton , Hugh Dickins , Michal Hocko , Linux MM , Cgroups , LKML , Kernel Team Subject: Re: [PATCH 2/2] mm: memcontrol: deprecate swapaccounting=0 mode Message-ID: References: <20210319054944.50048-1-hannes@cmpxchg.org> <20210319054944.50048-2-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: t9fi9tqt16dssy4djxim99yyuu7nkehr X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 42430A000293 Received-SPF: none (cmpxchg.org>: No applicable sender policy available) receiver=imf15; identity=mailfrom; envelope-from=""; helo=mail-qt1-f180.google.com; client-ip=209.85.160.180 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616175830-210707 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 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. > > > WARN_ON(cgroup_add_dfl_cftypes(&memory_cgrp_subsys, swap_files)); > > WARN_ON(cgroup_add_legacy_cftypes(&memory_cgrp_subsys, memsw_files)); > > > > return 0; > > } > > -core_initcall(mem_cgroup_swap_init); > > +subsys_initcall(mem_cgroup_swap_init); > > > > #endif /* CONFIG_MEMCG_SWAP */ > > -- > > 2.30.1 > >