From: "Michal Koutný" <mkoutny@suse.com> To: Johannes Weiner <hannes@cmpxchg.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Tejun Heo <tj@kernel.org>, Michal Hocko <mhocko@suse.com>, Roman Gushchin <guro@fb.com>, Shakeel Butt <shakeelb@google.com>, 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 Date: Wed, 17 Feb 2021 18:42:32 +0100 [thread overview] Message-ID: <20210217174232.GA19239@blackbody.suse.cz> (raw) In-Reply-To: <20210209163304.77088-5-hannes@cmpxchg.org> [-- Attachment #1: Type: text/plain, Size: 1432 bytes --] Hello. On Tue, Feb 09, 2021 at 11:33:00AM -0500, Johannes Weiner <hannes@cmpxchg.org> wrote: > @@ -1971,10 +1978,14 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask) > if (ret) > goto destroy_root; > > - ret = rebind_subsystems(root, ss_mask); > + ret = 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.) > @@ -5159,11 +5170,9 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name, > if (ret) > goto out_free_cgrp; > > - if (cgroup_on_dfl(parent)) { > - ret = cgroup_rstat_init(cgrp); > - if (ret) > - goto out_cancel_ref; > - } > + ret = cgroup_rstat_init(cgrp); And here do cgroup_rstat_init() only when parent has it. > @@ -285,8 +285,6 @@ void __init cgroup_rstat_boot(void) > > 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: int cpu; - /* the root cgrp has rstat_cpu preallocated */ if (!cgrp->rstat_cpu) { cgrp->rstat_cpu = alloc_percpu(struct cgroup_rstat_cpu); Regards, Michal [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: "Michal Koutný" <mkoutny-IBi9RG/b67k@public.gmane.org> To: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>, Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>, Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-team-b10kYP2dOMg@public.gmane.org Subject: Re: [PATCH v3 4/8] cgroup: rstat: support cgroup1 Date: Wed, 17 Feb 2021 18:42:32 +0100 [thread overview] Message-ID: <20210217174232.GA19239@blackbody.suse.cz> (raw) In-Reply-To: <20210209163304.77088-5-hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> [-- Attachment #1: Type: text/plain, Size: 1432 bytes --] Hello. On Tue, Feb 09, 2021 at 11:33:00AM -0500, Johannes Weiner <hannes@cmpxchg.org> wrote: > @@ -1971,10 +1978,14 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask) > if (ret) > goto destroy_root; > > - ret = rebind_subsystems(root, ss_mask); > + ret = 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.) > @@ -5159,11 +5170,9 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name, > if (ret) > goto out_free_cgrp; > > - if (cgroup_on_dfl(parent)) { > - ret = cgroup_rstat_init(cgrp); > - if (ret) > - goto out_cancel_ref; > - } > + ret = cgroup_rstat_init(cgrp); And here do cgroup_rstat_init() only when parent has it. > @@ -285,8 +285,6 @@ void __init cgroup_rstat_boot(void) > > 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: int cpu; - /* the root cgrp has rstat_cpu preallocated */ if (!cgrp->rstat_cpu) { cgrp->rstat_cpu = alloc_percpu(struct cgroup_rstat_cpu); Regards, Michal [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2021-02-17 17:43 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-09 16:32 [PATCH v3 0/8] mm: memcontrol: switch to rstat Johannes Weiner 2021-02-09 16:32 ` Johannes Weiner 2021-02-09 16:32 ` [PATCH v3 1/8] mm: memcontrol: fix cpuhotplug statistics flushing Johannes Weiner 2021-02-09 16:32 ` Johannes Weiner 2021-02-17 17:43 ` Michal Koutný 2021-02-17 17:43 ` Michal Koutný 2021-02-09 16:32 ` [PATCH v3 2/8] mm: memcontrol: kill mem_cgroup_nodeinfo() Johannes Weiner 2021-02-09 16:32 ` Johannes Weiner 2021-02-17 17:43 ` Michal Koutný 2021-02-17 17:43 ` Michal Koutný 2021-02-09 16:32 ` [PATCH v3 3/8] mm: memcontrol: privatize memcg_page_state query functions Johannes Weiner 2021-02-09 16:32 ` Johannes Weiner 2021-02-17 17:44 ` Michal Koutný 2021-02-17 17:44 ` Michal Koutný 2021-02-09 16:33 ` [PATCH v3 4/8] cgroup: rstat: support cgroup1 Johannes Weiner 2021-02-09 16:33 ` Johannes Weiner 2021-02-17 17:42 ` Michal Koutný [this message] 2021-02-17 17:42 ` Michal Koutný 2021-02-17 20:52 ` Johannes Weiner 2021-02-18 15:45 ` Michal Koutný 2021-02-18 15:45 ` Michal Koutný 2021-02-18 17:06 ` Johannes Weiner 2021-02-09 16:33 ` [PATCH v3 5/8] cgroup: rstat: punt root-level optimization to individual controllers Johannes Weiner 2021-02-09 16:33 ` Johannes Weiner 2021-02-09 16:33 ` [PATCH v3 6/8] mm: memcontrol: switch to rstat Johannes Weiner 2021-02-09 16:33 ` Johannes Weiner 2021-02-18 15:36 ` Michal Koutný 2021-02-18 15:36 ` Michal Koutný 2021-02-09 16:33 ` [PATCH v3 7/8] mm: memcontrol: consolidate lruvec stat flushing Johannes Weiner 2021-02-09 16:33 ` Johannes Weiner 2021-02-09 20:21 ` Roman Gushchin 2021-02-09 20:21 ` Roman Gushchin 2021-02-09 16:33 ` [PATCH v3 8/8] kselftests: cgroup: update kmem test for new vmstat implementation Johannes Weiner 2021-02-09 20:17 ` Roman Gushchin 2021-02-09 20:17 ` Roman Gushchin 2021-02-18 15:37 ` Michal Koutný 2021-02-18 15:37 ` Michal Koutný
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210217174232.GA19239@blackbody.suse.cz \ --to=mkoutny@suse.com \ --cc=akpm@linux-foundation.org \ --cc=cgroups@vger.kernel.org \ --cc=guro@fb.com \ --cc=hannes@cmpxchg.org \ --cc=kernel-team@fb.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@suse.com \ --cc=shakeelb@google.com \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.