| 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: <4f1825cc-5d80-c1b1-b56b-7f2535e406f7@intel.com>
Date: Tue, 15 Jun 2021 13:41:08 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86/resctrl: Fix kernel-doc in internal.h
Hi Fabio,
On 6/14/2021 8:44 AM, Fabio M. De Francesco wrote:
> Add description of undocumented parameters. Issues detected by
> scripts/kernel-doc.
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.com>
> ---
>
> v1->v2: According to a first review by Reinette Chartre, remove changes
> unrelated to the subject of this patch and modify the descriptions of
> two parameters.
>
> arch/x86/kernel/cpu/resctrl/internal.h | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index c4d320d02fd5..ac691af0174b 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -70,6 +70,7 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
> * struct mon_evt - Entry in the event list of a resource
> * @evtid: event id
> * @name: name of the event
> + * @list: entry in &rdt_resource->evt_list
> */
> struct mon_evt {
> u32 evtid;
> @@ -78,10 +79,13 @@ struct mon_evt {
> };
>
> /**
> - * struct mon_data_bits - Monitoring details for each event file
> - * @rid: Resource id associated with the event file.
> + * union mon_data_bits - Monitoring details for each event file
> + * @priv: Used to store monitoring event data in @u
> + * as kernfs private data
> + * @rid: Resource id associated with the event file
> * @evtid: Event id associated with the event file
> * @domid: The domain to which the event file belongs
> + * @u: Name of the bit fields struct
> */
This snippet is whitespace damaged. Your changes add tabs as well as
spaces while the existing code uses just spaces. Please follow existing
style of this area and just use spaces. As a note for any future
changes, in one line you add spaces before tabs, that is generally not
the right formatting in kernel-doc - running scripts/checkpatch.pl on
this patch would also warn about this.
> union mon_data_bits {
> void *priv;
> @@ -119,6 +123,7 @@ enum rdt_group_type {
> * @RDT_MODE_PSEUDO_LOCKSETUP: Resource group will be used for Pseudo-Locking
> * @RDT_MODE_PSEUDO_LOCKED: No sharing of this resource group's allocations
> * allowed AND the allocations are Cache Pseudo-Locked
> + * @RDT_NUM_MODES: Total number of modes
> *
> * The mode of a resource group enables control over the allowed overlap
> * between allocations associated with different resource groups (classes
> @@ -142,7 +147,7 @@ enum rdtgrp_mode {
>
> /**
> * struct mongroup - store mon group's data in resctrl fs.
> - * @mon_data_kn kernlfs node for the mon_data directory
> + * @mon_data_kn: kernlfs node for the mon_data directory
Sorry I did not notice this before, could you please also fix the typo
kernlfs -> kernfs ?
> * @parent: parent rdtgrp
> * @crdtgrp_list: child rdtgroup node list
> * @rmid: rmid for this rdtgroup
> @@ -282,11 +287,11 @@ struct rftype {
> /**
> * struct mbm_state - status for each MBM counter in each domain
> * @chunks: Total data moved (multiply by rdt_group.mon_scale to get bytes)
> - * @prev_msr Value of IA32_QM_CTR for this RMID last time we read it
> + * @prev_msr: Value of IA32_QM_CTR for this RMID last time we read it
> * @prev_bw_msr:Value of previous IA32_QM_CTR for bandwidth counting
> - * @prev_bw The most recent bandwidth in MBps
> - * @delta_bw Difference between the current and previous bandwidth
> - * @delta_comp Indicates whether to compute the delta_bw
> + * @prev_bw: The most recent bandwidth in MBps
> + * @delta_bw: Difference between the current and previous bandwidth
> + * @delta_comp: Indicates whether to compute the delta_bw
> */
> struct mbm_state {
> u64 chunks;
> @@ -450,17 +455,19 @@ struct rdt_parse_data {
> * @name: Name to use in "schemata" file
> * @num_closid: Number of CLOSIDs available
> * @cache_level: Which cache level defines scope of this resource
> - * @default_ctrl: Specifies default cache cbm or memory B/W percent.
> + * @default_ctrl: Specifies default cache cbm or memory B/W percent
> * @msr_base: Base MSR address for CBMs
> * @msr_update: Function pointer to update QOS MSRs
> * @data_width: Character width of data when displaying
> * @domains: All domains for this resource
> * @cache: Cache allocation related data
> + * @membw: Memory bandwidth allocation related data
> * @format_str: Per resource format string to show domain value
> * @parse_ctrlval: Per resource function pointer to parse control values
> * @evt_list: List of monitoring events
> * @num_rmid: Number of RMIDs available
> * @mon_scale: cqm counter * mon_scale = occupancy in bytes
> + * @mbm_width: Width of memory bandwidth monitoring hardware counter
> * @fflags: flags to choose base and info files
> */
Fixes to membw and mbm_width are also arriving via another patch series
(see commit
https://lore.kernel.org/lkml/20210614200941.12383-2-james.morse@arm.com/).
To make it easier to merge that patch and yours could you please inherit
the descriptions from there?
@mbm_width: Monitor width, to detect and correct for overflow.
@membw: If the component has bandwidth controls, their properties.
Thank you
Reinette
Powered by blists - more mailing lists