Re: [PATCH] drm/sysfs: fix hotplug regression since lifetime changes

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Nov 21, 2013 at 04:40:29PM +0100, David Herrmann wrote:
> Hi
> 
> On Thu, Nov 21, 2013 at 4:24 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Nov 21, 2013 at 10:22:55AM +0100, Daniel Vetter wrote:
> >> On Thu, Nov 21, 2013 at 11:51:04AM +1000, Dave Airlie wrote:
> >> > 5bdebb183c9702a8c57a01dff09337be3de337a6 changed the lifetimes, but it
> >> > also meant we no longer set the device_type field properly, so the
> >> > hotplug events in userspace weren't fully formed enough for drivers to care.
> >> >
> >> > Reported-by: Jesse Barnes <jbarnes@virtuosugeek.org>
> >> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> >> > ---
> >> >  drivers/gpu/drm/drm_sysfs.c | 1 +
> >> >  1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> >> > index 1a35ea5..c6a3902 100644
> >> > --- a/drivers/gpu/drm/drm_sysfs.c
> >> > +++ b/drivers/gpu/drm/drm_sysfs.c
> >> > @@ -516,6 +516,7 @@ int drm_sysfs_device_add(struct drm_minor *minor)
> >> >             DRM_ERROR("device create failed %ld\n", PTR_ERR(minor->kdev));
> >> >             return PTR_ERR(minor->kdev);
> >> >     }
> >> > +   minor->kdev->type = &drm_sysfs_device_minor;
> >>
> >> Isn't this one of the sysfs races Greg is fighting against? At least from
> >> a cursor read through the driver core it looks like we'd better set the
> >> dev->type before we set it up with device_create().
> >>
> >> Cc'ing Greg for insight.
> >
> > No, setting the type at this point is fine, I don't see the race, care
> > to explain what I'm missing?
> >
> > But, if you want to be "correct" you can just create the device
> > structure yourself, set the fields (including the type) and then call
> > device_register() yourself.  Look at usb_create_ep_devs() for an
> > example of how to do this.
> 
> We already pushed a fix:
> http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-fixes&id=760c960bd6880cf22a57c0af9ff60c96250aad39
> 
> The race is the same as with sysfs attributes. If we set the device
> type _after_ device_add(), the initial uevent might not have
> DEVTYPE=drm_minor set, thus, slip through any
> udev_*_match_subsystem_devtype() filters.

Ah, that makes sense, nice fix.

greg k-h