lkml.org 
[lkml]   [2021]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 1/1] dmaengine: idxd: Add IDXD performance monitor support
From
Date
Hi Vinod,

On 4/21/2021 1:01 AM, Vinod Koul wrote:
> On 20-04-21, 09:13, Zanussi, Tom wrote:
>> Hi Vinod,
>>
>> On 4/20/2021 6:11 AM, Vinod Koul wrote:
>>> On 03-04-21, 11:45, Tom Zanussi wrote:
>>>
>>>> +config INTEL_IDXD_PERFMON
>>>> + bool "Intel Data Accelerators performance monitor support"
>>>> + depends on INTEL_IDXD
>>>> + default y
>>>
>>> default y..?
>>
>> Will change to n.
>
> That is the default, you may drop this line
>

OK, will do.

>>
>>>
>>>> /* IDXD software descriptor */
>>>> @@ -369,4 +399,19 @@ int idxd_cdev_get_major(struct idxd_device *idxd);
>>>> int idxd_wq_add_cdev(struct idxd_wq *wq);
>>>> void idxd_wq_del_cdev(struct idxd_wq *wq);
>>>> +/* perfmon */
>>>> +#ifdef CONFIG_INTEL_IDXD_PERFMON
>>>
>>> maybe use IS_ENABLED()
>
> ?
>

Yes, I'll change to this.

>>>
>>>> @@ -556,6 +562,8 @@ static int __init idxd_init_module(void)
>>>> for (i = 0; i < IDXD_TYPE_MAX; i++)
>>>> idr_init(&idxd_idrs[i]);
>>>> + perfmon_init();
>>>> +
>>>> err = idxd_register_bus_type();
>>>> if (err < 0)
>>>> return err;
>>>> @@ -589,5 +597,6 @@ static void __exit idxd_exit_module(void)
>>>> pci_unregister_driver(&idxd_pci_driver);
>>>> idxd_cdev_remove();
>>>> idxd_unregister_bus_type();
>>>> + perfmon_exit();
>>>
>>> Ideally would make sense to add perfmon module first and then add use in
>>> idxd..
>>>
>>
>> OK, I'll separate this out into a separate patch.
>>
>>>> +static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
>>>> + char *buf);
>>>> +
>>>> +static cpumask_t perfmon_dsa_cpu_mask;
>>>> +static bool cpuhp_set_up;
>>>> +static enum cpuhp_state cpuhp_slot;
>>>> +
>>>> +static DEVICE_ATTR_RO(cpumask);
>>>
>>> Pls document these new attributes added
>
> ?
>

Yes, I'll add comments to all the attributes (also I'm assuming they don't need to be documented elsewhere).

>>>
>>>> +static int perfmon_collect_events(struct idxd_pmu *idxd_pmu,
>>>> + struct perf_event *leader,
>>>> + bool dogrp)
>>>
>>> dogrp..?
>>>
>>
>> Yeah, bad name, first thought on seeing it is always 'dog'. ;-)
>
> Yep, that was my first read as well... i guess it would be better as
> do_grp
>

Yep, will make change it to that.

Thanks,

Tom

\
 
 \ /
  Last update: 2022-09-17 16:10    [W:0.048 / U:2.912 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site