Use the per-package counts of known events to allocate arrays to
make a copy of just the known events.
Add hook into resctrl_exit() to cleanup.
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
arch/x86/kernel/cpu/resctrl/internal.h | 2 +
arch/x86/kernel/cpu/resctrl/core.c | 2 +
arch/x86/kernel/cpu/resctrl/intel_aet.c | 60 ++++++++++++++++++++++++-
3 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index ada402c7678b..2503a24e4177 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -170,8 +170,10 @@ int rdt_get_mon_l3_config(struct rdt_resource *r);
#ifdef CONFIG_INTEL_AET_RESCTRL
int rdt_get_intel_aet_mon_config(void);
+void rdt_intel_aet_exit(void);
#else
static inline int rdt_get_intel_aet_mon_config(void) { return 0; }
+static inline void rdt_intel_aet_exit(void) { };
#endif
bool rdt_cpu_has(int flag);
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 2adf40d8de32..d011c095aafa 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -1095,6 +1095,8 @@ static void __exit resctrl_arch_exit(void)
{
cpuhp_remove_state(rdt_online);
+ rdt_intel_aet_exit();
+
resctrl_exit();
}
diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
index 67862e81b9e0..e2d8eab997fc 100644
--- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
+++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
@@ -188,6 +188,26 @@ static bool count_events(struct pkg_info *pkg, int max_pkgs, struct pmt_feature_
return found;
}
+static int setup(struct pkg_info *pkg, int pkgnum, struct pmt_feature_group *p, int slot)
+{
+ struct telem_entry **tentry;
+
+ for (int i = 0; i < p->count; i++) {
+ for (tentry = telem_entry; *tentry; tentry++) {
+ if (!(*tentry)->active)
+ continue;
+ if (pkgnum != p->regions[i].plat_info.package_id)
+ continue;
+ if (p->regions[i].guid != (*tentry)->guid)
+ continue;
+
+ pkg[pkgnum].regions[slot++] = p->regions[i];
+ }
+ }
+
+ return slot;
+}
+
DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *, \
if (!IS_ERR_OR_NULL(_T)) \
intel_pmt_put_feature_group(_T))
@@ -202,6 +222,8 @@ static bool get_events(void)
struct pmt_feature_group *p2 __free(intel_pmt_put_feature_group) = NULL;
int num_pkgs = topology_max_packages();
struct pkg_info *pkg __free(kfree) = NULL;
+ bool found_known_features = false;
+ int i, slot;
pkg = kmalloc_array(num_pkgs, sizeof(*pkg_info), GFP_KERNEL | __GFP_ZERO);
if (!pkg)
@@ -220,13 +242,32 @@ static bool get_events(void)
if (!count_events(pkg, num_pkgs, p2))
intel_pmt_put_feature_group(no_free_ptr(p2));
+ for (i = 0; i < num_pkgs; i++) {
+ if (!pkg[i].count)
+ continue;
+ found_known_features = true;
+ pkg[i].regions = kmalloc_array(pkg[i].count, sizeof(*pkg[i].regions), GFP_KERNEL);
+ if (!pkg[i].regions)
+ goto fail;
+
+ slot = 0;
+ if (!IS_ERR_VALUE(p1))
+ slot = setup(pkg, i, p1, slot);
+ if (!IS_ERR_VALUE(p2))
+ slot = setup(pkg, i, p2, slot);
+ }
+
if (!IS_ERR_OR_NULL(p1))
feat_energy = no_free_ptr(p1);
if (!IS_ERR_OR_NULL(p2))
feat_perf = no_free_ptr(p2);
pkg_info = no_free_ptr(pkg);
- return true;
+ return found_known_features;
+fail:
+ while (--i > 0)
+ kfree(pkg[i].regions);
+ return false;
}
/*
@@ -242,6 +283,23 @@ int rdt_get_intel_aet_mon_config(void)
return 1;
}
+/* Clean up when resctrl shuts down completely */
+void rdt_intel_aet_exit(void)
+{
+ int num_pkgs = topology_max_packages();
+
+ if (pkg_info) {
+ for (int i = 0; i < num_pkgs; i++)
+ kfree(pkg_info[i].regions);
+ kfree(pkg_info);
+ }
+
+ if (feat_energy)
+ intel_pmt_put_feature_group(feat_energy);
+ if (feat_perf)
+ intel_pmt_put_feature_group(feat_perf);
+}
+
/*
* Late (first mount) initialization. Safe to ask OOBMSM which telemetry
* event groups are supported.
--
2.48.1hi Tony,
On 3/21/25 4:16 PM, Tony Luck wrote:
> Use the per-package counts of known events to allocate arrays to
> make a copy of just the known events.
>
> Add hook into resctrl_exit() to cleanup.
(above is not done in patch)
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> arch/x86/kernel/cpu/resctrl/internal.h | 2 +
> arch/x86/kernel/cpu/resctrl/core.c | 2 +
> arch/x86/kernel/cpu/resctrl/intel_aet.c | 60 ++++++++++++++++++++++++-
> 3 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index ada402c7678b..2503a24e4177 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -170,8 +170,10 @@ int rdt_get_mon_l3_config(struct rdt_resource *r);
>
> #ifdef CONFIG_INTEL_AET_RESCTRL
> int rdt_get_intel_aet_mon_config(void);
> +void rdt_intel_aet_exit(void);
> #else
> static inline int rdt_get_intel_aet_mon_config(void) { return 0; }
> +static inline void rdt_intel_aet_exit(void) { };
> #endif
>
> bool rdt_cpu_has(int flag);
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 2adf40d8de32..d011c095aafa 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -1095,6 +1095,8 @@ static void __exit resctrl_arch_exit(void)
> {
> cpuhp_remove_state(rdt_online);
>
> + rdt_intel_aet_exit();
> +
> resctrl_exit();
> }
>
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index 67862e81b9e0..e2d8eab997fc 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -188,6 +188,26 @@ static bool count_events(struct pkg_info *pkg, int max_pkgs, struct pmt_feature_
> return found;
> }
>
> +static int setup(struct pkg_info *pkg, int pkgnum, struct pmt_feature_group *p, int slot)
Could you please add a comment to this function to explain what it does?
> +{
> + struct telem_entry **tentry;
> +
> + for (int i = 0; i < p->count; i++) {
> + for (tentry = telem_entry; *tentry; tentry++) {
> + if (!(*tentry)->active)
> + continue;
> + if (pkgnum != p->regions[i].plat_info.package_id)
> + continue;
> + if (p->regions[i].guid != (*tentry)->guid)
> + continue;
> +
> + pkg[pkgnum].regions[slot++] = p->regions[i];
> + }
> + }
> +
> + return slot;
> +}
> +
> DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *, \
> if (!IS_ERR_OR_NULL(_T)) \
> intel_pmt_put_feature_group(_T))
> @@ -202,6 +222,8 @@ static bool get_events(void)
> struct pmt_feature_group *p2 __free(intel_pmt_put_feature_group) = NULL;
> int num_pkgs = topology_max_packages();
> struct pkg_info *pkg __free(kfree) = NULL;
> + bool found_known_features = false;
> + int i, slot;
>
> pkg = kmalloc_array(num_pkgs, sizeof(*pkg_info), GFP_KERNEL | __GFP_ZERO);
> if (!pkg)
> @@ -220,13 +242,32 @@ static bool get_events(void)
> if (!count_events(pkg, num_pkgs, p2))
> intel_pmt_put_feature_group(no_free_ptr(p2));
>
> + for (i = 0; i < num_pkgs; i++) {
> + if (!pkg[i].count)
> + continue;
> + found_known_features = true;
> + pkg[i].regions = kmalloc_array(pkg[i].count, sizeof(*pkg[i].regions), GFP_KERNEL);
> + if (!pkg[i].regions)
> + goto fail;
> +
> + slot = 0;
> + if (!IS_ERR_VALUE(p1))
> + slot = setup(pkg, i, p1, slot);
> + if (!IS_ERR_VALUE(p2))
> + slot = setup(pkg, i, p2, slot);
> + }
> +
> if (!IS_ERR_OR_NULL(p1))
> feat_energy = no_free_ptr(p1);
> if (!IS_ERR_OR_NULL(p2))
> feat_perf = no_free_ptr(p2);
> pkg_info = no_free_ptr(pkg);
>
> - return true;
> + return found_known_features;
> +fail:
include/linux/cleanup.h has this to say about mixing goto and free helpers:
"convert all resources that need a "goto" cleanup to scope-based cleanup, or convert
none of them"
> + while (--i > 0)
> + kfree(pkg[i].regions);
> + return false;
> }
>
> /*
> @@ -242,6 +283,23 @@ int rdt_get_intel_aet_mon_config(void)
> return 1;
> }
>
> +/* Clean up when resctrl shuts down completely */
> +void rdt_intel_aet_exit(void)
> +{
> + int num_pkgs = topology_max_packages();
> +
> + if (pkg_info) {
> + for (int i = 0; i < num_pkgs; i++)
> + kfree(pkg_info[i].regions);
> + kfree(pkg_info);
> + }
> +
> + if (feat_energy)
> + intel_pmt_put_feature_group(feat_energy);
> + if (feat_perf)
> + intel_pmt_put_feature_group(feat_perf);
> +}
> +
> /*
> * Late (first mount) initialization. Safe to ask OOBMSM which telemetry
> * event groups are supported.
Reinette
On Mon, Mar 31, 2025 at 09:21:49AM -0700, Reinette Chatre wrote:
> hi Tony,
>
> On 3/21/25 4:16 PM, Tony Luck wrote:
> > Use the per-package counts of known events to allocate arrays to
> > make a copy of just the known events.
> >
> > Add hook into resctrl_exit() to cleanup.
>
> (above is not done in patch)
I think I shuffled it to a different patch. I will track it
down and move this comment to the right place.
> >
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> > arch/x86/kernel/cpu/resctrl/internal.h | 2 +
> > arch/x86/kernel/cpu/resctrl/core.c | 2 +
> > arch/x86/kernel/cpu/resctrl/intel_aet.c | 60 ++++++++++++++++++++++++-
> > 3 files changed, 63 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index ada402c7678b..2503a24e4177 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -170,8 +170,10 @@ int rdt_get_mon_l3_config(struct rdt_resource *r);
> >
> > #ifdef CONFIG_INTEL_AET_RESCTRL
> > int rdt_get_intel_aet_mon_config(void);
> > +void rdt_intel_aet_exit(void);
> > #else
> > static inline int rdt_get_intel_aet_mon_config(void) { return 0; }
> > +static inline void rdt_intel_aet_exit(void) { };
> > #endif
> >
> > bool rdt_cpu_has(int flag);
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 2adf40d8de32..d011c095aafa 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -1095,6 +1095,8 @@ static void __exit resctrl_arch_exit(void)
> > {
> > cpuhp_remove_state(rdt_online);
> >
> > + rdt_intel_aet_exit();
> > +
> > resctrl_exit();
> > }
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > index 67862e81b9e0..e2d8eab997fc 100644
> > --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > @@ -188,6 +188,26 @@ static bool count_events(struct pkg_info *pkg, int max_pkgs, struct pmt_feature_
> > return found;
> > }
> >
> > +static int setup(struct pkg_info *pkg, int pkgnum, struct pmt_feature_group *p, int slot)
>
> Could you please add a comment to this function to explain what it does?
Indeed yes, it really needs some description.
>
> > +{
> > + struct telem_entry **tentry;
> > +
> > + for (int i = 0; i < p->count; i++) {
> > + for (tentry = telem_entry; *tentry; tentry++) {
> > + if (!(*tentry)->active)
> > + continue;
> > + if (pkgnum != p->regions[i].plat_info.package_id)
> > + continue;
> > + if (p->regions[i].guid != (*tentry)->guid)
> > + continue;
> > +
> > + pkg[pkgnum].regions[slot++] = p->regions[i];
> > + }
> > + }
> > +
> > + return slot;
> > +}
> > +
> > DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *, \
> > if (!IS_ERR_OR_NULL(_T)) \
> > intel_pmt_put_feature_group(_T))
> > @@ -202,6 +222,8 @@ static bool get_events(void)
> > struct pmt_feature_group *p2 __free(intel_pmt_put_feature_group) = NULL;
> > int num_pkgs = topology_max_packages();
> > struct pkg_info *pkg __free(kfree) = NULL;
> > + bool found_known_features = false;
> > + int i, slot;
> >
> > pkg = kmalloc_array(num_pkgs, sizeof(*pkg_info), GFP_KERNEL | __GFP_ZERO);
> > if (!pkg)
> > @@ -220,13 +242,32 @@ static bool get_events(void)
> > if (!count_events(pkg, num_pkgs, p2))
> > intel_pmt_put_feature_group(no_free_ptr(p2));
> >
> > + for (i = 0; i < num_pkgs; i++) {
> > + if (!pkg[i].count)
> > + continue;
> > + found_known_features = true;
> > + pkg[i].regions = kmalloc_array(pkg[i].count, sizeof(*pkg[i].regions), GFP_KERNEL);
> > + if (!pkg[i].regions)
> > + goto fail;
> > +
> > + slot = 0;
> > + if (!IS_ERR_VALUE(p1))
> > + slot = setup(pkg, i, p1, slot);
> > + if (!IS_ERR_VALUE(p2))
> > + slot = setup(pkg, i, p2, slot);
> > + }
> > +
> > if (!IS_ERR_OR_NULL(p1))
> > feat_energy = no_free_ptr(p1);
> > if (!IS_ERR_OR_NULL(p2))
> > feat_perf = no_free_ptr(p2);
> > pkg_info = no_free_ptr(pkg);
> >
> > - return true;
> > + return found_known_features;
> > +fail:
>
> include/linux/cleanup.h has this to say about mixing goto and free helpers:
> "convert all resources that need a "goto" cleanup to scope-based cleanup, or convert
> none of them"
Seems an awkward restriction for this case. "pkg" is a pointer to
a dynamic array, and each of the elements of the array might have
been initialized by another allocation. "pkg" is under control of
the __free() cleanup function.
Maybe I could define a custom cleanup (syntax of multi-statement
action to be figured out):
DEFINE_FREE(pkg_free, struct pkg_info *,
if (_T)
for (int i = 0; i < num_pkgs; i++)
kfree(_T[i].regions);
kfree(_T)
)
struct pkg_info *pkg __free(pkg_free) = NULL;
>
> > + while (--i > 0)
> > + kfree(pkg[i].regions);
> > + return false;
> > }
> >
> > /*
> > @@ -242,6 +283,23 @@ int rdt_get_intel_aet_mon_config(void)
> > return 1;
> > }
> >
> > +/* Clean up when resctrl shuts down completely */
> > +void rdt_intel_aet_exit(void)
> > +{
> > + int num_pkgs = topology_max_packages();
> > +
> > + if (pkg_info) {
> > + for (int i = 0; i < num_pkgs; i++)
> > + kfree(pkg_info[i].regions);
> > + kfree(pkg_info);
> > + }
> > +
> > + if (feat_energy)
> > + intel_pmt_put_feature_group(feat_energy);
> > + if (feat_perf)
> > + intel_pmt_put_feature_group(feat_perf);
> > +}
> > +
> > /*
> > * Late (first mount) initialization. Safe to ask OOBMSM which telemetry
> > * event groups are supported.
>
> Reinette
On Mon, Mar 31, 2025 at 03:23:53PM -0700, Luck, Tony wrote: > On Mon, Mar 31, 2025 at 09:21:49AM -0700, Reinette Chatre wrote: > > include/linux/cleanup.h has this to say about mixing goto and free helpers: > > "convert all resources that need a "goto" cleanup to scope-based cleanup, or convert > > none of them" > > Seems an awkward restriction for this case. "pkg" is a pointer to > a dynamic array, and each of the elements of the array might have > been initialized by another allocation. "pkg" is under control of > the __free() cleanup function. > > Maybe I could define a custom cleanup (syntax of multi-statement > action to be figured out): > > DEFINE_FREE(pkg_free, struct pkg_info *, > if (_T) > for (int i = 0; i < num_pkgs; i++) > kfree(_T[i].regions); > kfree(_T) > ) > > struct pkg_info *pkg __free(pkg_free) = NULL; Tried this out and it works (without any odd syntax for the multi-line action (though I did miss that num_pkgs was a local variable, thankfully simply initialized from topology_max_packages(). DEFINE_FREE(free_pkg_info, struct pkg_info *, \ if (_T) \ for (int i = 0; i < topology_max_packages(); i++) \ kfree(_T[i].regions); \ kfree(_T)) ... struct pkg_info *pkg __free(free_pkg_info) = NULL; -Tony
© 2016 - 2026 Red Hat, Inc.