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=-8.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS 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 188D9C433DB for ; Wed, 17 Feb 2021 20:53:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7DDB064E2E for ; Wed, 17 Feb 2021 20:53:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7DDB064E2E 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 EF7306B0006; Wed, 17 Feb 2021 15:53:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id EA84E6B006C; Wed, 17 Feb 2021 15:53:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D700D6B006E; Wed, 17 Feb 2021 15:53:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0200.hostedemail.com [216.40.44.200]) by kanga.kvack.org (Postfix) with ESMTP id C18EA6B0006 for ; Wed, 17 Feb 2021 15:53:02 -0500 (EST) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 91CE58012B26 for ; Wed, 17 Feb 2021 20:53:02 +0000 (UTC) X-FDA: 77828959404.19.D79B940 Received: from mail-qv1-f45.google.com (mail-qv1-f45.google.com [209.85.219.45]) by imf12.hostedemail.com (Postfix) with ESMTP id 8D312D7 for ; Wed, 17 Feb 2021 20:52:58 +0000 (UTC) Received: by mail-qv1-f45.google.com with SMTP id p12so6969129qvv.5 for ; Wed, 17 Feb 2021 12:53:01 -0800 (PST) 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:content-transfer-encoding:in-reply-to; bh=/nI5rj9bvE6w+kmHwM58CvDkIfatHnQ+1wEcvE6t0qM=; b=LRegk7bSkLpAy9xvp3N/hncxZhzP8Xrlr1UaRRu2N5qWlQWPo52bjTvGA8VIv4kbEF lShAhk2O99mAa5clhSXPVAdxFkq1AKCbtlnxLGkUORLjGHudJIhkQeT0LGFRVqc5czce olwaIu6YmleDNMi6Guf87kfg0jryOdIC22S8AdK8aVVunaMZgAICA1OTr0M2XS8qZzcJ HJ/Ene0AyDh2JrCED8TVxmid8NiihzpDXbpRyzZXTsibJGK4tUGn/lpepfII6xFrarI0 OrxhWiNJ0YIv4ARellkRePr3xqNExRIieZ/h2vVX/HYHYfLPkwImW+FqC2dlsRHhIy7W UcUQ== 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:content-transfer-encoding :in-reply-to; bh=/nI5rj9bvE6w+kmHwM58CvDkIfatHnQ+1wEcvE6t0qM=; b=AmvVIaavldl/lNH4ftjDTWSluH+josMv2h1NWrWZwBaOpdNKTf0ujGUhhBOj4nt3Rl EOfzegxf4FjXOsnEpcVesUwj8maZejE+h0Qi0PWvFcaPVm9qMFFmE6WFW4jv0wdmLTOc VSQ6hBnbr+X34/urbz5CYHW++IR0cTGDhGpbSpld50Ex4tOPkxwqGlufcIlX2xLluFjZ PSr8Hj20BJ3Q7p5Ot3ZRGOWWmQ+gVY7+0mMgAQK2SWf0cSUC6dOJGlnTnWE0J83fJQ2U uGPBojZjoO37Is2UAun9oWM3UDAl228RfG/VXXjXeWfHfknRgMcJ9L3SttvXxhKdtoEm Gx5w== X-Gm-Message-State: AOAM5336hi4hQIQ5UbkVJha2zh65CJPqLTrFIoJmpXBJN40aZrFPZ0ih tKDBfPqEcsNWGzuORrF0VDiDQQ== X-Google-Smtp-Source: ABdhPJyHeJThS2CGZr+cqb3k6241+1z9NseEWmeB1Bv/4KPcEilqB4W9QFeL5x2OlasWZh2soeTT9Q== X-Received: by 2002:a0c:c345:: with SMTP id j5mr957810qvi.13.1613595181073; Wed, 17 Feb 2021 12:53:01 -0800 (PST) Received: from localhost (70.44.39.90.res-cmts.bus.ptd.net. [70.44.39.90]) by smtp.gmail.com with ESMTPSA id n82sm2478957qkn.114.2021.02.17.12.53.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Feb 2021 12:53:00 -0800 (PST) Date: Wed, 17 Feb 2021 15:52:59 -0500 From: Johannes Weiner To: Michal =?iso-8859-1?Q?Koutn=FD?= Cc: Andrew Morton , Tejun Heo , Michal Hocko , Roman Gushchin , Shakeel Butt , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH v3 4/8] cgroup: rstat: support cgroup1 Message-ID: References: <20210209163304.77088-1-hannes@cmpxchg.org> <20210209163304.77088-5-hannes@cmpxchg.org> <20210217174232.GA19239@blackbody.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20210217174232.GA19239@blackbody.suse.cz> X-Stat-Signature: gbde94opwkt6kajmbhc9km7dtdestaby X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 8D312D7 Received-SPF: none (cmpxchg.org>: No applicable sender policy available) receiver=imf12; identity=mailfrom; envelope-from=""; helo=mail-qv1-f45.google.com; client-ip=209.85.219.45 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1613595178-696786 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 Wed, Feb 17, 2021 at 06:42:32PM +0100, Michal Koutn=FD wrote: > Hello. >=20 > On Tue, Feb 09, 2021 at 11:33:00AM -0500, Johannes Weiner wrote: > > @@ -1971,10 +1978,14 @@ int cgroup_setup_root(struct cgroup_root *roo= t, u16 ss_mask) > > if (ret) > > goto destroy_root; > > =20 > > - ret =3D rebind_subsystems(root, ss_mask); > > + ret =3D cgroup_rstat_init(root_cgrp); > Would it make sense to do cgroup_rstat_init() only if there's a subsys > in ss_mask that makes use of rstat? > (On legacy systems there could be individual hierarchy for each > controller so the rstat space can be saved.) It's possible, but I don't think worth the trouble. It would have to be done from rebind_subsystems(), as remount can add more subsystems to an existing cgroup1 root. That in turn means we'd have to have separate init paths for cgroup1 and cgroup2. While we split cgroup1 and cgroup2 paths where necessary in the code, it's a significant maintenance burden and a not unlikely source of subtle errors (see the recent 'fix swap undercounting in cgroup2'). In this case, we're talking about a relatively small data structure and the overhead is per mountpoint. Comparatively, we're allocating the full vmstats structures for cgroup1 groups which barely use them, and cgroup1 softlimit tree structures for each cgroup2 group. So I don't think it's a good tradeoff. Subtle bugs that require kernel patches are more disruptive to the user experience than the amount of memory in question here. > > @@ -285,8 +285,6 @@ void __init cgroup_rstat_boot(void) > > =20 > > for_each_possible_cpu(cpu) > > raw_spin_lock_init(per_cpu_ptr(&cgroup_rstat_cpu_lock, cpu)); > > - > > - BUG_ON(cgroup_rstat_init(&cgrp_dfl_root.cgrp)); > > } > Regardless of the suggestion above, this removal obsoletes the comment > cgroup_rstat_init: >=20 > int cpu; > =20 > - /* the root cgrp has rstat_cpu preallocated */ > if (!cgrp->rstat_cpu) { > cgrp->rstat_cpu =3D alloc_percpu(struct cgroup_rstat_c= pu); Oh, I'm not removing the init call, I'm merely moving it from cgroup_rstat_boot() to cgroup_setup_root(). The default root group has statically preallocated percpu data before and after this patch. See cgroup.c: static DEFINE_PER_CPU(struct cgroup_rstat_cpu, cgrp_dfl_root_rstat_cpu)= ; /* the default hierarchy */ struct cgroup_root cgrp_dfl_root =3D { .cgrp.rstat_cpu =3D &cgrp_dfl_ro= ot_rstat_cpu }; EXPORT_SYMBOL_GPL(cgrp_dfl_root);