From: Maneesh Soni <maneesh@in.ibm.com>
To: Greg KH <greg@kroah.com>
Cc: Mike Gorse <mgorse@mgorse.dhs.org>,
linux-kernel@vger.kernel.org, Patrick Mochel <mochel@osdl.org>
Subject: Re: Oops w/sysfs when closing a disconnected usb serial device
Date: Mon, 8 Dec 2003 10:26:38 +0530 [thread overview]
Message-ID: <20031208045638.GA4667@in.ibm.com> (raw)
In-Reply-To: <20031206005644.GA14249@kroah.com>
On Fri, Dec 05, 2003 at 04:56:44PM -0800, Greg KH wrote:
[..]
>
> I agree with this patch. It fixes the usbserial oops for me in my
> testing.
>
> But wait, no, this patch is not good...
>
> I think Mike's patch is correct. Here's the problem (tree simplified
> for this example to make it readable):
> - insert usb device, this creates a sysfs directory something
> like:
> - pci.../usb1/1.0
> - the usbserial driver binds to this device and creates a
> ttyUSB0 directory:
> - pci.../usb1/1.0/ttyUSB0
> - a user opens the ttyUSB0 device node (in /dev) which
> increments the reference count of the kobject that controls
> the ttyUSB0 directory in sysfs.
> - the usb device is removed from the system. In doing this,
> the driver core, and then the kobject code has to delete the
> tree 1.0 directory.
>
> Now at this point, Maneesh, your patch would prevent this directory from
> being removed, until after the ttyUSB0 directory is removed. That's all
> well and good, but what happens if we have another USB device plugged
> into the same place before this happens. Then the USB code would try to
> create the 1.0 directory over again (the directory names are built off
> of the USB topology. but even if they were built off of something
> unique, we would still have a mess...) If that happens, the creation of
> the directory would fail, which is not acceptable.
I see and agree, in this case my patch will create more problems.. Actually the
problem here is _not_ that the directory is removed even though it is in use.
The inherent dentry ref. counting should take care for that and
corresponding dentry will be around as long as there are existing users. The
problem here is parent going away before child and we are trying to remove the
child directory more than once.
> But Mike's patch allows the whole tree from 1.0 on down to be removed,
> and then later, when the ttyUSB0 node is really closed, the directory
> will not tried to be removed. Memory is still cleaned up properly, and
> the kobjects are properly reference counted.
>
> I know in the past I've argued against this kind of patch (sorry scsi
> people), but now in thinking about it a bunch (and sitting though a
> zillion oops messages trying to figure out what's wrong here) I think
> this is the correct fix.
>
> Any other opinions?
>
The only problem with Mike's fix is that it fixes the symptom and does
not fix the real cause. I don't see in which case we can have a NULL
d_inode at that point, checking for that means hiding the real problem.
Probably dump_stack() in sysfs_remove_dir() could bring more light in this
problem.
Maneesh
--
Maneesh Soni
Linux Technology Center,
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696
prev parent reply other threads:[~2003-12-08 4:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-12-01 0:18 Oops w/sysfs when closing a disconnected usb serial device Mike Gorse
2003-12-01 9:38 ` Maneesh Soni
2003-12-01 23:55 ` Mike Gorse
2003-12-02 7:11 ` Maneesh Soni
2003-12-05 23:36 ` Greg KH
2003-12-06 1:38 ` Greg KH
2003-12-06 2:16 ` Mike Gorse
2003-12-07 2:58 ` Greg KH
2003-12-07 3:09 ` Mike Gorse
2003-12-06 0:56 ` Greg KH
2003-12-08 4:56 ` Maneesh Soni [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20031208045638.GA4667@in.ibm.com \
--to=maneesh@in.ibm.com \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorse@mgorse.dhs.org \
--cc=mochel@osdl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).