| Message ID | 20200323144950.3018436-3-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm_device managed resources, v5 | expand |
On Mon, 23 Mar 2020, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > For two reasons: > > - The driver core clears this already for us after we're unloaded in > __device_release_driver(). > > - It's way too late, the drm_device ->release callback might massively > outlive the underlying physical device, since a drm_device can't be *can be*? > kept alive by open drm_file or well really anything else userspace > is still hanging onto. So if we clear this ourselves, we should > clear it in the pci ->remove callback, not in the drm_device > ->relase callback. ->release Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Looking at git history this was fixed in the driver core with > > commit 0998d0631001288a5974afc0b2a5f568bcdecb4d > Author: Hans de Goede <hdegoede@redhat.com> > Date: Wed May 23 00:09:34 2012 +0200 > > device-core: Ensure drvdata = NULL when no driver is bound > > v2: Cite the core fix in the commit message (Chris). > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 48ba37e35bea..4792051e9e2e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -909,9 +909,6 @@ static void i915_driver_destroy(struct drm_i915_private *i915) > > drm_dev_fini(&i915->drm); > kfree(i915); > - > - /* And make sure we never chase our dangling pointer from pci_dev */ > - pci_set_drvdata(pdev, NULL); > } > > /**
On Wed, 25 Mar 2020, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Mon, 23 Mar 2020, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> For two reasons: >> >> - The driver core clears this already for us after we're unloaded in >> __device_release_driver(). >> >> - It's way too late, the drm_device ->release callback might massively >> outlive the underlying physical device, since a drm_device can't be > > *can be*? > >> kept alive by open drm_file or well really anything else userspace >> is still hanging onto. So if we clear this ourselves, we should >> clear it in the pci ->remove callback, not in the drm_device >> ->relase callback. > > ->release > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> Oops, drivers/gpu/drm/i915/i915_drv.c: In function ‘i915_driver_destroy’: drivers/gpu/drm/i915/i915_drv.c:911:18: error: unused variable ‘pdev’ [-Werror=unused-variable] struct pci_dev *pdev = i915->drm.pdev; ^~~~ > >> >> Looking at git history this was fixed in the driver core with >> >> commit 0998d0631001288a5974afc0b2a5f568bcdecb4d >> Author: Hans de Goede <hdegoede@redhat.com> >> Date: Wed May 23 00:09:34 2012 +0200 >> >> device-core: Ensure drvdata = NULL when no driver is bound >> >> v2: Cite the core fix in the commit message (Chris). >> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index 48ba37e35bea..4792051e9e2e 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -909,9 +909,6 @@ static void i915_driver_destroy(struct drm_i915_private *i915) >> >> drm_dev_fini(&i915->drm); >> kfree(i915); >> - >> - /* And make sure we never chase our dangling pointer from pci_dev */ >> - pci_set_drvdata(pdev, NULL); >> } >> >> /**
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 48ba37e35bea..4792051e9e2e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -909,9 +909,6 @@ static void i915_driver_destroy(struct drm_i915_private *i915) drm_dev_fini(&i915->drm); kfree(i915); - - /* And make sure we never chase our dangling pointer from pci_dev */ - pci_set_drvdata(pdev, NULL); } /**
For two reasons: - The driver core clears this already for us after we're unloaded in __device_release_driver(). - It's way too late, the drm_device ->release callback might massively outlive the underlying physical device, since a drm_device can't be kept alive by open drm_file or well really anything else userspace is still hanging onto. So if we clear this ourselves, we should clear it in the pci ->remove callback, not in the drm_device ->relase callback. Looking at git history this was fixed in the driver core with commit 0998d0631001288a5974afc0b2a5f568bcdecb4d Author: Hans de Goede <hdegoede@redhat.com> Date: Wed May 23 00:09:34 2012 +0200 device-core: Ensure drvdata = NULL when no driver is bound v2: Cite the core fix in the commit message (Chris). Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 --- 1 file changed, 3 deletions(-)