| 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
| ||
|
Message-ID: <ff99af93-5121-5a27-e24f-6354b9dffa1f@linux.intel.com>
Date: Mon, 24 Apr 2023 18:12:29 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: "Moger, Babu" <Babu.Moger@....com>
cc: "corbet@....net" <corbet@....net>,
Reinette Chatre <reinette.chatre@...el.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>,
"fenghua.yu@...el.com" <fenghua.yu@...el.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"paulmck@...nel.org" <paulmck@...nel.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"quic_neeraju@...cinc.com" <quic_neeraju@...cinc.com>,
"rdunlap@...radead.org" <rdunlap@...radead.org>,
"damien.lemoal@...nsource.wdc.com" <damien.lemoal@...nsource.wdc.com>,
"songmuchun@...edance.com" <songmuchun@...edance.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"jpoimboe@...nel.org" <jpoimboe@...nel.org>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"chang.seok.bae@...el.com" <chang.seok.bae@...el.com>,
"pawan.kumar.gupta@...ux.intel.com"
<pawan.kumar.gupta@...ux.intel.com>,
"jmattson@...gle.com" <jmattson@...gle.com>,
"daniel.sneddon@...ux.intel.com" <daniel.sneddon@...ux.intel.com>,
"Das1, Sandipan" <Sandipan.Das@....com>,
"tony.luck@...el.com" <tony.luck@...el.com>,
"james.morse@....com" <james.morse@....com>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
"bagasdotme@...il.com" <bagasdotme@...il.com>,
"eranian@...gle.com" <eranian@...gle.com>,
"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
"jarkko@...nel.org" <jarkko@...nel.org>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"quic_jiles@...cinc.com" <quic_jiles@...cinc.com>,
"peternewman@...gle.com" <peternewman@...gle.com>
Subject: RE: [PATCH v4 7/7] x86/resctrl: Add debug files when mounted with
debug option
On Fri, 21 Apr 2023, Moger, Babu wrote:
> > -----Original Message-----
> > From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> > Sent: Thursday, April 20, 2023 3:59 AM
> > To: Moger, Babu <Babu.Moger@....com>
> > Cc: corbet@....net; Reinette Chatre <reinette.chatre@...el.com>;
> > tglx@...utronix.de; mingo@...hat.com; bp@...en8.de; fenghua.yu@...el.com;
> > dave.hansen@...ux.intel.com; x86@...nel.org; hpa@...or.com;
> > paulmck@...nel.org; akpm@...ux-foundation.org; quic_neeraju@...cinc.com;
> > rdunlap@...radead.org; damien.lemoal@...nsource.wdc.com;
> > songmuchun@...edance.com; peterz@...radead.org; jpoimboe@...nel.org;
> > pbonzini@...hat.com; chang.seok.bae@...el.com;
> > pawan.kumar.gupta@...ux.intel.com; jmattson@...gle.com;
> > daniel.sneddon@...ux.intel.com; Das1, Sandipan <Sandipan.Das@....com>;
> > tony.luck@...el.com; james.morse@....com; linux-doc@...r.kernel.org;
> > LKML <linux-kernel@...r.kernel.org>; bagasdotme@...il.com;
> > eranian@...gle.com; christophe.leroy@...roup.eu; jarkko@...nel.org;
> > adrian.hunter@...el.com; quic_jiles@...cinc.com; peternewman@...gle.com
> > Subject: Re: [PATCH v4 7/7] x86/resctrl: Add debug files when mounted with
> > debug option
> >
> > On Wed, 19 Apr 2023, Moger, Babu wrote:
> >
> > >
> > >
> > > On 4/19/23 08:20, Ilpo Järvinen wrote:
> > > > On Mon, 17 Apr 2023, Babu Moger wrote:
> > > >
> > > >> Add the debug files to the resctrl hierarchy.
> > > >>
> > > >> Signed-off-by: Babu Moger <babu.moger@....com>
> > > >> ---
> > > >> arch/x86/kernel/cpu/resctrl/internal.h | 1 +
> > > >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 54
> > +++++++++++++++++++++++++++++++-
> > > >> 2 files changed, 54 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
> > > >> b/arch/x86/kernel/cpu/resctrl/internal.h
> > > >> index 1eac07ebc31b..855109abb480 100644
> > > >> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > > >> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > > >> @@ -288,6 +288,7 @@ struct rdtgroup {
> > > >> #define RFTYPE_TOP BIT(4)
> > > >> #define RFTYPE_RES_CACHE BIT(5)
> > > >> #define RFTYPE_RES_MB BIT(6)
> > > >> +#define RFTYPE_DEBUG BIT(7)
> > > >> #define RFTYPE_CTRL_INFO (RFTYPE_INFO |
> > RFTYPE_CTRL)
> > > >> #define RFTYPE_MON_INFO (RFTYPE_INFO |
> > RFTYPE_MON)
> > > >> #define RFTYPE_TOP_INFO (RFTYPE_INFO |
> > RFTYPE_TOP)
> > > >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > >> index 15ded0dd5b09..1ec4359348c2 100644
> > > >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > >> @@ -1880,6 +1880,7 @@ static struct rftype res_common_files[] = {
> > > >> .mode = 0444,
> > > >> .kf_ops = &rdtgroup_kf_single_ops,
> > > >> .seq_show = rdtgroup_rmid_show,
> > > >> + .fflags = RFTYPE_BASE | RFTYPE_DEBUG,
> > > >> },
> > > >> {
> > > >> .name = "schemata",
> > > >> @@ -1909,6 +1910,7 @@ static struct rftype res_common_files[] = {
> > > >> .mode = 0444,
> > > >> .kf_ops = &rdtgroup_kf_single_ops,
> > > >> .seq_show = rdtgroup_closid_show,
> > > >> + .fflags = RFTYPE_CTRL_BASE | RFTYPE_DEBUG,
> > > >> },
> > > >>
> > > >> };
> > > >> @@ -2420,6 +2422,49 @@ static int mkdir_mondata_all(struct
> > kernfs_node *parent_kn,
> > > >> struct rdtgroup *prgrp,
> > > >> struct kernfs_node **mon_data_kn);
> > > >>
> > > >> +static void resctrl_add_debug_files(void) {
> > > >> + struct rftype *rfts, *rft;
> > > >> + int len;
> > > >> +
> > > >> + rfts = res_common_files;
> > > >> + len = ARRAY_SIZE(res_common_files);
> > > >> +
> > > >> + lockdep_assert_held(&rdtgroup_mutex);
> > > >> +
> > > >> + for (rft = rfts; rft < rfts + len; rft++) {
> > > >> + if (rft->fflags & RFTYPE_DEBUG) {
> > > >> + rft->fflags &= ~RFTYPE_DEBUG;
> > > >
> > > > I don't fully follow why you need to play with ->fflags like this.
> > > >
> > > > Is it for the ->fflags test in rdtgroup_add_files()? Can't you just
> > > > do some extra masking there for RFTYPE_DEBUG based on resctrl_debug
> > > > which you already keep?
> > >
> > > Actually with this change, I can remove all these tricks here.
> > > I don't have to change the check "if (rft->fflags && ((fflags &
> > > rft->fflags) == rft->fflags)) {"
> > >
> > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > index 1ec4359348c2..b560c44817bb 100644
> > > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > > @@ -1925,6 +1925,9 @@ static int rdtgroup_add_files(struct kernfs_node
> > > *kn, unsigned long fflags)
> > >
> > > lockdep_assert_held(&rdtgroup_mutex);
> > >
> > > + if (resctrl_debug)
> > > + fflags |= RFTYPE_DEBUG;
> >
> > Yes, looks good.
> >
> > It matches to the idea I had in my mind but doesn't require putting it
> > into the if condition itself.
>
> Without if condition? How? Let me know.
I was referring to the if condition within the loop, not to doing it
without some conditional (I had an (resctrl_debug ? RFTYPE_DEBUG : 0)
construct in my mind).
To remove if, it would, of course, be possible to use another static
file-level variable but it doesn't seem justified. I think what you
proposed is fine for this use and looks clean.
--
i.
Powered by blists - more mailing lists