From patchwork Thu Nov 21 10:00:32 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 3217221 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5067D9F3A0 for ; Thu, 21 Nov 2013 10:00:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B7EB62077F for ; Thu, 21 Nov 2013 10:00:40 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 7A96420738 for ; Thu, 21 Nov 2013 10:00:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5AAACFB878; Thu, 21 Nov 2013 02:00:37 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-lb0-f176.google.com (mail-lb0-f176.google.com [209.85.217.176]) by gabe.freedesktop.org (Postfix) with ESMTP id E9DE4FCC1D for ; Thu, 21 Nov 2013 02:00:33 -0800 (PST) Received: by mail-lb0-f176.google.com with SMTP id x18so2762208lbi.7 for ; Thu, 21 Nov 2013 02:00:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=WV2QpnQseT4I5egc/Wk15hlx1fhuwo45L5XuTwgUp7Q=; b=XAo/nveE8+Tjdz72owT5QRyc0wssmMEke+ipynEmQ1d/1BeWty5WCgVvEnFIRtKsfK HjkwbVtdjgIcU68rgce4OH95MvgK6K3l7dT2ULD11bDnBtcxdcdjCCVEEgGeSykT7NDW TFRxRUjUGRuDFLpzczhEO6gz18n5ZDXXPKVql2ARMiJTKd7ZDqoxEBcCa9sCkiAgi/3w HfVYnY1kHTgYNwgF9cuX7kDkbV4k8tvS0TXMrh8f9gpppbtScyDHl3M9VQEXLXnvwsP2 gWm9n2lMZHDJ7J4vQ7mqXa4qoi1GSttvhKEIdPGrOV+xB8rLHIHnwAN0TX0RFfxjZ5kg 4PDg== MIME-Version: 1.0 X-Received: by 10.152.225.161 with SMTP id rl1mr4280621lac.5.1385028032585; Thu, 21 Nov 2013 02:00:32 -0800 (PST) Received: by 10.114.232.133 with HTTP; Thu, 21 Nov 2013 02:00:32 -0800 (PST) In-Reply-To: References: <1384998664-5680-1-git-send-email-airlied@gmail.com> <20131121092255.GL27344@phenom.ffwll.local> Date: Thu, 21 Nov 2013 11:00:32 +0100 Message-ID: Subject: Re: [PATCH] drm/sysfs: fix hotplug regression since lifetime changes From: David Herrmann To: Dave Airlie Cc: Greg KH , dri-devel X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development 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 X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi On Thu, Nov 21, 2013 at 10:58 AM, Dave Airlie wrote: > On Thu, Nov 21, 2013 at 7:49 PM, David Herrmann wrote: >> Hi >> >> On Thu, Nov 21, 2013 at 10:25 AM, Dave Airlie wrote: >>> On Thu, Nov 21, 2013 at 7:22 PM, 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(). >>> >>> Possibly but how can we do that? since minor->kdev is something we >>> just created 2 lines earlier >>> unless there is another create function I should be calling, I don't >>> see a device_create_with_type. >> >> See device_create_groups_vargs() in drivers/base/core.c. Just copy the >> code from it and do device initialization yourself. device_create() is >> only a wrapper around kzalloc()+device_register() anyway. >> > > It does seem a bit like cut-n-paste magic to me to do that, > > It does however seem to be the accepted norm. This should do it (untested!): diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index 1a35ea5..cb951b2 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -489,6 +489,11 @@ void drm_sysfs_hotplug_event(struct drm_device *dev) } EXPORT_SYMBOL(drm_sysfs_hotplug_event); +static void drm_sysfs_release(struct device *dev) +{ + kfree(dev); +} + /** * drm_sysfs_device_add - adds a class device to sysfs for a character driver * @dev: DRM device to be added @@ -501,6 +506,7 @@ EXPORT_SYMBOL(drm_sysfs_hotplug_event); int drm_sysfs_device_add(struct drm_minor *minor) { char *minor_str; + int r; if (minor->type == DRM_MINOR_CONTROL) minor_str = "controlD%d"; @@ -509,14 +515,35 @@ int drm_sysfs_device_add(struct drm_minor *minor) else minor_str = "card%d"; - minor->kdev = device_create(drm_class, minor->dev->dev, - MKDEV(DRM_MAJOR, minor->index), - minor, minor_str, minor->index); - if (IS_ERR(minor->kdev)) { - DRM_ERROR("device create failed %ld\n", PTR_ERR(minor->kdev)); - return PTR_ERR(minor->kdev); + minor->kdev = kzalloc(sizeof(*minor->kdev), GFP_KERNEL); + if (!minor->dev) { + r = -ENOMEM; + goto error; } + + device_initialize(minor->kdev); + minor->kdev->devt = MKDEV(DRM_MAJOR, minor->index); + minor->kdev->class = drm_class; + minor->kdev->type = &drm_sysfs_device_minor; + minor->kdev->parent = minor->dev->dev; + minor->kdev->groups = NULL; + minor->kdev->release = drm_sysfs_release; + dev_set_drvdata(minor->kdev, minor); + + r = dev_set_name(minor->kdev, minor_str, minor->index); + if (r < 0) + goto error; + + r = device_add(minor->kdev); + if (r < 0) + goto error; + return 0; + +error: + DRM_ERROR("device create failed %ld\n", PTR_ERR(minor->kdev)); + put_device(minor->kdev); + return r; } /**