| 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
| ||
|
Date: Fri, 28 Jun 2024 09:45:26 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
<linux-kernel@...r.kernel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
<hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
<scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
"Dave Martin" <dave.martin@....com>
Subject: Re: [PATCH v3 06/38] x86/resctrl: Move data_width to be a schema
property
Hi James,
On 6/14/24 8:00 AM, James Morse wrote:
> The resctrl architecture code gets to specify the width of the schema
> entries that are used by resctrl. These are determined by the schema
> format, e.g. percentage or bitmap.
>
> Move this property into struct resctrl_schema and get the filesystem
> parts of resctrl to set it based on the schema format.
>
> This allows rdt_init_padding() to be removed, its work can be done
> by schemata_list_add(), allowing max_name_width and max_data_width
> to be moved out of core.c which has no counterpart after the
> move to fs.
Please do write commit messages in imperative mood.
>
> The logic for calculating max_name_width was moved in earlier patches,
> but the definition was not moved.
>
> Signed-off-by: James Morse <james.morse@....com>
> ---
> Changes since v2:
> * This patch is new.
> ---
> arch/x86/kernel/cpu/resctrl/core.c | 26 --------------------------
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++++
> include/linux/resctrl.h | 4 ++--
> 3 files changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 4a5216a13b46..4de7d20aa5aa 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -44,12 +44,6 @@ static DEFINE_MUTEX(domain_list_lock);
> */
> DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
>
> -/*
> - * Used to store the max resource name width and max resource data width
> - * to display the schemata in a tabular format
> - */
> -int max_name_width, max_data_width;
> -
> /*
> * Global boolean for rdt_alloc which is true if any
> * resource allocation is enabled.
> @@ -222,7 +216,6 @@ static bool __get_mem_config_intel(struct rdt_resource *r)
> return false;
> r->membw.arch_needs_linear = false;
> }
> - r->data_width = 3;
>
> if (boot_cpu_has(X86_FEATURE_PER_THREAD_MBA))
> r->membw.throttle_mode = THREAD_THROTTLE_PER_THREAD;
> @@ -262,8 +255,6 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
> r->membw.throttle_mode = THREAD_THROTTLE_UNDEFINED;
> r->membw.min_bw = 0;
> r->membw.bw_gran = 1;
> - /* Max value is 2048, Data width should be 4 in decimal */
> - r->data_width = 4;
>
> r->alloc_capable = true;
>
> @@ -283,7 +274,6 @@ static void rdt_get_cache_alloc_cfg(int idx, struct rdt_resource *r)
> r->cache.cbm_len = eax.split.cbm_len + 1;
> r->default_ctrl = BIT_MASK(eax.split.cbm_len + 1) - 1;
> r->cache.shareable_bits = ebx & r->default_ctrl;
> - r->data_width = (r->cache.cbm_len + 3) / 4;
> if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> r->cache.arch_has_sparse_bitmasks = ecx.split.noncont;
> r->alloc_capable = true;
> @@ -631,20 +621,6 @@ static int resctrl_arch_offline_cpu(unsigned int cpu)
> return 0;
> }
>
> -/*
> - * Choose a width for the resource name and resource data based on the
> - * resource that has widest name and cbm.
> - */
> -static __init void rdt_init_padding(void)
> -{
> - struct rdt_resource *r;
> -
> - for_each_alloc_capable_rdt_resource(r) {
> - if (r->data_width > max_data_width)
> - max_data_width = r->data_width;
> - }
> -}
> -
> enum {
> RDT_FLAG_CMT,
> RDT_FLAG_MBM_TOTAL,
> @@ -942,8 +918,6 @@ static int __init resctrl_late_init(void)
> if (!get_rdt_resources())
> return -ENODEV;
>
> - rdt_init_padding();
> -
> state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> "x86/resctrl/cat:online:",
> resctrl_arch_online_cpu,
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index af9968328771..4f8e20cc06eb 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -58,6 +58,12 @@ static struct kernfs_node *kn_mongrp;
> /* Kernel fs node for "mon_data" directory under root */
> static struct kernfs_node *kn_mondata;
>
> +/*
> + * Used to store the max resource name width and max resource data width
> + * to display the schemata in a tabular format
> + */
I understand that you just copied existing text here, but since you touch this line
could you please have this sentence end with a period?
> +int max_name_width, max_data_width;
> +
> static struct seq_buf last_cmd_status;
> static char last_cmd_status_buf[512];
>
> @@ -2600,15 +2606,20 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
> switch (r->schema_fmt) {
> case RESCTRL_SCHEMA_BITMAP:
> s->fmt_str = "%d=%0*x";
> + s->data_width = (r->cache.cbm_len + 3) / 4;
> break;
> case RESCTRL_SCHEMA_PERCENTAGE:
> s->fmt_str = "%d=%0*u";
> + s->data_width = 3;
> break;
> case RESCTRL_SCHEMA_MBPS:
> s->fmt_str = "%d=%0*u";
> + s->data_width = 4;
> break;
> }
>
> + max_data_width = max(max_data_width, s->data_width);
> +
> INIT_LIST_HEAD(&s->list);
> list_add(&s->list, &resctrl_schema_all);
>
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index abecbd92ac93..ddcd938972d2 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -182,7 +182,6 @@ enum resctrl_schema_fmt {
> * @membw: If the component has bandwidth controls, their properties.
> * @domains: RCU list of all domains for this resource
> * @name: Name to use in "schemata" file.
> - * @data_width: Character width of data when displaying
> * @default_ctrl: Specifies default cache cbm or memory B/W percent.
> * @schema_fmt: Which format string and parser is used for this schema.
> * @evt_list: List of monitoring events
> @@ -198,7 +197,6 @@ struct rdt_resource {
> struct resctrl_membw membw;
> struct list_head domains;
> char *name;
> - int data_width;
> u32 default_ctrl;
> enum resctrl_schema_fmt schema_fmt;
> struct list_head evt_list;
> @@ -218,6 +216,7 @@ struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l);
> * @list: Member of resctrl_schema_all.
> * @name: The name to use in the "schemata" file.
> * @fmt_str: Format string to show domain value
> + * @data_width: Character width of data when displaying
> * @conf_type: Whether this schema is specific to code/data.
> * @res: The resource structure exported by the architecture to describe
> * the hardware that is configured by this schema.
> @@ -229,6 +228,7 @@ struct resctrl_schema {
> struct list_head list;
> char name[8];
> const char *fmt_str;
> + int data_width;
> enum resctrl_conf_type conf_type;
> struct rdt_resource *res;
> u32 num_closid;
Reinette
Powered by blists - more mailing lists