[v2] platform/x86/intel-uncore-freq: Fix missing uncore sysfs during CPU hotplug

[PATCH v2] platform/x86/intel-uncore-freq: Fix missing uncore sysfs during CPU hotplug

Posted by shouyeliu 8 months, 1 week ago

From: Shouye Liu <shouyeliu@tencent.com>

In certain situations, the sysfs for uncore may not be present when all
CPUs in a package are offlined and then brought back online after boot.

This issue can occur if there is an error in adding the sysfs entry due
to a memory allocation failure. Retrying to bring the CPUs online will
not resolve the issue, as the uncore_cpu_mask is already set for the
package before the failure condition occurs.

This issue does not occur if the failure happens during module
initialization, as the module will fail to load in the event of any
error.

To address this, ensure that the uncore_cpu_mask is not set until the
successful return of uncore_freq_add_entry().

Signed-off-by: Shouye Liu <shouyeliu@tencent.com>
---
 .../x86/intel/uncore-frequency/uncore-frequency.c   | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
index 40bbf8e45fa4..bdee5d00f30b 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
@@ -146,15 +146,13 @@ static int uncore_event_cpu_online(unsigned int cpu)
 {
 	struct uncore_data *data;
 	int target;
+	int ret;
 
 	/* Check if there is an online cpu in the package for uncore MSR */
 	target = cpumask_any_and(&uncore_cpu_mask, topology_die_cpumask(cpu));
 	if (target < nr_cpu_ids)
 		return 0;
 
-	/* Use this CPU on this die as a control CPU */
-	cpumask_set_cpu(cpu, &uncore_cpu_mask);
-
 	data = uncore_get_instance(cpu);
 	if (!data)
 		return 0;
@@ -163,7 +161,14 @@ static int uncore_event_cpu_online(unsigned int cpu)
 	data->die_id = topology_die_id(cpu);
 	data->domain_id = UNCORE_DOMAIN_ID_INVALID;
 
-	return uncore_freq_add_entry(data, cpu);
+	ret = uncore_freq_add_entry(data, cpu);
+	if (ret)
+		return ret;
+
+	/* Use this CPU on this die as a control CPU */
+	cpumask_set_cpu(cpu, &uncore_cpu_mask);
+
+	return 0;
 }
 
 static int uncore_event_cpu_offline(unsigned int cpu)
-- 
2.19.1

Re: [PATCH v2] platform/x86/intel-uncore-freq: Fix missing uncore sysfs during CPU hotplug

Posted by srinivas pandruvada 8 months ago

