From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg KH Subject: Re: [PATCH] drm/sysfs: fix hotplug regression since lifetime changes Date: Thu, 21 Nov 2013 07:48:40 -0800 Message-ID: <20131121154840.GA8210@kroah.com> References: <1384998664-5680-1-git-send-email-airlied@gmail.com> <20131121092255.GL27344@phenom.ffwll.local> <20131121152427.GC1988@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) by gabe.freedesktop.org (Postfix) with ESMTP id 770B4105AD7 for ; Thu, 21 Nov 2013 07:48:10 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: David Herrmann Cc: "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.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 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 > >> > Signed-off-by: Dave Airlie > >> > --- > >> > 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