[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28f474e7-0bcd-45ec-b50e-6708f97d6403@intel.com>
Date: Fri, 28 Jun 2024 09:41:45 -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>, Shaopeng Tan
<tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v3 01/38] x86/resctrl: Fix allocation of cleanest CLOSID
on platforms with no monitors
Hi James,
On 6/14/24 7:59 AM, James Morse wrote:
> commit 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by
> searching closid_num_dirty_rmid") added logic that causes resctrl to
There is a custom in x86 on how messages containing references to commits
are formatted. For reference you can see how the recent fix
739c9765793e ("x86/resctrl: Don't try to free nonexistent RMIDs") was
reformatted before merge. Applied to this patch, it may look something
like:
Commit
6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid")
added logic ...
Looking at, for example, commit e3ca96e479c9 ("x86/resctrl: Pass domain to
target CPU"), it does seem ok to split the name of the commit.
> search for the CLOSID with the fewest dirty cache lines when creating a
> new control group, if requested by the arch code. This depends on the
> values read from the llc_occupancy counters. The logic is applicable to
> architectures where the CLOSID effectively forms part of the monitoring
> identifier and so do not allow complete freedom to choose an unused
> monitoring identifier for a given CLOSID.
>
> This support missed that some platforms may not have these counters.
> This causes a NULL pointer dereference when creating a new control
> group as the array was not allocated by dom_data_init().
>
> As this feature isn't necessary on platforms that don't have cache
> occupancy monitors, add this to the check that occurs when a new
> control group is allocated.
>
The snippet below does not belong in changelog and can be moved to
maintainer notes.
> The existing code is not selected by any upstream platform, it makes
> no sense to backport this patch to stable.
>
> Fixes: 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid")
> Signed-off-by: James Morse <james.morse@....com>
> Tested-by: Peter Newman <peternewman@...gle.com>
> Tested-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> Reviewed-by: David Hildenbrand <david@...hat.com>
>
> ---
> Changes since v1:
> * [Commit message only] Reword the first paragraph to make it clear
> that the issue being fixed wasn't directly associated with addition
> of a Kconfig option. (Actually, the option is not in Kconfig yet,
> and gets added later in this series.)
> ---
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 02f213f1c51c..d02f4c97e40f 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -149,7 +149,8 @@ static int closid_alloc(void)
>
> lockdep_assert_held(&rdtgroup_mutex);
>
> - if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) &&
> + is_llc_occupancy_enabled()) {
> cleanest_closid = resctrl_find_cleanest_closid();
> if (cleanest_closid < 0)
> return cleanest_closid;
With changelog updated:
| Reviewed-by: Reinette Chatre <reinette.chatre@...el.com>
Reinette
Powered by blists - more mailing lists