| 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: <6c1e02c6-085a-4866-99a3-59b954b687ee@intel.com>
Date: Wed, 19 Feb 2025 20:21:53 -0800
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>
Subject: Re: [PATCH v6 31/42] x86/resctrl: Remove the limit on the number of
CLOSID
Hi James,
On 2/7/25 10:18 AM, James Morse wrote:
> From: Amit Singh Tomar <amitsinght@...vell.com>
>
> Resctrl allocates and finds free CLOSID values using the bits of a u32.
> This restricts the number of control groups that can be created by
> user-space.
>
> MPAM has an architectural limit of 2^16 CLOSID values, Intel x86 could
> be extended beyond 32 values. There is at least one MPAM platform which
> supports more than 32 CLOSID values.
>
> Replace the fixed size bitmap with calls to the bitmap API to allocate
> an array of a sufficient size.
>
> ffs() returns '1' for bit 0, hence the existing code subtracts 1 from
> the index to get the CLOSID value. find_first_bit() returns the bit
> number which does not need adjusting.
>
> Signed-off-by: Amit Singh Tomar <amitsinght@...vell.com>
> [ morse: fixed the off-by-one in the allocator and the wrong
> not-found value. Removed the limit. Rephrase the commit message. ]
> Signed-off-by: James Morse <james.morse@....com>
> ---
> Changes since v5:
> * This patch got pulled into this series.
> ---
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 45 +++++++++++++++++---------
> 1 file changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 08fec23a38bf..de79da30d500 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -130,8 +130,8 @@ static bool resctrl_is_mbm_event(int e)
> }
>
> /*
> - * Trivial allocator for CLOSIDs. Since h/w only supports a small number,
> - * we can keep a bitmap of free CLOSIDs in a single integer.
> + * Trivial allocator for CLOSIDs. Use BITMAP APIs to manipulate a bitmap
> + * of free CLOSIDs.
> *
> * Using a global CLOSID across all resources has some advantages and
> * some drawbacks:
> @@ -144,7 +144,7 @@ static bool resctrl_is_mbm_event(int e)
> * - Our choices on how to configure each resource become progressively more
> * limited as the number of resources grows.
> */
> -static unsigned long closid_free_map;
> +static unsigned long *closid_free_map;
> static int closid_free_map_len;
>
> int closids_supported(void)
> @@ -152,20 +152,30 @@ int closids_supported(void)
> return closid_free_map_len;
> }
>
> -static void closid_init(void)
> +static int closid_init(void)
> {
> struct resctrl_schema *s;
> - u32 rdt_min_closid = 32;
> + u32 rdt_min_closid = ~0;
>
> /* Compute rdt_min_closid across all resources */
> list_for_each_entry(s, &resctrl_schema_all, list)
> rdt_min_closid = min(rdt_min_closid, s->num_closid);
>
> - closid_free_map = BIT_MASK(rdt_min_closid) - 1;
> + closid_free_map = bitmap_alloc(rdt_min_closid, GFP_KERNEL);
> + if (!closid_free_map)
> + return -ENOMEM;
> + bitmap_fill(closid_free_map, rdt_min_closid);
>
> /* RESCTRL_RESERVED_CLOSID is always reserved for the default group */
> - __clear_bit(RESCTRL_RESERVED_CLOSID, &closid_free_map);
> + __clear_bit(RESCTRL_RESERVED_CLOSID, closid_free_map);
> closid_free_map_len = rdt_min_closid;
> +
> + return 0;
> +}
> +
> +static void closid_exit(void)
> +{
> + bitmap_free(closid_free_map);
With closid_free_map being a global, could this also set
closid_free_map to NULL?
> }
>
> static int closid_alloc(void)
> @@ -182,12 +192,11 @@ static int closid_alloc(void)
> return cleanest_closid;
> closid = cleanest_closid;
> } else {
> - closid = ffs(closid_free_map);
> - if (closid == 0)
> + closid = find_first_bit(closid_free_map, closid_free_map_len);
> + if (closid == closid_free_map_len)
> return -ENOSPC;
> - closid--;
> }
> - __clear_bit(closid, &closid_free_map);
> + __clear_bit(closid, closid_free_map);
>
> return closid;
> }
> @@ -196,7 +205,7 @@ void closid_free(int closid)
> {
> lockdep_assert_held(&rdtgroup_mutex);
>
> - __set_bit(closid, &closid_free_map);
> + __set_bit(closid, closid_free_map);
> }
>
> /**
> @@ -210,7 +219,7 @@ bool closid_allocated(unsigned int closid)
> {
> lockdep_assert_held(&rdtgroup_mutex);
>
> - return !test_bit(closid, &closid_free_map);
> + return !test_bit(closid, closid_free_map);
> }
>
> /**
> @@ -2754,20 +2763,22 @@ static int rdt_get_tree(struct fs_context *fc)
> goto out_ctx;
> }
>
> - closid_init();
> + ret = closid_init();
> + if (ret)
> + goto out_schemata_free;
>
> if (resctrl_arch_mon_capable())
> flags |= RFTYPE_MON;
>
> ret = rdtgroup_add_files(rdtgroup_default.kn, flags);
> if (ret)
> - goto out_schemata_free;
> + goto out_closid_exit;
>
> kernfs_activate(rdtgroup_default.kn);
>
> ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
> if (ret < 0)
> - goto out_schemata_free;
> + goto out_closid_exit;
>
> if (resctrl_arch_mon_capable()) {
> ret = mongroup_create_dir(rdtgroup_default.kn,
> @@ -2818,6 +2829,8 @@ static int rdt_get_tree(struct fs_context *fc)
> kernfs_remove(kn_mongrp);
> out_info:
> kernfs_remove(kn_info);
> +out_closid_exit:
> + closid_exit();
> out_schemata_free:
> schemata_list_destroy();
> out_ctx:
With closid_init() called from rdt_get_tree() during mount I expected
closid_exit() to be called from rdt_kill_sb() during unmount ?
Reinette
Powered by blists - more mailing lists