From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751746AbdG0QWu (ORCPT ); Thu, 27 Jul 2017 12:22:50 -0400 Received: from mail-qt0-f174.google.com ([209.85.216.174]:33579 "EHLO mail-qt0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751465AbdG0QWs (ORCPT ); Thu, 27 Jul 2017 12:22:48 -0400 Date: Thu, 27 Jul 2017 12:22:43 -0400 From: Tejun Heo To: Roman Gushchin Cc: cgroups@vger.kernel.org, Zefan Li , Waiman Long , Johannes Weiner , kernel-team@fb.com, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] cgroup: add cgroup.stat interface with basic hierarchy stats Message-ID: <20170727162243.GF742618@devbig577.frc2.facebook.com> References: <20170727161420.2552-1-guro@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170727161420.2552-1-guro@fb.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Thu, Jul 27, 2017 at 05:14:20PM +0100, Roman Gushchin wrote: > Add a cgroup.stat interface to the base cgroup control files > with the following metrics: > > nr_descendants total number of descendant cgroups > nr_dying_descendants total number of dying descendant cgroups > max_descendant_depth maximum descent depth below the current cgroup Yeah, this'd be great to have. Some comments below. > + cgroup.stat > + A read-only flat-keyed file with the following entries: > + > + nr_descendants > + Total number of descendant cgroups. > + > + nr_dying_descendants > + Total number of dying descendant cgroups. Can you please go into more detail on what's going on with dying descendants here? > +static int cgroup_stats_show(struct seq_file *seq, void *v) > +{ > + struct cgroup_subsys_state *css; > + unsigned long total = 0; > + unsigned long offline = 0; > + int max_level = 0; > + > + rcu_read_lock(); > + css_for_each_descendant_pre(css, seq_css(seq)) { > + if (css == seq_css(seq)) > + continue; > + ++total; Let's do post increment for consistency. > + if (!(css->flags & CSS_ONLINE)) > + ++offline; > + if (css->cgroup->level > max_level) > + max_level = css->cgroup->level; > + } > + rcu_read_unlock(); I wonder whether we want to keep these counters in sync instead of trying to gather the number on read. Walking all descendants can get expensive pretty quickly and things like nr_descendants will be useful for other purposes too. Thanks. -- tejun