From: Eric Paris <eparis@parisplace.org>
To: npiggin@suse.de
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 09/33] fs: dcache scale dentry refcount
Date: Sun, 6 Sep 2009 14:01:05 -0400 [thread overview]
Message-ID: <7e0fb38c0909061101t6b4f337cvf691814c28bcf50e@mail.gmail.com> (raw)
In-Reply-To: <20090904065535.281876981@nick.local0.net>
On Fri, Sep 4, 2009 at 2:51 AM, <npiggin@suse.de> wrote:
> Make d_count non-atomic and protect it with d_lock. This allows us to
> ensure a 0 refcount dentry remains 0 without dcache_lock. It is also
> fairly natural when we start protecting many other dentry members with
> d_lock.
> +struct dentry *dget_parent(struct dentry *dentry)
> +{
> + struct dentry *ret;
> +
> +repeat:
> + spin_lock(&dentry->d_lock);
> + ret = dentry->d_parent;
> + if (!ret)
> + goto out;
> + if (!spin_trylock(&ret->d_lock)) {
> + spin_unlock(&dentry->d_lock);
> + goto repeat;
> + }
> + BUG_ON(!ret->d_count);
> + ret->d_count++;
> + spin_unlock(&ret->d_lock);
> +out:
> + spin_unlock(&dentry->d_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(dget_parent);
> Index: linux-2.6/fs/notify/inotify/inotify.c
> ===================================================================
> --- linux-2.6.orig/fs/notify/inotify/inotify.c
> +++ linux-2.6/fs/notify/inotify/inotify.c
> @@ -339,18 +339,26 @@ void inotify_dentry_parent_queue_event(s
> if (!(dentry->d_flags & DCACHE_INOTIFY_PARENT_WATCHED))
> return;
>
> +again:
> spin_lock(&dentry->d_lock);
> parent = dentry->d_parent;
> + if (!spin_trylock(&parent->d_lock)) {
> + spin_unlock(&dentry->d_lock);
> + goto again;
> + }
> inode = parent->d_inode;
>
> if (inotify_inode_watched(inode)) {
> - dget(parent);
> + dget_dlock(parent);
> spin_unlock(&dentry->d_lock);
> + spin_unlock(&parent->d_lock);
> inotify_inode_queue_event(inode, mask, cookie, name,
> dentry->d_inode);
> dput(parent);
> - } else
> + } else {
> spin_unlock(&dentry->d_lock);
> + spin_unlock(&parent->d_lock);
> + }
I don't think I understand why in both of these cases you don't need
to check for dentry->d_parent == dentry
> Index: linux-2.6/fs/notify/fsnotify.c
> ===================================================================
> --- linux-2.6.orig/fs/notify/fsnotify.c
> +++ linux-2.6/fs/notify/fsnotify.c
> @@ -87,13 +87,18 @@ void __fsnotify_parent(struct dentry *de
> if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))
> return;
>
> +again:
> spin_lock(&dentry->d_lock);
> parent = dentry->d_parent;
> + if (parent != dentry && !spin_trylock(&parent->d_lock)) {
> + spin_unlock(&dentry->d_lock);
> + goto again;
> + }
> p_inode = parent->d_inode;
>
> if (fsnotify_inode_watches_children(p_inode)) {
> if (p_inode->i_fsnotify_mask & mask) {
> - dget(parent);
> + dget_dlock(parent);
> send = true;
> }
> } else {
And yet in this case we do check for dentry->d_parent == dentry. (my
unknowing self thinks we'd want to check in all places)
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2009-09-06 18:01 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-04 6:51 [patch 00/33] my current vfs scalability patch queue npiggin
2009-09-04 6:51 ` [patch 01/33] fs: no games with DCACHE_UNHASHED npiggin
2009-09-04 6:51 ` [patch 02/33] fs: cleanup files_lock npiggin
2009-09-04 6:51 ` [patch 03/33] fs: scale files_lock npiggin
2009-09-28 13:22 ` Peter Zijlstra
2009-09-28 13:24 ` Peter Zijlstra
2009-10-01 2:16 ` Nick Piggin
[not found] ` <r2i3282373b1004011751j440635b3n484018db2e2bc50c@mail.gmail.com>
2010-04-02 2:24 ` [patch 1/2] fs: cleanup files_lock tim
2009-09-04 6:51 ` [patch 04/33] fs: brlock vfsmount_lock npiggin
2009-09-04 15:19 ` Jens Axboe
2009-09-07 7:39 ` Nick Piggin
2009-09-22 15:17 ` Al Viro
2009-09-27 19:56 ` Nick Piggin
2009-09-28 13:21 ` Peter Zijlstra
2009-10-01 2:10 ` Nick Piggin
2009-09-04 6:51 ` [patch 05/33] fs: scale mntget/mntput npiggin
2009-09-07 9:41 ` Nick Piggin
2009-09-04 6:51 ` [patch 06/33] fs: dcache scale hash npiggin
2009-09-04 6:51 ` [patch 07/33] fs: dcache scale lru npiggin
2009-09-04 6:51 ` [patch 08/33] fs: dcache scale nr_dentry npiggin
2009-09-04 14:41 ` Daniel Walker
2009-09-07 7:36 ` Nick Piggin
2009-09-04 6:51 ` [patch 09/33] fs: dcache scale dentry refcount npiggin
2009-09-06 18:01 ` Eric Paris [this message]
2009-09-07 7:44 ` Nick Piggin
2009-09-07 11:21 ` Eric Paris
2009-09-07 11:35 ` Nick Piggin
2009-09-04 6:51 ` [patch 10/33] fs: dcache scale d_unhashed npiggin
2009-09-04 6:51 ` [patch 11/33] fs: dcache scale subdirs npiggin
2010-06-17 15:13 ` Peter Zijlstra
2010-06-17 16:53 ` Nick Piggin
2010-06-21 13:35 ` Peter Zijlstra
2010-06-21 14:48 ` Nick Piggin
2010-06-21 14:55 ` Peter Zijlstra
2010-06-22 6:02 ` john stultz
2010-06-22 6:06 ` Nick Piggin
2010-06-22 7:27 ` Peter Zijlstra
2010-06-23 2:03 ` john stultz
2010-06-23 7:23 ` Peter Zijlstra
2009-09-04 6:51 ` [patch 12/33] fs: scale inode alias list npiggin
2009-09-04 6:51 ` [patch 13/33] fs: use RCU / seqlock logic for reverse and multi-step operaitons npiggin
2009-09-04 6:51 ` [patch 14/33] fs: dcache remove dcache_lock npiggin
2009-09-04 6:51 ` [patch 15/33] fs: dcache reduce dput locking npiggin
2009-09-04 6:51 ` [patch 16/33] fs: dcache per-bucket dcache hash locking npiggin
2009-09-04 14:51 ` Daniel Walker
2009-09-07 7:38 ` Nick Piggin
2009-09-04 6:51 ` [patch 17/33] fs: dcache reduce dcache_inode_lock npiggin
2009-09-04 6:51 ` [patch 18/33] fs: dcache per-inode inode alias locking npiggin
2009-09-04 6:52 ` [patch 19/33] fs: icache lock s_inodes list npiggin
2009-09-04 6:52 ` [patch 20/33] fs: icache lock inode hash npiggin
2009-09-04 6:52 ` [patch 21/33] fs: icache lock i_state npiggin
2009-09-04 6:52 ` [patch 22/33] fs: icache lock i_count npiggin
2009-09-04 6:52 ` [patch 23/33] fs: icache atomic inodes_stat npiggin
2009-09-04 6:52 ` [patch 24/33] fs: icache lock lru/writeback lists npiggin
2009-09-04 6:52 ` [patch 25/33] fs: icache protect inode state npiggin
2009-09-04 6:52 ` [patch 26/33] fs: inode atomic last_ino, iunique lock npiggin
2009-09-04 6:52 ` [patch 27/33] fs: icache remove inode_lock npiggin
2009-09-04 6:52 ` [patch 28/33] fs: inode factor hash lock into functions npiggin
2009-09-04 6:52 ` [patch 29/33] Remove the global inode_hash_lock and replace it with per-hash-bucket locks. fs: inode per-bucket inode hash locks npiggin
2009-09-04 7:05 ` Nick Piggin
2009-09-04 6:52 ` [patch 30/33] fs: inode lazy lru npiggin
2009-09-04 6:52 ` [patch 31/33] fs: RCU free inodes npiggin
2009-09-04 6:52 ` [patch 32/33] fs: rcu walk for i_sb_list npiggin
2009-09-04 6:52 ` [patch 33/33] fs: improve scalability of pseudo filesystems npiggin
2009-09-04 7:05 ` [patch 00/33] my current vfs scalability patch queue Nick Piggin
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=7e0fb38c0909061101t6b4f337cvf691814c28bcf50e@mail.gmail.com \
--to=eparis@parisplace.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=npiggin@suse.de \
/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).