Re: [PATCH 2/2][v2] x86/resctrl: Add task resctrl information display

From: Borislav Petkov <bp@alien8.de>
To: Chen Yu <yu.c.chen@intel.com>
Cc: x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Will Deacon <will@kernel.org>, Fenghua Yu <fenghua.yu@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Michal Hocko <mhocko@suse.com>, Kees Cook <keescook@chromium.org>,
	Christian Brauner <christian@brauner.io>,
	Shakeel Butt <shakeelb@google.com>,
	Casey Schaufler <casey@schaufler-ca.com>,
	Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
	Kent Overstreet <kent.overstreet@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 2/2][v2] x86/resctrl: Add task resctrl information display
Date: Fri, 15 Nov 2019 10:24:20 +0100	[thread overview]
Message-ID: <20191115092420.GF18929@zn.tnic> (raw)
In-Reply-To: <5dcd6580b51342c0803db6bc27866dd569914b0d.1573788882.git.yu.c.chen@intel.com>

On Fri, Nov 15, 2019 at 01:25:06PM +0800, Chen Yu wrote:
> Monitoring tools that want to find out which resctrl CTRL
> and MONITOR groups a task belongs to must currently read
> the "tasks" file in every group until they locate the process
> ID.
> 
> Add an additional file /proc/{pid}/resctrl to provide this
> information.
> 
> For example:
>  cat /proc/1193/resctrl
> CTRL_MON:/ctrl_grp0
> MON:/ctrl_grp0/mon_groups/mon_grp0
> 
> If the resctrl filesystem has not been mounted,
> reading /proc/{pid}/resctrl returns an error:
> cat: /proc/1193/resctrl: No such device

Eww, this doesn't sound very user-friendly. How is the user supposed to
know that the resctrl fs needs to be mounted for this to work?

Why does the resctrl fs need to be mounted at all to show this?

I'm guessing if it is not mounted, you have no groups so you don't have
to return an error - you simply return "". Right?

> Tested-by: Jinshi Chen <jinshi.chen@intel.com>
> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
> Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>

When you send a new version which has non-trivial changes, you should
drop those tags because they don't apply anymore. Unless those people
have managed to review and test the new version ...

Looking at CONFIG_PROC_PID_ARCH_STATUS for an example of proc/ calling
arch-specific functions, I guess you need to do:

select CPU_RESCTRL if PROC_FS

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette