| Message ID | 1383130745-32165-1-git-send-email-treding@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/10/30 Thierry Reding <thierry.reding@gmail.com>: > device_unregister() already drops its reference to the struct device, so > explicitly calling put_device() before device_unregister() can cause the > device to have been freed before it can be unregistered. > > Signed-off-by: Thierry Reding <treding@nvidia.com> I started investigating this problem yesterday and reached the same conclusion. The connector path can be easily reproduced on i915.ko: get a machine that has an eDP panel, physically disconnect the panel, boot the machine, "modprobe i915" and watch the segfault. Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> I didn't really bisect, but I believe this is probably a regression from "drm/sysfs: sort out minor and connector device object lifetimes". And kudos to whoever invented CONFIG_DEBUG_KOBJECT :) > --- > drivers/gpu/drm/drm_sysfs.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index dae42c7..db1c8f9 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -439,7 +439,6 @@ err_out_files: > device_remove_file(connector->kdev, &connector_attrs_opt1[i]); > for (i = 0; i < attr_cnt; i++) > device_remove_file(connector->kdev, &connector_attrs[i]); > - put_device(connector->kdev); > device_unregister(connector->kdev); > > out: > @@ -472,7 +471,6 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) > for (i = 0; i < ARRAY_SIZE(connector_attrs); i++) > device_remove_file(connector->kdev, &connector_attrs[i]); > sysfs_remove_bin_file(&connector->kdev->kobj, &edid_attr); > - put_device(connector->kdev); > device_unregister(connector->kdev); > connector->kdev = NULL; > } > -- > 1.8.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Oct 30, 2013 at 11:59:05AM +0100, Thierry Reding wrote: > device_unregister() already drops its reference to the struct device, so > explicitly calling put_device() before device_unregister() can cause the > device to have been freed before it can be unregistered. > > Signed-off-by: Thierry Reding <treding@nvidia.com> Thanks for fixing this. It was driving me nuts. Tested-by: Ben Widawsky <ben@bwidawsk.net> [snip]
On Wed, Oct 30, 2013 at 02:05:02PM -0200, Paulo Zanoni wrote: > 2013/10/30 Thierry Reding <thierry.reding@gmail.com>: > > device_unregister() already drops its reference to the struct device, so > > explicitly calling put_device() before device_unregister() can cause the > > device to have been freed before it can be unregistered. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > I started investigating this problem yesterday and reached the same > conclusion. The connector path can be easily reproduced on i915.ko: > get a machine that has an eDP panel, physically disconnect the panel, > boot the machine, "modprobe i915" and watch the segfault. > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > I didn't really bisect, but I believe this is probably a regression > from "drm/sysfs: sort out minor and connector device object > lifetimes". Yes, I think that's the one that broke it. Thierry
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index dae42c7..db1c8f9 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -439,7 +439,6 @@ err_out_files: device_remove_file(connector->kdev, &connector_attrs_opt1[i]); for (i = 0; i < attr_cnt; i++) device_remove_file(connector->kdev, &connector_attrs[i]); - put_device(connector->kdev); device_unregister(connector->kdev); out: @@ -472,7 +471,6 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) for (i = 0; i < ARRAY_SIZE(connector_attrs); i++) device_remove_file(connector->kdev, &connector_attrs[i]); sysfs_remove_bin_file(&connector->kdev->kobj, &edid_attr); - put_device(connector->kdev); device_unregister(connector->kdev); connector->kdev = NULL; }
device_unregister() already drops its reference to the struct device, so explicitly calling put_device() before device_unregister() can cause the device to have been freed before it can be unregistered. Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/gpu/drm/drm_sysfs.c | 2 -- 1 file changed, 2 deletions(-)