| 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: <ac19ac7e-e230-4310-9b70-b9d57ee4439e@intel.com>
Date: Thu, 1 May 2025 10:04:10 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
<linux-kernel@...r.kernel.org>
CC: 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>, Koba Ko <kobak@...dia.com>, Shanker
Donthineni <sdonthineni@...dia.com>, <fenghuay@...dia.com>, Tony Luck
<tony.luck@...el.com>
Subject: Re: [PATCH v9 08/27] x86/resctrl: Expand the width of domid by
replacing mon_data_bits
Hi James,
On 4/25/25 10:37 AM, James Morse wrote:
> ---
> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 19 ++++--
> arch/x86/kernel/cpu/resctrl/internal.h | 39 ++++++-----
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 79 +++++++++++++++++++++--
> 3 files changed, 103 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 0a0ac5f6112e..159972c3fe73 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -667,7 +667,7 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
> u32 resid, evtid, domid;
I was expecting this to look differently after reading
https://lore.kernel.org/lkml/a9008c2d-e83d-4bc6-8197-0753666a7ec2@arm.com/
I believe u32 was used for resid, evtid, and domid because of how they
used to be initialized from the bitfield within the union. With the switch to
a struct that now has the proper types these can also use proper types.
enum resctrl_res_level resid;
enum resctrl_event_id evtid;
int domid;
This highlights that the incorrect type propagated from rdtgroup_mondata_show()
to mon_event_read() where its "int evtid" parameter should also be
"enum resctrl_event_id evtid", which is a good complement to patch #14
that fixes the type used by functions called by mon_event_read().
> struct rdtgroup *rdtgrp;
> struct rdt_resource *r;
> - union mon_data_bits md;
> + struct mon_data *md;
> int ret = 0;
>
> rdtgrp = rdtgroup_kn_lock_live(of->kn);
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 36a862a4832f..954dc391fc33 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -103,27 +103,26 @@ struct mon_evt {
> };
>
> /**
> - * union mon_data_bits - Monitoring details for each event file.
> - * @priv: Used to store monitoring event data in @u
> - * as kernfs private data.
> - * @u.rid: Resource id associated with the event file.
> - * @u.evtid: Event id associated with the event file.
> - * @u.sum: Set when event must be summed across multiple
> - * domains.
> - * @u.domid: When @u.sum is zero this is the domain to which
> - * the event file belongs. When @sum is one this
> - * is the id of the L3 cache that all domains to be
> - * summed share.
> - * @u: Name of the bit fields struct.
> + * struct mon_data - Monitoring details for each event file.
> + * @list: Member of the global @mon_data_kn_priv_list list.
> + * @rid: Resource id associated with the event file.
> + * @evtid: Event id associated with the event file.
> + * @sum: Set when event must be summed across multiple
> + * domains.
> + * @domid: When @sum is zero this is the domain to which
> + * the event file belongs. When @sum is one this
> + * is the id of the L3 cache that all domains to be
> + * summed share.
> + *
> + * Pointed to by the kernfs kn->priv field of monitoring event files.
> + * Readers and writers must hold rdtgroup_mutex.
> */
> -union mon_data_bits {
> - void *priv;
> - struct {
> - unsigned int rid : 10;
> - enum resctrl_event_id evtid : 7;
> - unsigned int sum : 1;
> - unsigned int domid : 14;
> - } u;
> +struct mon_data {
> + struct list_head list;
> + enum resctrl_res_level rid;
> + enum resctrl_event_id evtid;
> + int domid;
> + bool sum;
> };
>
Thank you.
> /**
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index eccdfcb1a6f5..7ef5cf0c4d1d 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -45,6 +45,12 @@ LIST_HEAD(rdt_all_groups);
> /* list of entries for the schemata file */
> LIST_HEAD(resctrl_schema_all);
>
> +/*
> + * List of struct mon_data containing private data of event files for use by
> + * rdtgroup_mondata_show(). Protected by rdtgroup_mutex.
> + */
> +static LIST_HEAD(mon_data_kn_priv_list);
> +
> /* The filesystem can only be mounted once. */
> bool resctrl_mounted;
>
> @@ -3093,6 +3099,63 @@ static void rmdir_all_sub(void)
> kernfs_remove(kn_mondata);
> }
>
> +/**
> + * mon_get_kn_priv() - Get the mon_data priv data for this event.
> + *
> + * The same values are used across the mon_data directories of all control and
> + * monitor groups for the same event in the same domain. Keep a list of
> + * allocated structures and re-use an existing one with the same values for
> + * @rid, @domid, etc.
> + *
> + * @rid: The resource id for the event file being created.
> + * @domid: The domain id for the event file being created.
> + * @mevt: The type of event file being created.
> + * @do_sum: Whether SNC summing monitors are being created.
> + */
> +static struct mon_data *mon_get_kn_priv(int rid, int domid,
"int rid" -> "enum resctrl_res_level rid"
> + struct mon_evt *mevt,
> + bool do_sum)
> +{
> + struct mon_data *priv;
> +
> + lockdep_assert_held(&rdtgroup_mutex);
> +
> + list_for_each_entry(priv, &mon_data_kn_priv_list, list) {
> + if (priv->rid == rid && priv->domid == domid &&
> + priv->sum == do_sum && priv->evtid == mevt->evtid)
> + return priv;
> + }
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return NULL;
> +
> + priv->rid = rid;
> + priv->domid = domid;
> + priv->sum = do_sum;
> + priv->evtid = mevt->evtid;
> + list_add_tail(&priv->list, &mon_data_kn_priv_list);
> +
> + return priv;
> +}
> +
> +/**
> + * mon_put_kn_priv() - Free all allocated mon_data structures.
> + *
> + * Called when resctrl file system is unmounted.
> + */
> +static void mon_put_kn_priv(void)
> +{
> + struct mon_data *priv, *tmp;
> +
> + lockdep_assert_held(&rdtgroup_mutex);
> +
> + list_for_each_entry_safe(priv, tmp, &mon_data_kn_priv_list, list) {
> + list_del(&priv->list);
> + kfree(priv);
> + }
> +}
> +
> static void resctrl_fs_teardown(void)
> {
> lockdep_assert_held(&rdtgroup_mutex);
Reinette
Powered by blists - more mailing lists