|
|
Subscribe / Log in / New account

Re: VFS deadlock ?

From:  Linus Torvalds <torvalds-AT-linux-foundation.org>
To:  Al Viro <viro-AT-zeniv.linux.org.uk>
Subject:  Re: VFS deadlock ?
Date:  Thu, 21 Mar 2013 16:58:41 -0700
Message-ID:  <CA+55aFyLJ9vVm1T994EaE4tnEbV8rHj43m+cWsFFBi_qndao1A@mail.gmail.com>
Cc:  Dave Jones <davej-AT-redhat.com>, Linux Kernel <linux-kernel-AT-vger.kernel.org>, "Eric W. Biederman" <ebiederm-AT-xmission.com>
Archive‑link:  Article

On Thu, Mar 21, 2013 at 4:36 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Some netns-related idiocy.  Oh, shit...
>
> al@duke:~/linux/trees/vfs$ ls -lid /proc/{1,2}/net/stat
> 4026531842 dr-xr-xr-x 2 root root 0 Mar 21 19:33 /proc/1/net/stat
> 4026531842 dr-xr-xr-x 2 root root 0 Mar 21 19:33 /proc/2/net/stat
>
> Eric, would you mind explaining WTF is going on here?  Again, WE CAN NOT
> HAVE SEVERAL DENTRIES OVER THE SAME DIRECTORY INODE.  Ever.  We do that,
> we are fucked.

Hmm. That certainly explains the situation, but it leaves me wondering
whether the simplest solution to this is not to say "ok, let's allow
it in this case".

The locking is already per-inode, so we can literally change the code
that checks "if same dentry" to "if same inode" instead.

And the only other reason we don't want to allow it is to make sure
you can't have directory loops etc, afaik, and again, for this
particular case of /proc, we happen to be ok.

So yes, it's against the rules, and we get that deadlock right now,
but one solution would be to just allow this particular case. The
patch for the deadlock looks dead simple:

    diff --git a/fs/namei.c b/fs/namei.c
    index 57ae9c8c66bf..435002f99bd8 100644
    --- a/fs/namei.c
    +++ b/fs/namei.c
    @@ -2277,7 +2277,7 @@ struct dentry *lock_rename(struct dentry
*p1, struct dentry *p2)
     {
             struct dentry *p;

    -        if (p1 == p2) {
    +        if (p1->d_inode == p2->d_inode) {
                     mutex_lock_nested(&p1->d_inode->i_mutex, I_MUTEX_PARENT);
                     return NULL;
             }
    @@ -2306,7 +2306,7 @@ struct dentry *lock_rename(struct dentry
*p1, struct dentry *p2)
     void unlock_rename(struct dentry *p1, struct dentry *p2)
     {
             mutex_unlock(&p1->d_inode->i_mutex);
    -        if (p1 != p2) {
    +        if (p1->d_inode != p2->d_inode) {
                     mutex_unlock(&p2->d_inode->i_mutex);
                     mutex_unlock(&p1->d_inode->i_sb->s_vfs_rename_mutex);
             }

Are there any other reasons why these kinds of "hardlinked
directories" would cause problems?

             Linus



to post comments


Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds