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

[David Herrmann]

Hi

On Thu, Nov 21, 2013 at 4:24 PM, Greg KH <gregkh at 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 at virtuosugeek.org>
>> > Signed-off-by: Dave Airlie <airlied at 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.

Thanks
David