| Message ID | 20191213200231.71350-1-venkata.s.dhanalakota@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/perf: Register sysctl path globally | expand |
On Fri, 13 Dec 2019, Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> wrote: > We do not require to register the sysctl paths per instance, > so making registration global. > > v2: make sysctl path register and unregister function driver > specific (Tvrtko and Lucas). > > v3: remove the NULL-check as unregister_sysctl_table is null > safe. (Chris and Lucas) > > 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> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> Chris, I understand that *some* version of this passed IGT, and that the patch series was re-sent rather too many times over a short period of time, but please only push versions that actually pass CI. BR, Jani. > --- > drivers/gpu/drm/i915/i915_pci.c | 9 ++++++++- > drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++---- > drivers/gpu/drm/i915/i915_perf.h | 2 ++ > drivers/gpu/drm/i915/i915_perf_types.h | 1 - > 4 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 877560b1031e..9571611b4b16 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -30,6 +30,7 @@ > #include "display/intel_fbdev.h" > > #include "i915_drv.h" > +#include "i915_perf.h" > #include "i915_globals.h" > #include "i915_selftest.h" > > @@ -1053,7 +1054,12 @@ static int __init i915_init(void) > return 0; > } > > - return pci_register_driver(&i915_pci_driver); > + err = pci_register_driver(&i915_pci_driver); > + if (err) > + return err; > + > + i915_perf_sysctl_register(); > + return 0; > } > > static void __exit i915_exit(void) > @@ -1061,6 +1067,7 @@ static void __exit i915_exit(void) > if (!i915_pci_driver.driver.owner) > return; > > + i915_perf_sysctl_unregister(); > pci_unregister_driver(&i915_pci_driver); > i915_globals_exit(); > } > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 8d2e37949f46..3c163a9d69a9 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) > @@ -4228,7 +4230,7 @@ static struct ctl_table dev_root[] = { > }; > > /** > - * i915_perf_init - initialize i915-perf state on module load > + * i915_perf_init - initialize i915-perf state on module bind > * @i915: i915 device instance > * > * Initializes i915-perf state without exposing anything to userspace. > @@ -4345,7 +4347,6 @@ 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); > > mutex_init(&perf->metrics_lock); > idr_init(&perf->metrics_idr); > @@ -4381,6 +4382,16 @@ static int destroy_config(int id, void *p, void *data) > return 0; > } > > +void i915_perf_sysctl_register(void) > +{ > + sysctl_header = register_sysctl_table(dev_root); > +} > + > +void i915_perf_sysctl_unregister(void) > +{ > + unregister_sysctl_table(sysctl_header); > +} > + > /** > * i915_perf_fini - Counter part to i915_perf_init() > * @i915: i915 device instance > @@ -4395,8 +4406,6 @@ 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); > - > memset(&perf->ops, 0, sizeof(perf->ops)); > perf->i915 = NULL; > } > diff --git a/drivers/gpu/drm/i915/i915_perf.h b/drivers/gpu/drm/i915/i915_perf.h > index 4ceebce72060..882fdd0a7680 100644 > --- a/drivers/gpu/drm/i915/i915_perf.h > +++ b/drivers/gpu/drm/i915/i915_perf.h > @@ -23,6 +23,8 @@ void i915_perf_fini(struct drm_i915_private *i915); > void i915_perf_register(struct drm_i915_private *i915); > void i915_perf_unregister(struct drm_i915_private *i915); > int i915_perf_ioctl_version(void); > +void i915_perf_sysctl_register(void); > +void i915_perf_sysctl_unregister(void); > > int i915_perf_open_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > 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
Quoting Jani Nikula (2019-12-16 10:27:59) > On Fri, 13 Dec 2019, Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> wrote: > > We do not require to register the sysctl paths per instance, > > so making registration global. > > > > v2: make sysctl path register and unregister function driver > > specific (Tvrtko and Lucas). > > > > v3: remove the NULL-check as unregister_sysctl_table is null > > safe. (Chris and Lucas) > > > > 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> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> > > Chris, I understand that *some* version of this passed IGT, and that the > patch series was re-sent rather too many times over a short period of > time, but please only push versions that actually pass CI. CI Bug Log - changes from CI_DRM_7556_full -> Patchwork_15738_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_15738_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_15738_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_15738_full: ### IGT changes ### #### Possible regressions #### * igt@gem_ctx_persistence@bcs0-mixed-process: - shard-iclb: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7556/shard-iclb1/igt@gem_ctx_persistence@bcs0-mixed-process.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15738/shard-iclb3/igt@gem_ctx_persistence@bcs0-mixed-process.html Which is just a known timing issue. Is the version I was looking at. -Chris --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 877560b1031e..9571611b4b16 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -30,6 +30,7 @@ #include "display/intel_fbdev.h" #include "i915_drv.h" +#include "i915_perf.h" #include "i915_globals.h" #include "i915_selftest.h" @@ -1053,7 +1054,12 @@ static int __init i915_init(void) return 0; } - return pci_register_driver(&i915_pci_driver); + err = pci_register_driver(&i915_pci_driver); + if (err) + return err; + + i915_perf_sysctl_register(); + return 0; } static void __exit i915_exit(void) @@ -1061,6 +1067,7 @@ static void __exit i915_exit(void) if (!i915_pci_driver.driver.owner) return; + i915_perf_sysctl_unregister(); pci_unregister_driver(&i915_pci_driver); i915_globals_exit(); } diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 8d2e37949f46..3c163a9d69a9 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) @@ -4228,7 +4230,7 @@ static struct ctl_table dev_root[] = { }; /** - * i915_perf_init - initialize i915-perf state on module load + * i915_perf_init - initialize i915-perf state on module bind * @i915: i915 device instance * * Initializes i915-perf state without exposing anything to userspace. @@ -4345,7 +4347,6 @@ 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); mutex_init(&perf->metrics_lock); idr_init(&perf->metrics_idr); @@ -4381,6 +4382,16 @@ static int destroy_config(int id, void *p, void *data) return 0; } +void i915_perf_sysctl_register(void) +{ + sysctl_header = register_sysctl_table(dev_root); +} + +void i915_perf_sysctl_unregister(void) +{ + unregister_sysctl_table(sysctl_header); +} + /** * i915_perf_fini - Counter part to i915_perf_init() * @i915: i915 device instance @@ -4395,8 +4406,6 @@ 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); - memset(&perf->ops, 0, sizeof(perf->ops)); perf->i915 = NULL; } diff --git a/drivers/gpu/drm/i915/i915_perf.h b/drivers/gpu/drm/i915/i915_perf.h index 4ceebce72060..882fdd0a7680 100644 --- a/drivers/gpu/drm/i915/i915_perf.h +++ b/drivers/gpu/drm/i915/i915_perf.h @@ -23,6 +23,8 @@ void i915_perf_fini(struct drm_i915_private *i915); void i915_perf_register(struct drm_i915_private *i915); void i915_perf_unregister(struct drm_i915_private *i915); int i915_perf_ioctl_version(void); +void i915_perf_sysctl_register(void); +void i915_perf_sysctl_unregister(void); int i915_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_file *file); 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
We do not require to register the sysctl paths per instance, so making registration global. v2: make sysctl path register and unregister function driver specific (Tvrtko and Lucas). v3: remove the NULL-check as unregister_sysctl_table is null safe. (Chris and Lucas) 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> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> --- drivers/gpu/drm/i915/i915_pci.c | 9 ++++++++- drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++---- drivers/gpu/drm/i915/i915_perf.h | 2 ++ drivers/gpu/drm/i915/i915_perf_types.h | 1 - 4 files changed, 23 insertions(+), 6 deletions(-)