[Intel-gfx] [PATCH] drm/i915/pmu: Do not use colon characters in PMU names

[Tvrtko Ursulin]

On 10/01/2020 11:32, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-01-10 11:27:55)
>>
>> On 10/01/2020 11:21, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2020-01-10 11:11:26)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>
>>>> We use PCI device path in the registered PMU name in order to distinguish
>>>> between multiple GPUs. But since tools/perf reserves a special meaning to
>>>> the colon character we need to transliterate them to something else. We
>>>> choose a dash.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>> Reported-by: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
>>>> Fixes: 05488673a4d4 ("drm/i915/pmu: Support multiple GPUs")
>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>>> Cc: Andi Kleen <ak at linux.intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_pmu.c | 14 ++++++++++++--
>>>>    1 file changed, 12 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>>>> index f3ef6700a5f2..ecbd0e1f1a90 100644
>>>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>>>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>>>> @@ -1117,12 +1117,22 @@ void i915_pmu_register(struct drm_i915_private *i915)
>>>>           hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>>>           pmu->timer.function = i915_sample;
>>>>    
>>>> -       if (!is_igp(i915))
>>>> +       if (!is_igp(i915)) {
>>>>                   pmu->name = kasprintf(GFP_KERNEL,
>>>>                                         "i915-%s",
>>>>                                         dev_name(i915->drm.dev));
>>>> -       else
>>>> +               if (pmu->name) {
>>>
>>> /* tools/perf reserves colons as special. */
>>> strreplace(pmu->name, ':', '-');
>>
>> I didn't know this exists, thanks.
>>
>>> I worry because the err_idx pointed to the '-'. We may have to use _
>>
>> What is err_idx? But yes.. it would had served me well to test before
>> sending. :) I just find identifiers with a mix of underscores and dashes
>> so visually unappealing. :(
> 
> event syntax error: 'i915-0000:00:02.0/bcs0-busy/'
>                           \___ parser error
> 
> The parser sets err_idx on the character it failed at, and the error
> message includes it. So unless we lost whitespace in all the cutting and
> pasting, that says it barfed at '-'

Oh right, interesting that it has no problem with a dash in event name. 
In v2 full event string is:

   i915_0000_00_02.0/vcs0-busy/

A bit ugly but seems to work.

Regards,

Tvrtko