| From: |
| Linus Torvalds <torvalds@linux-foundation.org> |
| To: |
| Al Viro <viro@zeniv.linux.org.uk>, Waiman Long <Waiman.Long@hp.com> |
| Subject: |
| Adding concept of "dead" references to lockref |
| Date: |
| Sat, 7 Sep 2013 16:34:27 -0700 |
| Message-ID: |
| <CA+55aFwwvxHnJTFrqMGjjR_uXLOzPs0zucEL=x4D9TapWRyGfA@mail.gmail.com> |
| Cc: |
| Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, "Chandramouleeswaran, Aswin" <aswin@hp.com>, "Norton, Scott J" <scott.norton@hp.com> |
| Archive-link: |
| Article, Thread
|
Ok, I added the notion of having a dead entry to lockref, because it
simplifies the dcache code a bit. I committed and pushed out the
actual lockref infrastructure, but the actual dcache *use* of that
infrastructure is probably too late for 3.12, or at least it needs
some discussion.
I'm attaching the patch here (but it needs _current_ git as of not
very long ago for the new "dead lockref" support). It simplifies both
the actual killing of dentries (we used to pass this "ref" thing
around to show whether we held the last ref to the dentry or not, this
makes it pointless because we'll just overwrite the refcount anyway)
and it particularly simplifies the getting the reference for the RCU
case, because the whole "it's valid but we couldn't tell" case is
gone.
Comments? It doesn't matter for my insane test-case, since that one
was either looking at a directory with non-zero refcount to begin
with, and even if you looked up a regular file, all the other people
looking it up would guarantee that non-zero refcount. But it basically
makes the (common) special case of "we got the dentry d_lock because
we're looking up a file that nobody else had actively open yet", and
now that's just a reference count operation too.
Simpler code and one less special case sounds like a good thing to me.
Linus
fs/dcache.c | 17 ++++++++++-------
fs/namei.c | 23 +++++------------------
2 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 761e31bacbc2..ab34bfc4d199 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -229,7 +229,7 @@ static void __d_free(struct rcu_head *head)
*/
static void d_free(struct dentry *dentry)
{
- BUG_ON(dentry->d_lockref.count);
+ BUG_ON((int)dentry->d_lockref.count > 0);
this_cpu_dec(nr_dentry);
if (dentry->d_op && dentry->d_op->d_release)
dentry->d_op->d_release(dentry);
@@ -443,7 +443,7 @@ EXPORT_SYMBOL(d_drop);
* If ref is non-zero, then decrement the refcount too.
* Returns dentry requiring refcount drop, or NULL if we're done.
*/
-static inline struct dentry *dentry_kill(struct dentry *dentry, int ref)
+static inline struct dentry *dentry_kill(struct dentry *dentry)
__releases(dentry->d_lock)
{
struct inode *inode;
@@ -466,8 +466,11 @@ relock:
goto relock;
}
- if (ref)
- dentry->d_lockref.count--;
+ /*
+ * The dentry is now unrecoverably dead to the world.
+ */
+ lockref_mark_dead(&dentry->d_lockref);
+
/*
* inform the fs via d_prune that this dentry is about to be
* unhashed and destroyed.
@@ -535,7 +538,7 @@ repeat:
return;
kill_it:
- dentry = dentry_kill(dentry, 1);
+ dentry = dentry_kill(dentry);
if (dentry)
goto repeat;
}
@@ -760,7 +763,7 @@ static void try_prune_one_dentry(struct dentry *dentry)
{
struct dentry *parent;
- parent = dentry_kill(dentry, 0);
+ parent = dentry_kill(dentry);
/*
* If dentry_kill returns NULL, we have nothing more to do.
* if it returns the same dentry, trylocks failed. In either
@@ -781,7 +784,7 @@ static void try_prune_one_dentry(struct dentry *dentry)
while (dentry) {
if (lockref_put_or_lock(&dentry->d_lockref))
return;
- dentry = dentry_kill(dentry, 1);
+ dentry = dentry_kill(dentry);
}
}
diff --git a/fs/namei.c b/fs/namei.c
index f415c6683a83..cc4bcfaa8624 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -517,25 +517,12 @@ static inline void unlock_rcu_walk(void)
*/
static inline int d_rcu_to_refcount(struct dentry *dentry, seqcount_t *validate, unsigned seq)
{
- int gotref;
-
- gotref = lockref_get_or_lock(&dentry->d_lockref);
-
- /* Does the sequence number still match? */
- if (read_seqcount_retry(validate, seq)) {
- if (gotref)
- dput(dentry);
- else
- spin_unlock(&dentry->d_lock);
- return -ECHILD;
- }
-
- /* Get the ref now, if we couldn't get it originally */
- if (!gotref) {
- dentry->d_lockref.count++;
- spin_unlock(&dentry->d_lock);
+ if (likely(lockref_get_not_dead(&dentry->d_lockref))) {
+ if (!read_seqcount_retry(validate, seq))
+ return 0;
+ dput(dentry);
}
- return 0;
+ return -ECHILD;
}
/**