On Thu, 19 Mar 2009, Christoph Hellwig wrote:
> On Thu, Mar 19, 2009 at 01:16:30PM -0700, Sage Weil wrote:
> > + if (result) {
> > + /*
> > + * The cache was re-populated while we waited on the
> > + * mutex. We need to revalidate, this time while
> > + * holding i_mutex (to avoid another race).
> > + */
> > + if (result->d_op && result->d_op->d_revalidate) {
> > + result = do_revalidate(result, nd);
> > if (result)
> > + goto out_unlock;
> > + /*
> > + * The dentry was left behind invalid. Just
> > + * do the lookup.
> > + */
> > + } else {
> > + goto out_unlock;
> > }
>
> This would be better off as
>
> if (!result->d_op || !result->d_op->d_revalidate)
> goto out_unlock;
>
> result = do_revalidate(result, nd);
> if (result)
> goto out_unlock;
>
> /*
> * The dentry was left behind invalid. Just
> * do the lookup.
> */
>
> Otherwise looks good to me.
Fixed up below. Thanks!
sage
---
>From 81c8657b56f95419f8235188e6de1fc2fa73ff14 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@xxxxxxxxxxxx>
Date: Thu, 19 Mar 2009 13:25:12 -0700
Subject: [PATCH 2/2] vfs: clean up real_lookup
Get rid of the goto by flipping the if (!result) over. Make the
comments a bit more descriptive. Fix a few kernel style problems.
No functional changes.
Signed-off-by: Sage Weil <sage@xxxxxxxxxxxx>
---
fs/namei.c | 63 +++++++++++++++++++++++++++++++----------------------------
1 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index b9e7128..ead15a6 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -461,19 +461,20 @@ ok:
* This is called when everything else fails, and we actually have
* to go to the low-level filesystem to find out what we should do..
*
- * We get the directory semaphore, and after getting that we also
+ * We get the directory mutex, and after getting that we also
* make sure that nobody added the entry to the dcache in the meantime..
* SMP-safe
*/
-static struct dentry * real_lookup(struct dentry * parent, struct qstr * name, struct nameidata *nd)
+static struct dentry *real_lookup(struct dentry *parent, struct qstr *name,
+ struct nameidata *nd)
{
- struct dentry * result;
+ struct dentry *result, *dentry;
struct inode *dir = parent->d_inode;
mutex_lock(&dir->i_mutex);
/*
* First re-do the cached lookup just in case it was created
- * while we waited for the directory semaphore..
+ * while we waited for the directory mutex.
*
* FIXME! This could use version numbering or similar to
* avoid unnecessary cache lookups.
@@ -486,38 +487,40 @@ static struct dentry * real_lookup(struct dentry * parent, struct qstr * name, s
* so doing d_lookup() (with seqlock), instead of lockfree __d_lookup
*/
result = d_lookup(parent, name);
- if (!result) {
- struct dentry *dentry;
+ if (result) {
+ /*
+ * The cache was re-populated while we waited on the
+ * mutex. We need to revalidate, this time while
+ * holding i_mutex (to avoid another race).
+ */
+ if (!result->d_op || !result->d_op->d_revalidate)
+ goto out_unlock;
-do_the_lookup:
- /* Don't create child dentry for a dead directory. */
- result = ERR_PTR(-ENOENT);
- if (IS_DEADDIR(dir))
+ result = do_revalidate(result, nd);
+ if (result)
goto out_unlock;
- dentry = d_alloc(parent, name);
- result = ERR_PTR(-ENOMEM);
- if (dentry) {
- result = dir->i_op->lookup(dir, dentry, nd);
- if (result)
- dput(dentry);
- else
- result = dentry;
- }
-out_unlock:
- mutex_unlock(&dir->i_mutex);
- return result;
+ /*
+ * The dentry was left behind invalid. Just
+ * do the lookup.
+ */
}
- /*
- * Uhhuh! Nasty case: the cache was re-populated while
- * we waited on the semaphore. Need to revalidate.
- */
- if (result->d_op && result->d_op->d_revalidate) {
- result = do_revalidate(result, nd);
- if (!result)
- goto do_the_lookup;
+ /* Don't create child dentry for a dead directory. */
+ result = ERR_PTR(-ENOENT);
+ if (IS_DEADDIR(dir))
+ goto out_unlock;
+
+ dentry = d_alloc(parent, name);
+ result = ERR_PTR(-ENOMEM);
+ if (dentry) {
+ result = dir->i_op->lookup(dir, dentry, nd);
+ if (result)
+ dput(dentry);
+ else
+ result = dentry;
}
+out_unlock:
mutex_unlock(&dir->i_mutex);
return result;
}
--
1.5.6.5
--
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
