linux-kernel - Re: [PATCH] x86/amd/power: Replace do_div() with div64

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250728200506.46631e2d@pumpkin>
Date: Mon, 28 Jul 2025 20:05:06 +0100
From: David Laight <david.laight.linux@...il.com>
To: Niko Nikolov <nikolay.niko.nikolov@...il.com>
Cc: ray.huang@....com, gautham.shenoy@....com, mario.limonciello@....com,
 perry.yuan@....com, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 shuah@...nel.org
Subject: Re: [PATCH] x86/amd/power: Replace do_div() with div64_u64()

On Thu, 24 Jul 2025 14:21:05 -0700
Niko Nikolov <nikolay.niko.nikolov@...il.com> wrote:

> do_div() divides 64-by-32, and can risk truncation if divisor exceeds 32 bits.
> Use div64_u64() for full 64/64-bit division as recommended, resolving static analysis warnings.

There seem to be a lot of unrelated (mostly whitespace) changes.

You also need to check the domain of the values.

Ok and nak - is is just p-lain broken.

> 
> Signed-off-by: Niko Nikolov <nikolay.niko.nikolov@...il.com>
> ---
>  arch/x86/events/amd/power.c | 49 ++++++++++++++++++-------------------
>  1 file changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/x86/events/amd/power.c b/arch/x86/events/amd/power.c
> index dad42790cf7d..35eff3383660 100644
> --- a/arch/x86/events/amd/power.c
> +++ b/arch/x86/events/amd/power.c
> @@ -15,12 +15,12 @@
>  #include "../perf_event.h"
>  
>  /* Event code: LSB 8 bits, passed in attr->config any other bit is reserved. */
> -#define AMD_POWER_EVENT_MASK		0xFFULL
> +#define AMD_POWER_EVENT_MASK 0xFFULL
>  
>  /*
>   * Accumulated power status counters.
>   */
> -#define AMD_POWER_EVENTSEL_PKG		1
> +#define AMD_POWER_EVENTSEL_PKG 1
>  
>  /*
>   * The ratio of compute unit power accumulator sample period to the
> @@ -65,7 +65,7 @@ static void event_update(struct perf_event *event)
>  	delta *= cpu_pwr_sample_ratio * 1000;
>  	tdelta = new_ptsc - prev_ptsc;
>  
> -	do_div(delta, tdelta);
> +	div64_u64(delta, tdelta);

nak - this is broken...

	David

>  	local64_add(delta, &event->count);
>  }
>  
> @@ -144,8 +144,8 @@ static void pmu_event_read(struct perf_event *event)
>  	event_update(event);
>  }
>  
> -static ssize_t
> -get_attr_cpumask(struct device *dev, struct device_attribute *attr, char *buf)
> +static ssize_t get_attr_cpumask(struct device *dev,
> +				struct device_attribute *attr, char *buf)
>  {
>  	return cpumap_print_to_pagebuf(true, buf, &cpu_mask);
>  }
> @@ -165,12 +165,12 @@ static struct attribute_group pmu_attr_group = {
>   * Currently it only supports to report the power of each
>   * processor/package.
>   */
> -EVENT_ATTR_STR(power-pkg, power_pkg, "event=0x01");
> +EVENT_ATTR_STR(power - pkg, power_pkg, "event=0x01");
>  
> -EVENT_ATTR_STR(power-pkg.unit, power_pkg_unit, "mWatts");
> +EVENT_ATTR_STR(power - pkg.unit, power_pkg_unit, "mWatts");
>  
>  /* Convert the count from micro-Watts to milli-Watts. */
> -EVENT_ATTR_STR(power-pkg.scale, power_pkg_scale, "1.000000e-3");
> +EVENT_ATTR_STR(power - pkg.scale, power_pkg_scale, "1.000000e-3");
>  
>  static struct attribute *events_attr[] = {
>  	EVENT_PTR(power_pkg),
> @@ -180,8 +180,8 @@ static struct attribute *events_attr[] = {
>  };
>  
>  static struct attribute_group pmu_events_group = {
> -	.name	= "events",
> -	.attrs	= events_attr,
> +	.name = "events",
> +	.attrs = events_attr,
>  };
>  
>  PMU_FORMAT_ATTR(event, "config:0-7");
> @@ -192,8 +192,8 @@ static struct attribute *formats_attr[] = {
>  };
>  
>  static struct attribute_group pmu_format_group = {
> -	.name	= "format",
> -	.attrs	= formats_attr,
> +	.name = "format",
> +	.attrs = formats_attr,
>  };
>  
>  static const struct attribute_group *attr_groups[] = {
> @@ -204,17 +204,17 @@ static const struct attribute_group *attr_groups[] = {
>  };
>  
>  static struct pmu pmu_class = {
> -	.attr_groups	= attr_groups,
> +	.attr_groups = attr_groups,
>  	/* system-wide only */
> -	.task_ctx_nr	= perf_invalid_context,
> -	.event_init	= pmu_event_init,
> -	.add		= pmu_event_add,
> -	.del		= pmu_event_del,
> -	.start		= pmu_event_start,
> -	.stop		= pmu_event_stop,
> -	.read		= pmu_event_read,
> -	.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
> -	.module		= THIS_MODULE,
> +	.task_ctx_nr = perf_invalid_context,
> +	.event_init = pmu_event_init,
> +	.add = pmu_event_add,
> +	.del = pmu_event_del,
> +	.start = pmu_event_start,
> +	.stop = pmu_event_stop,
> +	.read = pmu_event_read,
> +	.capabilities = PERF_PMU_CAP_NO_EXCLUDE,
> +	.module = THIS_MODULE,
>  };
>  
>  static int power_cpu_exit(unsigned int cpu)
> @@ -278,10 +278,9 @@ static int __init amd_power_pmu_init(void)
>  		return -ENODEV;
>  	}
>  
> -
>  	cpuhp_setup_state(CPUHP_AP_PERF_X86_AMD_POWER_ONLINE,
> -			  "perf/x86/amd/power:online",
> -			  power_cpu_init, power_cpu_exit);
> +			  "perf/x86/amd/power:online", power_cpu_init,
> +			  power_cpu_exit);
>  
>  	ret = perf_pmu_register(&pmu_class, "power", -1);
>  	if (WARN_ON(ret)) {