Re: FW: [PATCH 5/6] hwmon: amd_energy: dump energy counters via debugfs

Naveen Krishna Ch
2020-09-05T16:41:48Z

Hi Guenter,

> On 9/5/20 7:32 AM, Naveen Krishna Chatradhi wrote:
> > Use seq_printf to capture the core and socket energies under debugfs
> > path in '/sys/kernel/debug/amd_energy/'
> > file cenergy_dump: To print out the core energy counters file
> > senergy_dump: To print out the socket energy counters
> >
> > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
>
> Isn't this a duplicate of other functionality available in the kernel ?
> I'd have to look it up, but I am quite sure that energy values are already available. Besides that, what is the point of duplicating the hwmon attributes ?

Idea is to reduce the latency in gathering all the counters (Rome has
128 cores on 2 sockets) from the user space.
If there is a better way to get this done, please let me know. I shall
implement and resubmit.

>
> Guenter
>
> > ---
> > drivers/hwmon/amd_energy.c | 110
> > +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 110 insertions(+)
> >
> > diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
> > index c294bea56c02..2184bd4510ed 100644
> > --- a/drivers/hwmon/amd_energy.c
> > +++ b/drivers/hwmon/amd_energy.c
> > @@ -8,6 +8,7 @@
> > #include <linux/bits.h>
> > #include <linux/cpu.h>
> > #include <linux/cpumask.h>
> > +#include <linux/debugfs.h>
> > #include <linux/delay.h>
> > #include <linux/device.h>
> > #include <linux/hwmon.h>
> > @@ -20,6 +21,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/sched.h>
> > #include <linux/slab.h>
> > +#include <linux/smp.h>
> > #include <linux/topology.h>
> > #include <linux/types.h>
> >
> > @@ -57,6 +59,8 @@ struct amd_energy_data {
> > int nr_socks;
> > int core_id;
> > char (*label)[10];
> > + u64 *cdump;
> > + u64 *sdump;
> > };
> >
> > static int amd_energy_read_labels(struct device *dev, @@ -329,6
> > +333,108 @@ static int create_accumulate_status_file(struct amd_energy_data *data)
> > &accumulate_attr.attr); }
> >
> > +#ifdef CONFIG_DEBUG_FS
> > +static void dump_on_each_cpu(void *info) {
> > + struct amd_energy_data *data = info;
> > + int cpu = smp_processor_id();
> > +
> > + amd_add_delta(data, cpu, cpu, (long *)&data->cdump[cpu],
> > + ENERGY_CORE_MSR);
> > +}
> > +
> > +static int cenergy_dump_show(struct seq_file *s, void *unused) {
> > + struct amd_energy_data *data = s->private;
> > + struct cpumask *cpus_mask;
> > + int i;
> > +
> > + cpus_mask = kmalloc(sizeof(*cpus_mask), GFP_KERNEL);
> > + memset(data->cdump, 0, (data->nr_cpus) * sizeof(u64));
> > + cpumask_clear(cpus_mask);
> > + for (i = 0; i < data->nr_cpus; i++) {
> > + if (cpu_online(i))
> > + cpumask_set_cpu(i, cpus_mask);
> > + }
> > +
> > + on_each_cpu_mask(cpus_mask, dump_on_each_cpu, data, true);
> > +
> > + for (i = 0; i < data->nr_cpus; i++) {
> > + if (!(i & 3))
> > + seq_printf(s, "Core %3d: ", i);
> > +
> > + seq_printf(s, "%16llu ", data->cdump[i]);
> > + if ((i & 3) == 3)
> > + seq_puts(s, "\n");
> > + }
> > + seq_puts(s, "\n");
> > +
> > + kfree(cpus_mask);
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(cenergy_dump);
> > +
> > +static int senergy_dump_show(struct seq_file *s, void *unused) {
> > + struct amd_energy_data *data = s->private;
> > + int i, cpu;
> > +
> > + for (i = 0; i < data->nr_socks; i++) {
> > + cpu = cpumask_first_and(cpu_online_mask,
> > + cpumask_of_node(i));
> > + amd_add_delta(data, data->nr_cpus + i, cpu,
> > + (long *)&data->sdump[i], ENERGY_PKG_MSR);
> > + seq_printf(s, "Socket %1d: %16llu\n",
> > + i, data->sdump[i]);
> > + }
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(senergy_dump);
> > +
> > +static void dump_debugfs_cleanup(void *ddir) {
> > + debugfs_remove_recursive(ddir);
> > +}
> > +
> > +static int create_dump_file(struct device *dev,
> > + struct amd_energy_data *data) {
> > + struct dentry *debugfs;
> > + char name[] = "amd_energy";
> > +
> > + data->cdump = devm_kcalloc(dev, data->nr_cpus,
> > + sizeof(u64), GFP_KERNEL);
> > + if (!data->cdump)
> > + return -ENOMEM;
> > +
> > + data->sdump = devm_kcalloc(dev, data->nr_socks,
> > + sizeof(u64), GFP_KERNEL);
> > + if (!data->sdump)
> > + return -ENOMEM;
> > +
> > + debugfs = debugfs_create_dir(name, NULL);
> > + if (debugfs) {
> > + debugfs_create_file("cenergy_dump", 0440,
> > + debugfs, data, &cenergy_dump_fops);
> > + debugfs_create_file("senergy_dump", 0440,
> > + debugfs, data, &senergy_dump_fops);
> > + devm_add_action_or_reset(data->hwmon_dev,
> > + dump_debugfs_cleanup, debugfs);
> > + }
> > +
> > + return 0;
> > +}
> > +#else
> > +
> > +static int create_dump_file(struct device *dev,
> > + struct amd_energy_data *data) {
> > + return 0;
> > +}
> > +
> > +#endif //CONFIG_DEBUG_FS
> > +
> > static int amd_energy_probe(struct platform_device *pdev) {
> > struct amd_energy_data *data;
> > @@ -376,6 +482,10 @@ static int amd_energy_probe(struct platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > + ret = create_dump_file(dev, data);
> > + if (ret)
> > + return ret;
> > +
> > return 0;
> > }
> >
> >
Naveen