All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [CFT] Re: VFS deadlock ?
Date: Fri, 22 Mar 2013 05:18:17 +0000	[thread overview]
Message-ID: <20130322051817.GM21522@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFzUobqDR0RSh8t18m32BdTvm4k7j2PCseXpFVCH_gR=Fw@mail.gmail.com>

On Thu, Mar 21, 2013 at 09:55:10PM -0700, Linus Torvalds wrote:
> However, I do wonder if we could take another approach... There's
> really no reason why we should look up an old inode with iget_locked()
> in proc_get_inode() and only fill it in if it is new. We might as well
> just always create a new inode. The "iget_locked()" logic really comes
> from the bad old days when the inode was the primary data structure,
> but it's really the dentry that is the important data structure, and
> the inode might as well follow the dentry, instead of the other way
> around.
> 
> So why not just use "new_inode_pseudo()" instead? IOW, something like
> this (totally untested, natch) patch? This way, if you have a new
> dentry, you are guaranteed to always have a new inode. None of the
> silly inode alias issues..
> 
> This seems too simple, but I don't see why iget_locked() would be any
> better. It just wastes time hashing the inode that we aren't really
> interested in hashing. The inode is always filled by the caller
> anyway, so we migth as well just get a fresh pseudo-filesystem inode
> without any crazy hashing..

Umm...
static int proc_delete_dentry(const struct dentry * dentry)
{
        return 1;
}

static const struct dentry_operations proc_dentry_operations =
{
        .d_delete       = proc_delete_dentry,
};

IOW, dcache retention in procfs is inexistent and the damn thing tries
to cut down on the amount of inode allocation/freeing/filling.

I agree that we could get rid of icache retention there and everything
ought to keep working.  Hell knows...  It applies only to the stuff that
isn't per-process, so it's probably not particulary hot anyway, but it
does need profiling...  OTOH, we probably could mark "stable" dentries
in some way and let proc_delete_dentry() check that flag in proc_dir_entry -
I seriously suspect that really hot non-per-process ones are of the
"never become invalid" variety.

  reply	other threads:[~2013-03-22  5:18 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-21 19:06 VFS deadlock ? Dave Jones
2013-03-21 19:21 ` Al Viro
2013-03-21 20:31   ` Dave Jones
2013-03-21 19:29 ` Al Viro
2013-03-21 20:15   ` Linus Torvalds
2013-03-21 20:26     ` Dave Jones
2013-03-21 20:32       ` Linus Torvalds
2013-03-21 20:36         ` Dave Jones
2013-03-21 20:47           ` Al Viro
2013-03-21 21:02             ` Dave Jones
2013-03-21 21:18               ` Linus Torvalds
2013-03-21 21:26                 ` Al Viro
2013-03-21 21:41                   ` Dave Jones
2013-03-21 21:47                     ` Linus Torvalds
2013-03-21 21:55                       ` Al Viro
2013-03-21 21:57                         ` Linus Torvalds
2013-03-21 22:03                           ` Al Viro
2013-03-21 21:52                     ` Al Viro
2013-03-21 22:12                 ` Dave Jones
2013-03-21 22:29                   ` Dave Jones
2013-03-21 22:53                   ` Linus Torvalds
2013-03-21 23:07                     ` Dave Jones
2013-03-21 23:36                     ` Al Viro
2013-03-21 23:58                       ` Linus Torvalds
2013-03-22  0:01                         ` Linus Torvalds
2013-03-22  0:12                           ` Al Viro
2013-03-22  0:20                             ` Al Viro
2013-03-22  0:22                             ` Linus Torvalds
2013-03-22  1:22                               ` Al Viro
2013-03-22  1:33                                 ` Linus Torvalds
2013-03-22  1:40                                   ` Al Viro
2013-03-22  4:37                                     ` [CFT] " Al Viro
2013-03-22  4:55                                       ` Linus Torvalds
2013-03-22  5:18                                         ` Al Viro [this message]
2013-03-22  5:33                                           ` Linus Torvalds
2013-03-22  6:09                                             ` Al Viro
2013-03-22  6:22                                               ` Al Viro
2013-03-22 16:23                                             ` Dave Jones
2013-03-22 19:43                                             ` Linus Torvalds
2013-03-22 21:28                                               ` Al Viro
2013-03-22 22:57                                               ` Eric W. Biederman
2013-03-22  5:19                                         ` Linus Torvalds
2013-03-22  0:08                         ` Al Viro
2013-03-22  0:15                           ` Linus Torvalds
2013-03-22  0:19                             ` Linus Torvalds

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=20130322051817.GM21522@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davej@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.