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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADjb_WR_DNAR_4jVEJ1C4LO7Xfnc54J2u2AoqyCjZT39+AhrWA@mail.gmail.com>
Date:   Sat, 16 Nov 2019 23:01:12 +0800
From:   Ryan Chen <yu.chen.surf@...il.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Chen Yu <yu.c.chen@...el.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will@...nel.org>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Michal Hocko <mhocko@...e.com>,
        Kees Cook <keescook@...omium.org>,
        Christian Brauner <christian@...uner.io>,
        Shakeel Butt <shakeelb@...gle.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Konstantin Khlebnikov <khlebnikov@...dex-team.ru>,
        Kent Overstreet <kent.overstreet@...il.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 2/2][v2] x86/resctrl: Add task resctrl information display

Hi Boris,
On Fri, Nov 15, 2019 at 5:25 PM Borislav Petkov <bp@...en8.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@...el.com>
> > Reviewed-by: Reinette Chatre <reinette.chatre@...el.com>
> > Reviewed-by: Fenghua Yu <fenghua.yu@...el.com>
> > Reviewed-by: Tony Luck <tony.luck@...el.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