On Wed, Dec 11, 2019 at 09:13:18AM -0800, Venkata Sandeep Dhanalakota wrote:
On 19/12/11 04:39, Tvrtko Ursulin wrote: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@xxxxxxxxx> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > > Signed-off-by: Venkata Sandeep Dhanalakota > > <venkata.s.dhanalakota@xxxxxxxxx> > > --- > > 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.Apparently this is not called during module exit, using krefs is way go to or have some helper which are called during module init/exit. void i915_perf_sysctl_register() { sysctl_header = register_sysctl_table(dev_root); } void i915_perf_sysctl_unregister() { unregister_sysctl_table(sysctl_header); }
yeah, since you are moving this to be global, I don't think it belongs to the device, but rather to the module. So, IMO it makes more sense to use this approach than kref/kunref on bind/unbind. Lucas De Marchi
> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
