Re: [PATCH v4 01/20] xen/pmstat: consolidate code into pmstat.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.05.2025 11:16, Penny Zheng wrote:
> We move the following functions into drivers/acpi/pmstat.c, as they
> are all designed for performance statistic:
> - cpufreq_residency_update()
> - cpufreq_statistic_reset()
> - cpufreq_statistic_update()
> - cpufreq_statistic_init()
> - cpufreq_statistic_exit()

You say move, but ...

> +int cpufreq_statistic_init(unsigned int cpu)
> +{
> +    unsigned int i, count;
> +    struct pm_px *pxpt;
> +    const struct processor_pminfo *pmpt = processor_pminfo[cpu];
> +    spinlock_t *cpufreq_statistic_lock = &per_cpu(cpufreq_statistic_lock, 
> cpu);
> +
> +    spin_lock_init(cpufreq_statistic_lock);
> +
> +    if ( !pmpt )
> +        return -EINVAL;
> +
> +    spin_lock(cpufreq_statistic_lock);
> +
> +    pxpt = per_cpu(cpufreq_statistic_data, cpu);
> +    if ( pxpt )
> +    {
> +        spin_unlock(cpufreq_statistic_lock);
> +        return 0;
> +    }
> +
> +    count = pmpt->perf.state_count;
> +
> +    pxpt = xzalloc(struct pm_px);
> +    if ( !pxpt )
> +    {
> +        spin_unlock(cpufreq_statistic_lock);
> +        return -ENOMEM;
> +    }
> +    per_cpu(cpufreq_statistic_data, cpu) = pxpt;

... how come this moved up (back) here, when it was ...

> -int cpufreq_statistic_init(unsigned int cpu)
> -{
> -    uint32_t i, count;
> -    struct pm_px *pxpt;
> -    const struct processor_pminfo *pmpt = processor_pminfo[cpu];
> -    spinlock_t *cpufreq_statistic_lock = &per_cpu(cpufreq_statistic_lock, 
> cpu);
> -
> -    spin_lock_init(cpufreq_statistic_lock);
> -
> -    if ( !pmpt )
> -        return -EINVAL;
> -
> -    spin_lock(cpufreq_statistic_lock);
> -
> -    pxpt = per_cpu(cpufreq_statistic_data, cpu);
> -    if ( pxpt ) {
> -        spin_unlock(cpufreq_statistic_lock);
> -        return 0;
> -    }
> -
> -    count = pmpt->perf.state_count;
> -
> -    pxpt = xzalloc(struct pm_px);
> -    if ( !pxpt ) {
> -        spin_unlock(cpufreq_statistic_lock);
> -        return -ENOMEM;
> -    }
> -
> -    pxpt->u.trans_pt = xzalloc_array(uint64_t, count * count);
> -    if (!pxpt->u.trans_pt) {
> -        xfree(pxpt);
> -        spin_unlock(cpufreq_statistic_lock);
> -        return -ENOMEM;
> -    }
> -
> -    pxpt->u.pt = xzalloc_array(struct pm_px_val, count);
> -    if (!pxpt->u.pt) {
> -        xfree(pxpt->u.trans_pt);
> -        xfree(pxpt);
> -        spin_unlock(cpufreq_statistic_lock);
> -        return -ENOMEM;
> -    }
> -
> -    pxpt->u.total = count;
> -    pxpt->u.usable = count - pmpt->perf.platform_limit;
> -
> -    for ( i = 0; i < count; i++ )
> -        pxpt->u.pt[i].freq = pmpt->perf.states[i].core_frequency;
> -
> -    pxpt->prev_state_wall = NOW();
> -    pxpt->prev_idle_wall = get_cpu_idle_time(cpu);
> -
> -    per_cpu(cpufreq_statistic_data, cpu) = pxpt;

... moved down here just a few weeks ago? See a1ce987411f61 ("cpufreq: don't
leave stale statistics pointer"). I'll make yet another patch, but you want
to be more careful when re-basing. The other recent change, c6e0a55396233
("cpufreq: use existing local var in cpufreq_statistic_init()"), was also
lost.

Jan