Re: [RFC PATCH] Add support for RAPL MSRs in KVM/Qemu

["Anthony Harivel" <aharivel@redhat.com>]

Philippe Mathieu-Daudé, May 19, 2023 at 13:32:

Hi Philippe,

> > +/*
> > + * Package statistic
> > + * @ e_start: package energy counter before the sleep
> > + * @ e_end: package energy counter after the sleep
> > + * @ e_delta: delta of package energy counter
> > + * @ e_ratio: store the energy ratio of non-vCPU thread
> > + * @ nb_vcpu: number of vCPU running on this package
> > + */
> > +struct packge_energy_stat {
>
> "package"

My bad..
This will be corrected.

>
> > +    uint64_t e_start;
> > +    uint64_t e_end;
> > +    uint64_t e_delta;
> > +    uint64_t e_ratio;
> > +    unsigned int nb_vcpu;
> > +};
> > +
> > +typedef struct thread_stat thread_stat;
> > +typedef struct packge_energy_stat package_energy_stat;
> > +
> > +uint64_t read_msr(uint32_t reg, unsigned int cpu_id);
> > +void delta_ticks(thread_stat *thd_stat, int i);
> > +unsigned int get_maxcpus(unsigned int package_num);
> > +int read_thread_stat(struct thread_stat *thread, int pid, int index);
> > +pid_t *get_thread_ids(pid_t pid, int *num_threads);
> > +double get_ratio(package_energy_stat *pkg_stat,
> > +                        thread_stat *thd_stat,
> > +                        int maxticks, int i);
>
> Would prefixing these declarations with 'vmsr_' provide
> a clearer API? Otherwise, maybe this isn't the best header
> to declare them.

I agree with you this lack the prefixing you mention for better API clarity. 
I will correct that.

Thanks !

> > +
> > +#endif /* VMSR_ENERGY_H */