[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08ca8fe7-9da6-41da-aad2-0081b789326c@intel.com>
Date: Tue, 28 May 2024 15:55:29 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>, Maciej
Wieczor-Retman <maciej.wieczor-retman@...el.com>, Peter Newman
<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
<Dave.Martin@....com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
<patches@...ts.linux.dev>
Subject: Re: [PATCH v19 00/20] Add support for Sub-NUMA cluster (SNC) systems
Hi Tony,
On 5/28/24 3:19 PM, Tony Luck wrote:
> This series based on top of Linus upstream commit 33e02dc69afb ("Merge
> tag 'sound-6.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound")
>
> The Sub-NUMA cluster feature on some Intel processors partitions the CPUs
> that share an L3 cache into two or more sets. This plays havoc with the
> Resource Director Technology (RDT) monitoring features. Prior to this
> patch Intel has advised that SNC and RDT are incompatible.
>
> Some of these CPUs support an MSR that can partition the RMID counters
> in the same way. This allows monitoring features to be used. Legacy
> monitoring files provide the sum of counters from each SNC node for
> backwards compatibility. Additional files per SNC node provide details
> per node.
>
> Cache and memory bandwidth allocation features continue to operate at
> the scope of the L3 cache.
>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
>
> ---
> Changes since v18: https://lore.kernel.org/all/20240515222326.74166-1-tony.luck@intel.com/
>
> Global: Consistent use of "Sub-NUMA Cluster (SNC)"
(briefly scanning changes)
>
> 1-4: No change
>
> 5: Rename RESCTRL_NODE as RESCTRL_L3_NODE to make it clear that
> these "nodes" are each subsets of L3 cache instances.
>
> 6: Changes for snc_nodes_per_l3_cache are localized to monitor.c
> Don't use it in decision block use of mba_MBps option.
> Moved the old get_node_rmid() function here, but renamed it to
> logical_rmid_to_physical_rmid() with a block comment explaining
> how RMIDs are distributed when SNC is enabled. Function now
> checks if snc_nodes_per_l3_cache == 1 for fast return.
>
> 7: New patch. Only allow mba_MBps option if scope of MBM matches MBA
>
> 8: Replaces old patch 8. "display_id" field is no more. Add and
> initialize the @ci (struct cachinfo *) to rdt_mon_domain.
> Note that the new get_cpu_cacheinfo_level() helper function is
> added to internal.h as it will also be needed by patch 19.
>
> 9: Instead of display_id, add pointer to cacheinfo structure to
> struct rmid_read. Add kerneldoc description of existing and
> new fields.
>
> 10: Added to commit comment describing why mkdir_mondata_subdir()
> needs to be refactored.
>
> 11: Dropped Intel specific description of fields in the mon_evt
> structure. Say that choice of bit to steal was arbitrary, but
> can be changed in the future.
>
> 12: Fixed typo s/and file/and files/ in commit message. Now using
> the cacheinfo structure (specifically "id" field) instead of
> display_id.
>
> 13: Wordsmith commit into imperative.
> I looked at using kobject_has_children() to check for empty
> directory, but it needs a "struct kobject *" and all I have
> is "struct kernfs_node *". I'm now checking how many CPUs
Consider how kobject_has_children() uses that struct kobject *.
Specifically:
return kobj->sd && kobj->sd->dir.subdirs
It operates on kobj->sd, which is exactly what you have: struct kernfs_node.
> remain in ci->shared_cpu_map to detect whether this is the
> last SNC node.
hmmm, ok, will take a look ... but please finalize discussion of a patch series
before submitting a new series that rejects feedback without discussion and
does something completely different in new version.
Reinette
Powered by blists - more mailing lists