linux-kernel - RE: [PATCH v20 09/18] x86/resctrl: Add a new field to struct rmid

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]

Date: Thu, 20 Jun 2024 22:42:12 +0000
From: "Luck, Tony" <tony.luck@...el.com>
To: "Chatre, Reinette" <reinette.chatre@...el.com>, "Yu, Fenghua"
	<fenghua.yu@...el.com>, "Wieczor-Retman, Maciej"
	<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
	James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
 Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>
CC: "x86@...nel.org" <x86@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>
Subject: RE: [PATCH v20 09/18] x86/resctrl: Add a new field to struct
 rmid_read for summation of domains

> > When a user reads a monitor file rdtgroup_mondata_show() calls
> > mon_event_read() to package up all the required details into an rmid_read
> > structure which is passed across the smp_call*() infrastructure to code
> > that will read data from hardware and return the value (or error status)
> > in the rmid_read structure.
> >
> > Sub-NUMA Cluster (SNC) mode adds files with new semantics. These require
> > the smp_call-ed code to sum event data from all domains that share an
> > L3 cache.
> >
> > Add a pointer to the L3 "cacheinfo" structure to struct rmid_read
> > for the data collection routines to use to pick the domains to be
> > summed.
> >
> > Reinette suggested that the rmid_read structure has become complex
> > enough to warrant documentation of each of its fields. Add the kerneldoc
> > documentation for struct rmid_read.
> >
> > Signed-off-by: Tony Luck <tony.luck@...el.com>
> > ---
> >   arch/x86/kernel/cpu/resctrl/internal.h | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index 99f601d05f3b..d29c7b58c151 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -145,12 +145,25 @@ union mon_data_bits {
> >     } u;
> >   };
> >
> > +/**
> > + * struct rmid_read - Data passed across smp_call*() to read event count
> > + * @rgrp:  Resctrl group
> > + * @r:             Resource
> > + * @d:             Domain. If NULL then sum all domains in @r sharing L3 @ci.id
> > + * @evtid: Which monitor event to read
> > + * @first: Initializes MBM counter when true
> > + * @ci:            Cacheinfo for L3. Used when summing domains
> > + * @err:   Return error indication
> > + * @val:   Return value of event counter
> > + * @arch_mon_ctx: Hardware monitor allocated for this read request (MPAM only)
> > + */
>
> Thank you for adding the kerneldoc. I understand that this file is not
> consistent on how these kerneldoc are formatted, but could you please
> pick whether you think sentences need to end with a period and then stick
> to it in this portion?

This is about the @d and @ci entries that have a "sentence" ending with period,
and then more text that doesn't (matching other lines in this block).

Maybe some other punctuation to split the parts?  Do you like "colon"

* @d:         Domain: If NULL then sum all domains in @r sharing L3 @ci.id
* @ci:        Cacheinfo for L3: Used when summing domains

of maybe "dash"

* @d:         Domain - If NULL then sum all domains in @r sharing L3 @ci.id
* @ci:        Cacheinfo for L3 - Used when summing domains

Or something else?

>
> >   struct rmid_read {
> >     struct rdtgroup         *rgrp;
> >     struct rdt_resource     *r;
> >     struct rdt_mon_domain   *d;
> >     enum resctrl_event_id   evtid;
> >     bool                    first;
> > +   struct cacheinfo        *ci;
> >     int                     err;
> >     u64                     val;
> >     void                    *arch_mon_ctx;

-Tony