On Mon, 30 May 2011, Sebastian Andrzej Siewior wrote: > rmmod of every hcd results currently in somethig like > > |uhci_hcd 0000:00:01.2: USB bus 1 deregistered > |------------[ cut here ]------------ > |kernel BUG at /home/bigeasy/git/linux/fs/dcache.c:419! > |invalid opcode: 0000 [#1] > |Modules linked in: uhci_hcd(-) ehci_hcd usbcore > | > |Pid: 1736, comm: rmmod Not tainted 3.0.0-rc1-00001-geb7d864-dirty #148 Bochs Bochs > |EIP: 0060:[<810c3f6b>] EFLAGS: 00010246 CPU: 0 > |EIP is at dput+0x12b/0x130 > |EAX: 00000000 EBX: af3ca9a0 ECX: af3cbd00 EDX: 00000202 > |ESI: af3c6000 EDI: af3cbca8 EBP: aeae7ddc ESP: aeae7dd4 > | DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 > |Process rmmod (pid: 1736, ti=aeae6000 task=ae815c00 task.ti=aeae6000) > |Stack: > | af3ca9a0 af3c6000 aeae7dfc b0a489b6 af3cb9d8 af3cbcc0 af3caa10 aefb2230 > | 00000000 00000000 aeae7e28 b0a48de5 afa37140 af402300 ade0a038 aeae7e40 > | 810ae9f5 81177295 b0a4dce0 00000000 00000000 aeae7e44 81049395 aefb2230 > |Call Trace: > | [<b0a489b6>] fs_remove_file+0x76/0x120 [usbcore] > | [<b0a48de5>] usbfs_notify+0x35/0x2d0 [usbcore] > | [<810ae9f5>] ? kfree+0x105/0x130 > | [<81177295>] ? kobject_release+0x45/0x80 > | [<81049395>] notifier_call_chain+0x45/0x60 > | [<81049793>] __blocking_notifier_call_chain+0x43/0x70 > | [<810497df>] blocking_notifier_call_chain+0x1f/0x30 > | [<b0a46a39>] usb_notify_remove_bus+0x19/0x20 [usbcore] > | [<b0a3af0b>] usb_deregister_bus+0x5b/0x70 [usbcore] > | [<b0a3b011>] usb_remove_hcd+0xf1/0x100 [usbcore] > > a bisect turned up: > > |commit 64252c75a2196a0cf1e0d3777143ecfe0e3ae650 > |Author: Sage Weil <sage@xxxxxxxxxxxx> > |Date: Tue May 24 13:06:05 2011 -0700 > | > | vfs: remove dget() from dentry_unhash() > | > | This serves no useful purpose that I can discern. All callers (rename, > | rmdir) hold their own reference to the dentry. > | > | A quick audit of all file systems showed no relevant checks on the value > | of d_count in vfs_rmdir/vfs_rename_dir paths. > | > | Signed-off-by: Sage Weil <sage@xxxxxxxxxxxx> > | Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > > I removed the "inner" dput() which is called on emptry dir and sets error > to zero. This looks like the same change in 64252c7 for vfs_rmdir() > where dput() is removed from the path where we have dont_mount() and > S_DEAD. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> > --- > Could one of VFS ppl look at this an NACK/ACK it? I think it's the other dput that you want to remove. 64252c75 is a misleading because the first hunk has to remove dput() from every exit path for the function. dentry_unhash is unconditionally doing dget, though. I think we want diff --git a/drivers/usb/core/inode.c b/drivers/usb/core/inode.c index 1b125c2..2278dad 100644 --- a/drivers/usb/core/inode.c +++ b/drivers/usb/core/inode.c @@ -389,7 +389,6 @@ static int usbfs_rmdir(struct inode *dir, struct dentry *dentry) mutex_unlock(&inode->i_mutex); if (!error) d_delete(dentry); - dput(dentry); return error; } Sorry I missed this one; I forgot there were filesystems outside of fs/. sage > > Jessi: I've seen your patch after I had mine done. If you send me a > sign-off-by for this, I will resend it with you as Author + SOB if you > like once the VFS are fine with this. > > drivers/usb/core/inode.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/drivers/usb/core/inode.c b/drivers/usb/core/inode.c > index 1b125c2..3ef9308 100644 > --- a/drivers/usb/core/inode.c > +++ b/drivers/usb/core/inode.c > @@ -381,7 +381,6 @@ static int usbfs_rmdir(struct inode *dir, struct dentry *dentry) > dont_mount(dentry); > drop_nlink(dentry->d_inode); > drop_nlink(dentry->d_inode); > - dput(dentry); > inode->i_flags |= S_DEAD; > drop_nlink(dir); > error = 0; > -- > 1.7.4.4 > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html