On Wed, 2025-04-16 at 11:38 +0800, shouyeliu wrote:
> From: Shouye Liu <shouyeliu@tencent.com>
> 
> In certain situations, the sysfs for uncore may not be present when
> all
> CPUs in a package are offlined and then brought back online after
> boot.
> 
> This issue can occur if there is an error in adding the sysfs entry
> due
> to a memory allocation failure. Retrying to bring the CPUs online
> will
> not resolve the issue, as the uncore_cpu_mask is already set for the
> package before the failure condition occurs.
> 
> This issue does not occur if the failure happens during module
> initialization, as the module will fail to load in the event of any
> error.
> 
> To address this, ensure that the uncore_cpu_mask is not set until the
> successful return of uncore_freq_add_entry().
> 
Fixes: dbce412a7733 ("platform/x86/intel-uncore-freq: Split common and
enumeration part")

> Signed-off-by: Shouye Liu <shouyeliu@tencent.com>
Cc: stable@vger.kernel.org

Thanks,
Srinivas

> ---
>  .../x86/intel/uncore-frequency/uncore-frequency.c   | 13 +++++++++--
> --
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-
> frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-
> frequency.c
> index 40bbf8e45fa4..bdee5d00f30b 100644
> --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
> +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
> @@ -146,15 +146,13 @@ static int uncore_event_cpu_online(unsigned int
> cpu)
>  {
>  	struct uncore_data *data;
>  	int target;
> +	int ret;
>  
>  	/* Check if there is an online cpu in the package for uncore
> MSR */
>  	target = cpumask_any_and(&uncore_cpu_mask,
> topology_die_cpumask(cpu));
>  	if (target < nr_cpu_ids)
>  		return 0;
>  
> -	/* Use this CPU on this die as a control CPU */
> -	cpumask_set_cpu(cpu, &uncore_cpu_mask);
> -
>  	data = uncore_get_instance(cpu);
>  	if (!data)
>  		return 0;
> @@ -163,7 +161,14 @@ static int uncore_event_cpu_online(unsigned int
> cpu)
>  	data->die_id = topology_die_id(cpu);
>  	data->domain_id = UNCORE_DOMAIN_ID_INVALID;
>  
> -	return uncore_freq_add_entry(data, cpu);
> +	ret = uncore_freq_add_entry(data, cpu);
> +	if (ret)
> +		return ret;
> +
> +	/* Use this CPU on this die as a control CPU */
> +	cpumask_set_cpu(cpu, &uncore_cpu_mask);
> +
> +	return 0;
>  }
>  
>  static int uncore_event_cpu_offline(unsigned int cpu)

Re: [PATCH v2] platform/x86/intel-uncore-freq: Fix missing uncore sysfs during CPU hotplug

Posted by liu shouye 8 months ago

srinivas pandruvada <srinivas.pandruvada@linux.intel.com> 于2025年4月17日周四 04:40写道：
>
> On Wed, 2025-04-16 at 11:38 +0800, shouyeliu wrote:
> > From: Shouye Liu <shouyeliu@tencent.com>
> >
> > In certain situations, the sysfs for uncore may not be present when
> > all
> > CPUs in a package are offlined and then brought back online after
> > boot.
> >
> > This issue can occur if there is an error in adding the sysfs entry
> > due
> > to a memory allocation failure. Retrying to bring the CPUs online
> > will
> > not resolve the issue, as the uncore_cpu_mask is already set for the
> > package before the failure condition occurs.
> >
> > This issue does not occur if the failure happens during module
> > initialization, as the module will fail to load in the event of any
> > error.
> >
> > To address this, ensure that the uncore_cpu_mask is not set until the
> > successful return of uncore_freq_add_entry().
> >
> Fixes: dbce412a7733 ("platform/x86/intel-uncore-freq: Split common and
> enumeration part")
>
> > Signed-off-by: Shouye Liu <shouyeliu@tencent.com>
> Cc: stable@vger.kernel.org
>
> Thanks,
> Srinivas

Got it, I will update it in v3 version
Thanks,
Shouye

>
> > ---
> >  .../x86/intel/uncore-frequency/uncore-frequency.c   | 13 +++++++++--
> > --
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-
> > frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-
> > frequency.c
> > index 40bbf8e45fa4..bdee5d00f30b 100644
> > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
> > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
> > @@ -146,15 +146,13 @@ static int uncore_event_cpu_online(unsigned int
> > cpu)
> >  {
> >       struct uncore_data *data;
> >       int target;
> > +     int ret;
> >
> >       /* Check if there is an online cpu in the package for uncore
> > MSR */
> >       target = cpumask_any_and(&uncore_cpu_mask,
> > topology_die_cpumask(cpu));
> >       if (target < nr_cpu_ids)
> >               return 0;
> >
> > -     /* Use this CPU on this die as a control CPU */
> > -     cpumask_set_cpu(cpu, &uncore_cpu_mask);
> > -
> >       data = uncore_get_instance(cpu);
> >       if (!data)
> >               return 0;
> > @@ -163,7 +161,14 @@ static int uncore_event_cpu_online(unsigned int
> > cpu)
> >       data->die_id = topology_die_id(cpu);
> >       data->domain_id = UNCORE_DOMAIN_ID_INVALID;
> >
> > -     return uncore_freq_add_entry(data, cpu);
> > +     ret = uncore_freq_add_entry(data, cpu);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Use this CPU on this die as a control CPU */
> > +     cpumask_set_cpu(cpu, &uncore_cpu_mask);
> > +
> > +     return 0;
> >  }
> >
> >  static int uncore_event_cpu_offline(unsigned int cpu)
>