Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1573788882.git.yu.c.chen@intel.com> <5dcd6580b51342c0803db6bc27866dd569914b0d.1573788882.git.yu.c.chen@intel.com>
 <20191115092420.GF18929@zn.tnic>
In-Reply-To: <20191115092420.GF18929@zn.tnic>
From:   Ryan Chen <yu.chen.surf@gmail.com>
Date:   Sat, 16 Nov 2019 23:01:12 +0800
Message-ID: <CADjb_WR_DNAR_4jVEJ1C4LO7Xfnc54J2u2AoqyCjZT39+AhrWA@mail.gmail.com>
Subject: Re: [PATCH 2/2][v2] x86/resctrl: Add task resctrl information display
To:     Borislav Petkov <bp@alien8.de>
Cc:     Chen Yu <yu.c.chen@intel.com>,
        "the arch/x86 maintainers" <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 Mailing List <linux-kernel@vger.kernel.org>,
        linux-fsdevel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Boris,
On Fri, Nov 15, 2019 at 5:25 PM Borislav Petkov <bp@alien8.de> wrote:
>
> 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?
>
Right, we can return 'blank' to user and let the user to parse the information.
And there is a similar behavior in cgroup that, for kernel thread that
does not belong
to any cgroup, /proc/{pid}/cgroup just show 'blank' without returning an error.

> > 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 ...
>
Got it, thanks for telling me this.
> 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
>
Yes, only when PROC_FS is set, /proc/{pid}/resctrl
can be displayed. However, CPU_RESCTRL might not
depend on proc fs, it is possible that the CPU_RESCTRL
is enabled but without PROC_FS set. If I understand correctly,
 CPU_RESCTRL is the 'root' config for X86_CPU_RESCTRL,
after reading this thread:
https://lists.gt.net/linux/kernel/3211659
 If this is the case, shall we add the new file at kernel/resctrl/resctrl.c?
And the generic proc_resctrl_show() could be put into this file. In the future
the generic code for resctrl could be added/moved to kernel/resctrl/resctrl.c

Thanks,
Chenyu

> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette