Re: [PATCH] perf/cxlpmu: Fix allocation argument order and minor formatting issues

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Mon, 23 Jun 2025 11:44:11 -0700
Alok Tiwari <alok.a.tiwari@oracle.com> wrote:

> Correct the argument order in devm_kcalloc() to follow the conventional
> count, size form to avoid any confusion or bugs.
> Also fix a formatting issue in the devm_kasprintf() call by removing a
> stray newline and fix a duplicated word in a comment.
> 
> Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Hi Alok,

This is a bit of an 'and' patch which usually means it should
be split up.  The devm_kcalloc() is a fix (sort of anyway)
which the others changes aren't.

All the actual changes are good, I'd just prefer this as a little series
of patches that do one thing each.  The first being the devm_kcalloc()
parameter ordering.

Thanks,

Jonathan



> ---
>  drivers/perf/cxl_pmu.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index d6693519eaee2..fb0b29f149807 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -113,7 +113,7 @@ struct cxl_pmu_info {
>  
>  /*
>   * All CPMU counters are discoverable via the Event Capabilities Registers.
> - * Each Event Capability register contains a a VID / GroupID.
> + * Each Event Capability register contains a VID / GroupID.
>   * A counter may then count any combination (by summing) of events in
>   * that group which are in the Supported Events Bitmask.
>   * However, there are some complexities to the scheme.
> @@ -834,8 +834,8 @@ static int cxl_pmu_probe(struct device *dev)
>  	if (rc)
>  		return rc;
>  
> -	info->hw_events = devm_kcalloc(dev, sizeof(*info->hw_events),
> -				       info->num_counters, GFP_KERNEL);
> +	info->hw_events = devm_kcalloc(dev, info->num_counters,
> +				       sizeof(*info->hw_events), GFP_KERNEL);
>  	if (!info->hw_events)
>  		return -ENOMEM;
>  
> @@ -873,7 +873,7 @@ static int cxl_pmu_probe(struct device *dev)
>  		return rc;
>  	irq = rc;
>  
> -	irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_overflow\n", dev_name);
> +	irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_overflow", dev_name);
>  	if (!irq_name)
>  		return -ENOMEM;
>