| 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:
<OSZPR01MB879824B7EBE583C7EB4EA7128B842@OSZPR01MB8798.jpnprd01.prod.outlook.com>
Date: Fri, 25 Apr 2025 02:56:08 +0000
From: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To: 'James Morse' <james.morse@....com>, "x86@...nel.org" <x86@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Reinette Chatre <reinette.chatre@...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"
<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
<scott@...amperecomputing.com>, "carl@...amperecomputing.com"
<carl@...amperecomputing.com>, "lcherian@...vell.com" <lcherian@...vell.com>,
"bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
"baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>, Jamie Iles
<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
"peternewman@...gle.com" <peternewman@...gle.com>, "dfustini@...libre.com"
<dfustini@...libre.com>, "amitsinght@...vell.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" <fenghuay@...dia.com>, Babu
Moger <babu.moger@....com>
Subject: RE: [PATCH v8 02/21] x86/resctrl: Remove the limit on the number of
CLOSID
Hello James, Reinette,
> 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>
> Tested-by: Peter Newman <peternewman@...gle.com>
> Tested-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> Tested-by: Amit Singh Tomar <amitsinght@...vell.com> # arm64
> Tested-by: Shanker Donthineni <sdonthineni@...dia.com> # arm64
> Tested-by: Babu Moger <babu.moger@....com>
> Reviewed-by: Fenghua Yu <fenghuay@...dia.com>
> ---
> Changes since v7:
> * Moved closid_exit() earlier in rdt_kill_sb() to match what
> rdt_get_tree() does.
>
> Changes since v6:
> * Set variable to NULL after kfree()ing it.
> * Call closid_exit() from rdt_kill_sb() to prevent a memory leak.
>
> Changes since v5:
> * This patch got pulled into this series.
> ---
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 47
> +++++++++++++++++---------
> 1 file changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 776c8e347654..4e0308040c6e 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,31 @@ 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);
> + closid_free_map = NULL;
> }
>
> static int closid_alloc(void)
> @@ -182,12 +193,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 +206,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 +220,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);
> }
>
> /**
> @@ -2765,20 +2775,22 @@ static int rdt_get_tree(struct fs_context *fc)
> goto out_ctx;
> }
Not relevant to this patch, but some of the code around here needs improvement. Maybe consider fixing it together?
ret = schemata_list_create();
if (ret) {
- schemata_list_destroy();
- goto out_ctx;
+ goto out_schemata_free;
}
Best regards,
Shaopeng TAN
> - 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,
> @@ -2829,6 +2841,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:
> @@ -3076,6 +3090,7 @@ static void rdt_kill_sb(struct super_block *sb)
> rmdir_all_sub();
> rdt_pseudo_lock_release();
> rdtgroup_default.mode = RDT_MODE_SHAREABLE;
> + closid_exit();
> schemata_list_destroy();
> rdtgroup_destroy_root();
> if (resctrl_arch_alloc_capable())
> --
> 2.20.1
Powered by blists - more mailing lists