On Wed, 15 Mar 2006, Paul Fulghum wrote:

> If my reading of the code is correct, an existing symlink
> does keep the dentry valid. In our case, there is no
> symlink at the time sysfs_remove_link() is called. (see next paragraph)
> And if the dentry is valid, sysfs looks to be smart enough to
> not do anything if the symlink is already removed.
> 
> The acm->control->dev to tty class device link (usb->tty)
> is removed when usb_disconnect() calls device_unregister().
> The acm->control->dev file and all children (including
> the symlink) get removed. The only thing that *might*
> hold the dentry at that point is being (parent of) the target of
> the forward symlink from the tty class device (tty->usb).

No, that shouldn't pin it.  It should simply render the forward symlink 
invalid.  You can easily create invalid symlinks in normal filesystems 
whenever you want (ln -s nonexistent_file newname); this is no different.

> The 2 calls in question in class_device_del():
> (remove tty->usb symlink) sysfs_remove_link class_dev=c9a6aec4 ref=2 
> dentry=c8f7a2b4
> (remove already released usb->tty symlink) sysfs_remove_link 
> class_dev->dev=cee88980 ref=1 dentry=ceaaa1a4
> 
> The tty->usb link gets removed first, so even *if*
> it pins the acm->control->dev dentry it is gone before
> trying to remove the (usb->tty) link (which is where the oops occurs).

Right.  Because that link has already been removed, during the 
device_unregister call.

> This looks like something that needs to be corrected in driver core,
> using the dentry=NULL to indicate no more sysfs entry.
> The changes might not be that bad. I need to look it over
> carefully and copy the maintainer.

It's a more general problem.  Unregistering a device in sysfs does the
equivalent of "rm -rf" on the device's directory, if I'm reading
sysfs_remove_dir correctly.  So later, when the owners of the entries in
that directory try to remove their entries, they run into problems because
the entries are already gone.

Perhaps this hasn't cropped up in the past because the entries were always 
removed before their parent directory.  And maybe it only matters for 
entries that are symlinks (as opposed to subdirectories or attribute 
files).  But you've got a case where things manage to go wrong.

It could be, as you say, that the symlink routines simply need to check 
for dentry == NULL to make sure the link hasn't already been removed.  The 
sysfs maintainer is the person to ask.  Hmmm, I see that the maintainer is 
in fact Greg...

> > You know, another possibility is that cdc_acm should call
> > tty_unregister_device as soon as the USB device is disconnected.  It would
> > make sense; no point leaving a tty visible in sysfs if it's no longer
> > accessible (except through already-opened file descriptors). However I
> > don't know whether the tty subsystem is designed to work that way.
> 
> tty would definately choke, as it has at that point open file descriptors
> to the device and may even be sleeping someplace. The entries that
> get eliminated by tty_unregister_device would result in some smoking ruins.
> That must wait until all file descriptors are closed.

Well, tty_unregister_device would be the wrong thing to call.  There ought
to be something like "tty_remove_device", which could safely be called
during the USB disconnect.  Then the final close could call
"tty_put_device" to complete the unregistration.  I don't know whether
such routines actually exist, though.

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to