Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757499AbYCaU56 (ORCPT ); Mon, 31 Mar 2008 16:57:58 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752335AbYCaU5u (ORCPT ); Mon, 31 Mar 2008 16:57:50 -0400 Received: from nf-out-0910.google.com ([64.233.182.187]:33495 "EHLO nf-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752139AbYCaU5t (ORCPT ); Mon, 31 Mar 2008 16:57:49 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:content-disposition:in-reply-to:user-agent; b=gPVFPE8/Lf+SLb7KKuvku4HsGID3n0fSX6pfOrJntAEalTh0JN/5VkQzv/+385j8tqd3yA4JKALTRoEIidZP7zvXzDpJyLzoHxuH8FSGAz/ADms8U7/3vA6Wg16rzcRuXkBd1J2UQw31CpZGce9m5Uo/HhgTS4iy3mzRr7lxKPw= Date: Mon, 31 Mar 2008 16:57:36 -0400 From: Dmitry Torokhov To: Greg KH Cc: Kay Sievers , Linus Torvalds , Bj?rn Steinbrink , Arjan van de Ven , Linux Kernel Mailing List , Johannes Berg , Jiri Kosina Subject: Re: [PATCH] evdev: Release eventual input device grabs when getting disconnected Message-ID: <20080331165456.ZZRA012@mailhub.coreip.homeip.net> References: <20080330184259.GB21375@atjola.homenet> <200803310215.39414.dtor@insightbb.com> <20080331172813.GA11583@kroah.com> <20080331135653.ZZRA012@mailhub.coreip.homeip.net> <20080331204221.GA19953@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080331204221.GA19953@kroah.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5601 Lines: 134 On Mon, Mar 31, 2008 at 01:42:21PM -0700, Greg KH wrote: > On Mon, Mar 31, 2008 at 02:01:20PM -0400, Dmitry Torokhov wrote: > > On Mon, Mar 31, 2008 at 10:28:13AM -0700, Greg KH wrote: > > > On Mon, Mar 31, 2008 at 02:15:39AM -0400, Dmitry Torokhov wrote: > > > > Hi Linus, > > > > > > > > On Sunday 30 March 2008, Linus Torvalds wrote: > > > > > > > > > > On Sun, 30 Mar 2008, Bj?rn Steinbrink wrote: > > > > > > > > > > > > I can't reproduce the bug on my UP box and currently can't afford > > > > > > crashing my SMP box (all the oopses seem to come from SMP kernels, so I > > > > > > guess it needs SMP to crash), so while this doesn't show any new > > > > > > problems, I can't tell whether it actually fixes anything. Testers > > > > > > welcome! > > > > > > > > > > Ok, I applied this because I will do an -rc8 today or tomorrow, but I > > > > > really really hope somebody can figure out what made this all start to > > > > > trigger. It does smell like some core device layer change, because we do > > > > > not seem to have a lot of changes since 2.6.24 in evdev.c and input.c that > > > > > seem relevant. > > > > > > > > > > Greg, are there any refcounting changes that would cause the input devices > > > > > to be free'd earlier or something? > > > > > > > > > > > > > The following commit changed lifetime runes on kobjects breaking input: > > > > > > > > commit 0f4dafc0563c6c49e17fe14b3f5f356e4c4b8806 > > > > Author: Kay Sievers > > > > Date: Wed Dec 19 01:40:42 2007 +0100 > > > > > > > > Kobject: auto-cleanup on final unref > > > > > > > > We save the current state in the object itself, so we can do proper > > > > cleanup when the last reference is dropped. > > > > > > > > If the initial reference is dropped, the object will be removed from > > > > sysfs if needed, if an "add" event was sent, "remove" will be send, and > > > > the allocated resources are released. > > > > > > > > This allows us to clean up some driver core usage as well as allowing us > > > > to do other such changes to the rest of the kernel. > > > > > > > > Signed-off-by: Kay Sievers > > > > Signed-off-by: Greg Kroah-Hartman > > > > > > > > Before we dropped reference to kobject's parent only when child kobject > > > > was released (in kobject_cleanup). The changeset above moves the release > > > > to kobject_del() which is way too early in my opinion. The kobject is only > > > > marked for deletion at that time, not really deleted. > > > > > > It was "deleted" from sysfs, and should have never been used again by > > > any callers. If the reference count was dropped to zero with this call, > > > it would be cleaned up as well, it seems that you were assuming that it > > > would not be? Perhaps you just need to grab another reference as this > > > would have caused you problems without this change anyway, but without > > > slab debugging, you never saw it. > > > > > > > Greg, please look at the change again. Before kobject_put(kobj->parent) > > was done in kobject_cleanup() and so the parent would only be freed when > > all its children are gone. Now parent is deleted early, even if its > > children are still referenced by other users. This is lifetime rule > > change and should really be announced as such. > > Ugh, this was done because of scsi, they required that if you really > were deleting the parent, you wanted it gone. > Gone from sysfs or gone from memory? > > If this change it intentional and is here to stay then I will just grab > > the references myself, although I wonder what else might be broken by > > it. > > Yes, if you need those references, you are going to have to hold on for > them, the kobject layer will not keep them around. It now is a "does > what you ask for" type model :) > > I fail to see where this affects the input code though, in glancing at > it, it looks like you are doing things properly. Kay, any thoughts > here, I think you looked at the kobject input layer interaction in the > past. > Ok, I really liked the old behavior better, but if it is to stay then we need something like this (not for inclusion yet as mousedev and joydev need to be adjusted as well): Signed-off-by: Dmitry Torokhov --- drivers/input/evdev.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) Index: linux/drivers/input/evdev.c =================================================================== --- linux.orig/drivers/input/evdev.c +++ linux/drivers/input/evdev.c @@ -124,6 +124,7 @@ static void evdev_free(struct device *de { struct evdev *evdev = container_of(dev, struct evdev, dev); + input_put_device(evdev->handle.dev); kfree(evdev); } @@ -853,9 +854,6 @@ static void evdev_cleanup(struct evdev * evdev_hangup(evdev); evdev_remove_chrdev(evdev); - if (evdev->grab) - evdev_ungrab(evdev, evdev->grab); - /* evdev is marked dead so no one else accesses evdev->open */ if (evdev->open) { input_flush_device(handle, NULL); @@ -896,7 +894,7 @@ static int evdev_connect(struct input_ha evdev->exist = 1; evdev->minor = minor; - evdev->handle.dev = dev; + evdev->handle.dev = input_get_device(dev); evdev->handle.name = evdev->name; evdev->handle.handler = handler; evdev->handle.private = evdev; -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/