Re: [Intel-gfx] [PATCH 1/2] drm/i915/perf: Register sysctl path globally

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: chris.p.wilson@intel.com
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/perf: Register sysctl path globally
Date: Wed, 11 Dec 2019 16:39:05 +0000	[thread overview]
Message-ID: <96a54ef7-86d2-0e9b-e0bc-20878883cfea@linux.intel.com> (raw)
In-Reply-To: <d7b95242-51d0-f052-aabb-fae4ab66e9c3@linux.intel.com>

On 11/12/2019 16:31, Tvrtko Ursulin wrote:
> On 11/12/2019 16:07, Venkata Sandeep Dhanalakota wrote:
>> We do not require to register the sysctl paths per instance,
>> so making registration global.
>>
>> Cc: Sudeep Dutt <sudeep.dutt@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Venkata Sandeep Dhanalakota 
>> <venkata.s.dhanalakota@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c       | 10 ++++++++--
>>   drivers/gpu/drm/i915/i915_perf_types.h |  1 -
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>> b/drivers/gpu/drm/i915/i915_perf.c
>> index 8d2e37949f46..426d04214a5d 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -387,6 +387,8 @@ struct i915_oa_config_bo {
>>       struct i915_vma *vma;
>>   };
>> +static struct ctl_table_header *sysctl_header;
>> +
>>   static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer 
>> *hrtimer);
>>   void i915_oa_config_release(struct kref *ref)
>> @@ -4345,7 +4347,8 @@ void i915_perf_init(struct drm_i915_private *i915)
>>           oa_sample_rate_hard_limit = 1000 *
>>               (RUNTIME_INFO(i915)->cs_timestamp_frequency_khz / 2);
>> -        perf->sysctl_header = register_sysctl_table(dev_root);
>> +        if (!sysctl_header)
>> +            sysctl_header = register_sysctl_table(dev_root);
>>           mutex_init(&perf->metrics_lock);
>>           idr_init(&perf->metrics_idr);
>> @@ -4395,7 +4398,10 @@ void i915_perf_fini(struct drm_i915_private *i915)
>>       idr_for_each(&perf->metrics_idr, destroy_config, perf);
>>       idr_destroy(&perf->metrics_idr);
>> -    unregister_sysctl_table(perf->sysctl_header);
>> +    if (sysctl_header) {
>> +        unregister_sysctl_table(sysctl_header);
>> +        sysctl_header = NULL;
>> +    }
> 
> I am not sure if this could be racy with manual unbind from sysfs. Does 
> PCI core serialize for us?

Actually with two devices you also need to reference count it since you 
don't want removal of the first device to remove the node but last.

> Regards,
> 
> Tvrtko
> 
>>       memset(&perf->ops, 0, sizeof(perf->ops));
>>       perf->i915 = NULL;
>> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h 
>> b/drivers/gpu/drm/i915/i915_perf_types.h
>> index 74ddc20a0d37..45e581455f5d 100644
>> --- a/drivers/gpu/drm/i915/i915_perf_types.h
>> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
>> @@ -380,7 +380,6 @@ struct i915_perf {
>>       struct drm_i915_private *i915;
>>       struct kobject *metrics_kobj;
>> -    struct ctl_table_header *sysctl_header;
>>       /*
>>        * Lock associated with adding/modifying/removing OA configs
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx