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

Guenter Roeck
2020-09-05T16:58:43Z

On 9/5/20 9:41 AM, Naveen Krishna Ch wrote:
> 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.
>

Isn't turbostat supposed to be able to do that ?

Guenter

>>
>> 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
>