A change to the VFS path walk locking is needed to resolve an issue
identified by Sage Weil. This locking change requires significant
changes to the autofs4 module to allow it to callback to userspace
without introducing a deadlock.
To cope with the change the autofs4 module needs to redirect mount
requests from ->d_revalidate() to ->lookup() if the directory
inode mutex is held when a callback needs to be done. Note that we
cannot redirect these requests when the mutex is not held because,
to function correctly, the mutex must be held over both revalidate
and lookup.
Of the patches in the series most are cleanups and refactoring done
to keep the real change in "autofs4 - always use lookup for lookup"
as clean as possible. Unfortuneately, there is still quite a bit
left in it.
Also, I need confirmation that the patch that changes the VFS path
walk locking is in fact correct, or at least like for like to what
will be submitted. I had some difficulty with the original patches
that were paosted. The patch in question below is "vfs: make
real_lookup do dentry revalidation with i_mutex held".
I've done quite a bit of fairly heavy stress testing of the patch
series and they (finally) hold up to it. Although I have also
managed to uncover a locking bug in the user space daemon as a
result, ;)
---
Ian Kent (10):
autofs4 - always use lookup for lookup
autofs4 - rename dentry to expiring in autofs4_lookup_expiring()
autofs4 - rename dentry to active in autofs4_lookup_active()
autofs4 - eliminate d_unhashed in path walk checks
autofs4 - cleanup active and expire lookup
autofs4 - renamer unhashed to active in autofs4_lookup()
autofs4 - use autofs_info for pending flag
autofs4 - use macro for need mount check
autofs4 - use macros for expiring list
autofs4 - use macros for active list handling
Sage Weil (1):
Subject: [PATCH] vfs: make real_lookup do dentry revalidation with i_mutex held
fs/autofs4/autofs_i.h | 38 +++
fs/autofs4/expire.c | 8 -
fs/autofs4/inode.c | 2
fs/autofs4/root.c | 616 ++++++++++++++++++++++++++++++++-----------------
fs/namei.c | 58 ++---
5 files changed, 480 insertions(+), 242 deletions(-)
--
Ian
From: Sage Weil <[email protected]>
real_lookup() is called by do_lookup() if dentry revalidation fails. If
the cache is re-populated while waiting for i_mutex, it may find that
a d_lookup() subsequently succeeds (see the "Uhhuh! Nasty case" comment).
Previously, real_lookup() would drop i_mutex and do_revalidate() again. If
revalidate failed _again_, however, it would give up with -ENOENT. The
problem here that network file systems may be invalidating dentries via
server callbacks, e.g. due to concurrent access from another client, and
-ENOENT is frequently the wrong answer.
This problem has been seen with both Lustre and Ceph. It seems possible
to hit this case with NFS as well if the cache lifetime is very short.
Instead, we should do_revalidate() while i_mutex is still held. If
revalidation fails, we can move on to a ->lookup() and ensure a correct
result without worrying about any subsequent races.
Note that do_revalidate() is called with i_mutex held elsewhere. For
example, do_filp_open(), lookup_create(), do_unlinkat(), do_rmdir(),
and possibly others all take the directory i_mutex, and then
-> lookup_hash
-> __lookup_hash
-> cached_lookup
-> do_revalidate
so this does not introduce any new locking rules for d_revalidate
implementations.
Signed-off-by: Yehuda Sadeh <[email protected]>
Signed-off-by: Sage Weil <[email protected]>
---
fs/namei.c | 58 +++++++++++++++++++++++++++++++---------------------------
1 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index d11f404..d68ea6d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -477,6 +477,7 @@ static struct dentry * real_lookup(struct dentry * parent, struct qstr * name, s
{
struct dentry * result;
struct inode *dir = parent->d_inode;
+ struct dentry *dentry;
mutex_lock(&dir->i_mutex);
/*
@@ -494,38 +495,41 @@ 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;
-
- /* 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) {
+ /*
+ * 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)
- dput(dentry);
- else
- result = dentry;
+ goto out_unlock;
+ /*
+ * The dentry was left behind invalid. Just
+ * do the lookup.
+ */
+ } else {
+ goto out_unlock;
}
-out_unlock:
- mutex_unlock(&dir->i_mutex);
- return result;
}
- /*
- * Uhhuh! Nasty case: the cache was re-populated while
- * we waited on the semaphore. Need to revalidate.
- */
- mutex_unlock(&dir->i_mutex);
- if (result->d_op && result->d_op->d_revalidate) {
- result = do_revalidate(result, nd);
- if (!result)
- result = ERR_PTR(-ENOENT);
+ /* 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;
}
Define some simple macro functions for adding and deleting entries
on the active (and unhashed) dentry list.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/autofs_i.h | 2 ++
fs/autofs4/inode.c | 1 +
fs/autofs4/root.c | 46 +++++++++++++++++++++++++++++++++++-----------
3 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 8f7cdde..f3cf151 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -75,6 +75,8 @@ struct autofs_info {
struct completion expire_complete;
struct list_head active;
+ int active_count;
+
struct list_head expiring;
struct autofs_sb_info *sbi;
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index 69c8142..4670a78 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -49,6 +49,7 @@ struct autofs_info *autofs4_init_ino(struct autofs_info *ino,
ino->dentry = NULL;
ino->size = 0;
INIT_LIST_HEAD(&ino->active);
+ ino->active_count = 0;
INIT_LIST_HEAD(&ino->expiring);
atomic_set(&ino->count, 0);
}
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index b96a3c5..67d8d96 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -72,6 +72,38 @@ const struct inode_operations autofs4_dir_inode_operations = {
.rmdir = autofs4_dir_rmdir,
};
+static void autofs4_add_active(struct dentry *dentry)
+{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ if (ino) {
+ spin_lock(&sbi->lookup_lock);
+ if (!ino->active_count) {
+ if (list_empty(&ino->active))
+ list_add(&ino->active, &sbi->active_list);
+ }
+ ino->active_count++;
+ spin_unlock(&sbi->lookup_lock);
+ }
+ return;
+}
+
+static void autofs4_del_active(struct dentry *dentry)
+{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ if (ino) {
+ spin_lock(&sbi->lookup_lock);
+ ino->active_count--;
+ if (!ino->active_count) {
+ if (!list_empty(&ino->active))
+ list_del_init(&ino->active);
+ }
+ spin_unlock(&sbi->lookup_lock);
+ }
+ return;
+}
+
static int autofs4_dir_open(struct inode *inode, struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
@@ -513,9 +545,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
dentry->d_fsdata = ino;
ino->dentry = dentry;
- spin_lock(&sbi->lookup_lock);
- list_add(&ino->active, &sbi->active_list);
- spin_unlock(&sbi->lookup_lock);
+ autofs4_add_active(dentry);
d_instantiate(dentry, NULL);
}
@@ -624,10 +654,7 @@ static int autofs4_dir_symlink(struct inode *dir,
if (!ino)
return -ENOMEM;
- spin_lock(&sbi->lookup_lock);
- if (!list_empty(&ino->active))
- list_del_init(&ino->active);
- spin_unlock(&sbi->lookup_lock);
+ autofs4_del_active(dentry);
ino->size = strlen(symname);
cp = kmalloc(ino->size + 1, GFP_KERNEL);
@@ -775,10 +802,7 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode)
if (!ino)
return -ENOMEM;
- spin_lock(&sbi->lookup_lock);
- if (!list_empty(&ino->active))
- list_del_init(&ino->active);
- spin_unlock(&sbi->lookup_lock);
+ autofs4_del_active(dentry);
inode = autofs4_get_inode(dir->i_sb, ino);
if (!inode) {
Define some simple macro functions for adding and deleting entries
on the expiring dentry list.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/autofs_i.h | 26 ++++++++++++++++++++++++++
fs/autofs4/root.c | 15 +++------------
2 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index f3cf151..fe9fc23 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -266,5 +266,31 @@ out:
return ret;
}
+static inline void autofs4_add_expiring(struct dentry *dentry)
+{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ if (ino) {
+ spin_lock(&sbi->lookup_lock);
+ if (list_empty(&ino->expiring))
+ list_add(&ino->expiring, &sbi->expiring_list);
+ spin_unlock(&sbi->lookup_lock);
+ }
+ return;
+}
+
+static inline void autofs4_del_expiring(struct dentry *dentry)
+{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ if (ino) {
+ spin_lock(&sbi->lookup_lock);
+ if (!list_empty(&ino->expiring))
+ list_del_init(&ino->expiring);
+ spin_unlock(&sbi->lookup_lock);
+ }
+ return;
+}
+
void autofs4_dentry_release(struct dentry *);
extern void autofs4_kill_sb(struct super_block *);
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 67d8d96..2954ac5 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -563,10 +563,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
*/
ino = autofs4_dentry_ino(expiring);
autofs4_expire_wait(expiring);
- spin_lock(&sbi->lookup_lock);
- if (!list_empty(&ino->expiring))
- list_del_init(&ino->expiring);
- spin_unlock(&sbi->lookup_lock);
+ autofs4_del_expiring(expiring);
dput(expiring);
}
@@ -732,10 +729,7 @@ static int autofs4_dir_unlink(struct inode *dir, struct dentry *dentry)
dir->i_mtime = CURRENT_TIME;
spin_lock(&dcache_lock);
- spin_lock(&sbi->lookup_lock);
- if (list_empty(&ino->expiring))
- list_add(&ino->expiring, &sbi->expiring_list);
- spin_unlock(&sbi->lookup_lock);
+ autofs4_add_expiring(dentry);
spin_lock(&dentry->d_lock);
__d_drop(dentry);
spin_unlock(&dentry->d_lock);
@@ -761,10 +755,7 @@ static int autofs4_dir_rmdir(struct inode *dir, struct dentry *dentry)
spin_unlock(&dcache_lock);
return -ENOTEMPTY;
}
- spin_lock(&sbi->lookup_lock);
- if (list_empty(&ino->expiring))
- list_add(&ino->expiring, &sbi->expiring_list);
- spin_unlock(&sbi->lookup_lock);
+ autofs4_add_expiring(dentry);
spin_lock(&dentry->d_lock);
__d_drop(dentry);
spin_unlock(&dentry->d_lock);
Define simple macro function for checking if we need to trigger
a mount.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/root.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 2954ac5..f6e8ca9 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -104,6 +104,14 @@ static void autofs4_del_active(struct dentry *dentry)
return;
}
+static unsigned int autofs4_need_mount(unsigned int flags)
+{
+ unsigned int res = 0;
+ if (flags & (TRIGGER_FLAGS | TRIGGER_INTENTS))
+ res = 1;
+ return res;
+}
+
static int autofs4_dir_open(struct inode *inode, struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
@@ -168,7 +176,7 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags)
}
/* Trigger mount for path component or follow link */
} else if (dentry->d_flags & DCACHE_AUTOFS_PENDING ||
- flags & (TRIGGER_FLAGS | TRIGGER_INTENTS) ||
+ autofs4_need_mount(flags) ||
current->link_count) {
DPRINTK("waiting for mount name=%.*s",
dentry->d_name.len, dentry->d_name.name);
@@ -234,7 +242,7 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
autofs4_expire_wait(dentry);
/* We trigger a mount for almost all flags */
- lookup_type = nd->flags & (TRIGGER_FLAGS | TRIGGER_INTENTS);
+ lookup_type = autofs4_need_mount(nd->flags);
if (!(lookup_type || dentry->d_flags & DCACHE_AUTOFS_PENDING))
goto follow;
Eliminate the use of the d_lock spin lock by using the autofs super
block info spin lock. This reduces the number of spin locks we use
by one and makes the code for the following patch (to redirect
->d_revalidate() to ->lookup()) a little simpler.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/autofs_i.h | 3 ++-
fs/autofs4/expire.c | 2 +-
fs/autofs4/root.c | 58 +++++++++++++++++++++++++++----------------------
3 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index fe9fc23..3d283ab 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -97,6 +97,7 @@ struct autofs_info {
#define AUTOFS_INF_EXPIRING (1<<0) /* dentry is in the process of expiring */
#define AUTOFS_INF_MOUNTPOINT (1<<1) /* mountpoint status for direct expire */
+#define AUTOFS_INF_PENDING (1<<2) /* dentry pending mount */
struct autofs_wait_queue {
wait_queue_head_t queue;
@@ -163,7 +164,7 @@ static inline int autofs4_ispending(struct dentry *dentry)
{
struct autofs_info *inf = autofs4_dentry_ino(dentry);
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING)
+ if (inf->flags & AUTOFS_INF_PENDING)
return 1;
if (inf->flags & AUTOFS_INF_EXPIRING)
diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index 3da18d4..a796c94 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -27,7 +27,7 @@ static inline int autofs4_can_expire(struct dentry *dentry,
return 0;
/* No point expiring a pending mount */
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING)
+ if (ino->flags & AUTOFS_INF_PENDING)
return 0;
if (!do_now) {
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index f6e8ca9..305136b 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -166,32 +166,32 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags)
/* Turn this into a real negative dentry? */
if (status == -ENOENT) {
- spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
return status;
} else if (status) {
/* Return a negative dentry, but leave it "pending" */
return status;
}
/* Trigger mount for path component or follow link */
- } else if (dentry->d_flags & DCACHE_AUTOFS_PENDING ||
+ } else if (ino->flags & AUTOFS_INF_PENDING ||
autofs4_need_mount(flags) ||
current->link_count) {
DPRINTK("waiting for mount name=%.*s",
dentry->d_name.len, dentry->d_name.name);
- spin_lock(&dentry->d_lock);
- dentry->d_flags |= DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
+ spin_lock(&sbi->fs_lock);
+ ino->flags |= AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
status = autofs4_wait(sbi, dentry, NFY_MOUNT);
DPRINTK("mount done status=%d", status);
if (status) {
- spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
return status;
}
}
@@ -200,9 +200,9 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags)
if (ino)
ino->last_used = jiffies;
- spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
return 0;
}
@@ -243,18 +243,23 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
/* We trigger a mount for almost all flags */
lookup_type = autofs4_need_mount(nd->flags);
- if (!(lookup_type || dentry->d_flags & DCACHE_AUTOFS_PENDING))
+ spin_lock(&sbi->fs_lock);
+ spin_lock(&dcache_lock);
+ if (!(lookup_type || ino->flags & AUTOFS_INF_PENDING)) {
+ spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
goto follow;
+ }
/*
* If the dentry contains directories then it is an autofs
* multi-mount with no root mount offset. So don't try to
* mount it again.
*/
- spin_lock(&dcache_lock);
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING ||
+ if (ino->flags & AUTOFS_INF_PENDING ||
(!d_mountpoint(dentry) && __simple_empty(dentry))) {
spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
status = try_to_fill_dentry(dentry, 0);
if (status)
@@ -263,6 +268,7 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
goto follow;
}
spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
follow:
/*
* If there is no root mount it must be an autofs
@@ -525,9 +531,10 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
current->pid, task_pgrp_nr(current), sbi->catatonic, oz_mode);
unhashed = autofs4_lookup_active(sbi, dentry->d_parent, &dentry->d_name);
- if (unhashed)
+ if (unhashed) {
dentry = unhashed;
- else {
+ ino = autofs4_dentry_ino(dentry);
+ } else {
/*
* Mark the dentry incomplete but don't hash it. We do this
* to serialize our inode creation operations (symlink and
@@ -569,15 +576,14 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
* be quite complete but the directory has been removed
* so it must have been successful, so just wait for it.
*/
- ino = autofs4_dentry_ino(expiring);
autofs4_expire_wait(expiring);
autofs4_del_expiring(expiring);
dput(expiring);
}
- spin_lock(&dentry->d_lock);
- dentry->d_flags |= DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
+ spin_lock(&sbi->fs_lock);
+ ino->flags |= AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
if (dentry->d_op && dentry->d_op->d_revalidate)
(dentry->d_op->d_revalidate)(dentry, nd);
mutex_lock(&dir->i_mutex);
@@ -587,7 +593,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
* If we are still pending, check if we had to handle
* a signal. If so we can force a restart..
*/
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING) {
+ if (ino->flags & AUTOFS_INF_PENDING) {
/* See if we were interrupted */
if (signal_pending(current)) {
sigset_t *sigset = ¤t->pending.signal;
@@ -600,9 +606,9 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
}
}
if (!oz_mode) {
- spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
}
}
Rename the variable unhashed to active in autofs4_lookup() to
better reflect its usage.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/root.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 305136b..961ff37 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -514,7 +514,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
{
struct autofs_sb_info *sbi;
struct autofs_info *ino;
- struct dentry *expiring, *unhashed;
+ struct dentry *expiring, *active;
int oz_mode;
DPRINTK("name = %.*s",
@@ -530,9 +530,9 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
DPRINTK("pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d",
current->pid, task_pgrp_nr(current), sbi->catatonic, oz_mode);
- unhashed = autofs4_lookup_active(sbi, dentry->d_parent, &dentry->d_name);
- if (unhashed) {
- dentry = unhashed;
+ active = autofs4_lookup_active(sbi, dentry->d_parent, &dentry->d_name);
+ if (active) {
+ dentry = active;
ino = autofs4_dentry_ino(dentry);
} else {
/*
@@ -600,8 +600,8 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
if (sigismember (sigset, SIGKILL) ||
sigismember (sigset, SIGQUIT) ||
sigismember (sigset, SIGINT)) {
- if (unhashed)
- dput(unhashed);
+ if (active)
+ dput(active);
return ERR_PTR(-ERESTARTNOINTR);
}
}
@@ -633,14 +633,14 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
else
dentry = ERR_PTR(-ENOENT);
- if (unhashed)
- dput(unhashed);
+ if (active)
+ dput(active);
return dentry;
}
- if (unhashed)
- return unhashed;
+ if (active)
+ return active;
return NULL;
}
The lookup functions for active and expiring dentrys use parameters
that can be easily obtained on entry so we change the call to to
take just the dentry. This makes the subsequent change, to send all
lookups to ->lookup(), a bit cleaner.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/root.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 961ff37..81700f4 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -405,8 +405,11 @@ static const struct dentry_operations autofs4_dentry_operations = {
.d_release = autofs4_dentry_release,
};
-static struct dentry *autofs4_lookup_active(struct autofs_sb_info *sbi, struct dentry *parent, struct qstr *name)
+static struct dentry *autofs4_lookup_active(struct dentry *dentry)
{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct dentry *parent = dentry->d_parent;
+ struct qstr *name = &dentry->d_name;
unsigned int len = name->len;
unsigned int hash = name->hash;
const unsigned char *str = name->name;
@@ -457,8 +460,11 @@ next:
return NULL;
}
-static struct dentry *autofs4_lookup_expiring(struct autofs_sb_info *sbi, struct dentry *parent, struct qstr *name)
+static struct dentry *autofs4_lookup_expiring(struct dentry *dentry)
{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct dentry *parent = dentry->d_parent;
+ struct qstr *name = &dentry->d_name;
unsigned int len = name->len;
unsigned int hash = name->hash;
const unsigned char *str = name->name;
@@ -530,7 +536,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
DPRINTK("pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d",
current->pid, task_pgrp_nr(current), sbi->catatonic, oz_mode);
- active = autofs4_lookup_active(sbi, dentry->d_parent, &dentry->d_name);
+ active = autofs4_lookup_active(dentry);
if (active) {
dentry = active;
ino = autofs4_dentry_ino(dentry);
@@ -567,9 +573,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
if (!oz_mode) {
mutex_unlock(&dir->i_mutex);
- expiring = autofs4_lookup_expiring(sbi,
- dentry->d_parent,
- &dentry->d_name);
+ expiring = autofs4_lookup_expiring(dentry);
if (expiring) {
/*
* If we are racing with expire the request might not
We unhash the dentry (in a subsequent patch) in ->d_revalidate() in
order to send mount requests to ->lookup(). But then we can not rely
on d_unhased() to give reliable results because it may be called at
any time by any code path. The d_unhashed() function is used by
__simple_empty() in the path walking callbacks but autofs mount
point dentrys should have no directories at all so a list_empty()
on d_subdirs should be (and is) sufficient.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/root.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 81700f4..b6530f3 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -133,7 +133,7 @@ static int autofs4_dir_open(struct inode *inode, struct file *file)
* it.
*/
spin_lock(&dcache_lock);
- if (!d_mountpoint(dentry) && __simple_empty(dentry)) {
+ if (!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
spin_unlock(&dcache_lock);
return -ENOENT;
}
@@ -257,7 +257,7 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
* mount it again.
*/
if (ino->flags & AUTOFS_INF_PENDING ||
- (!d_mountpoint(dentry) && __simple_empty(dentry))) {
+ (!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs))) {
spin_unlock(&dcache_lock);
spin_unlock(&sbi->fs_lock);
@@ -340,8 +340,7 @@ static int autofs4_revalidate(struct dentry *dentry, struct nameidata *nd)
/* Check for a non-mountpoint directory with no contents */
spin_lock(&dcache_lock);
if (S_ISDIR(dentry->d_inode->i_mode) &&
- !d_mountpoint(dentry) &&
- __simple_empty(dentry)) {
+ !d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
DPRINTK("dentry=%p %.*s, emptydir",
dentry, dentry->d_name.len, dentry->d_name.name);
spin_unlock(&dcache_lock);
In autofs4_lookup_active() a declaration within the list traversal
loop uses a declaration that has the same name as the function
parameter.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/root.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index b6530f3..e8a8881 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -419,23 +419,23 @@ static struct dentry *autofs4_lookup_active(struct dentry *dentry)
head = &sbi->active_list;
list_for_each(p, head) {
struct autofs_info *ino;
- struct dentry *dentry;
+ struct dentry *active;
struct qstr *qstr;
ino = list_entry(p, struct autofs_info, active);
- dentry = ino->dentry;
+ active = ino->dentry;
- spin_lock(&dentry->d_lock);
+ spin_lock(&active->d_lock);
/* Already gone? */
- if (atomic_read(&dentry->d_count) == 0)
+ if (atomic_read(&active->d_count) == 0)
goto next;
- qstr = &dentry->d_name;
+ qstr = &active->d_name;
- if (dentry->d_name.hash != hash)
+ if (active->d_name.hash != hash)
goto next;
- if (dentry->d_parent != parent)
+ if (active->d_parent != parent)
goto next;
if (qstr->len != len)
@@ -443,15 +443,15 @@ static struct dentry *autofs4_lookup_active(struct dentry *dentry)
if (memcmp(qstr->name, str, len))
goto next;
- if (d_unhashed(dentry)) {
- dget(dentry);
- spin_unlock(&dentry->d_lock);
+ if (d_unhashed(active)) {
+ dget(active);
+ spin_unlock(&active->d_lock);
spin_unlock(&sbi->lookup_lock);
spin_unlock(&dcache_lock);
- return dentry;
+ return active;
}
next:
- spin_unlock(&dentry->d_lock);
+ spin_unlock(&active->d_lock);
}
spin_unlock(&sbi->lookup_lock);
spin_unlock(&dcache_lock);
In autofs4_lookup_expiring() a declaration within the list traversal
loop uses a declaration that has the same name as the function
parameter.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/root.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index e8a8881..a015b49 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -474,23 +474,23 @@ static struct dentry *autofs4_lookup_expiring(struct dentry *dentry)
head = &sbi->expiring_list;
list_for_each(p, head) {
struct autofs_info *ino;
- struct dentry *dentry;
+ struct dentry *expiring;
struct qstr *qstr;
ino = list_entry(p, struct autofs_info, expiring);
- dentry = ino->dentry;
+ expiring = ino->dentry;
- spin_lock(&dentry->d_lock);
+ spin_lock(&expiring->d_lock);
/* Bad luck, we've already been dentry_iput */
- if (!dentry->d_inode)
+ if (!expiring->d_inode)
goto next;
- qstr = &dentry->d_name;
+ qstr = &expiring->d_name;
- if (dentry->d_name.hash != hash)
+ if (expiring->d_name.hash != hash)
goto next;
- if (dentry->d_parent != parent)
+ if (expiring->d_parent != parent)
goto next;
if (qstr->len != len)
@@ -498,15 +498,15 @@ static struct dentry *autofs4_lookup_expiring(struct dentry *dentry)
if (memcmp(qstr->name, str, len))
goto next;
- if (d_unhashed(dentry)) {
- dget(dentry);
- spin_unlock(&dentry->d_lock);
+ if (d_unhashed(expiring)) {
+ dget(expiring);
+ spin_unlock(&expiring->d_lock);
spin_unlock(&sbi->lookup_lock);
spin_unlock(&dcache_lock);
- return dentry;
+ return expiring;
}
next:
- spin_unlock(&dentry->d_lock);
+ spin_unlock(&expiring->d_lock);
}
spin_unlock(&sbi->lookup_lock);
spin_unlock(&dcache_lock);
We need to be able to cope with the directory mutex being held during
->d_revalidate() in some cases, but not all cases, and not necessarily
by us. Because we need to release the mutex when we call back to the
daemon to do perform a mount we must be sure that it is us who holds
the mutex so we must redirect mount requests to ->lookup() if the mutex
is held.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs4/autofs_i.h | 7 +
fs/autofs4/expire.c | 6 +
fs/autofs4/inode.c | 1
fs/autofs4/root.c | 474 +++++++++++++++++++++++++++++++++----------------
4 files changed, 330 insertions(+), 158 deletions(-)
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 3d283ab..0118d67 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -60,6 +60,11 @@ do { \
current->pid, __func__, ##args); \
} while (0)
+struct rehash_entry {
+ struct task_struct *task;
+ struct list_head list;
+};
+
/* Unified info structure. This is pointed to by both the dentry and
inode structures. Each file in the filesystem has an instance of this
structure. It holds a reference to the dentry, so dentries are never
@@ -76,6 +81,7 @@ struct autofs_info {
struct list_head active;
int active_count;
+ struct list_head rehash_list;
struct list_head expiring;
@@ -98,6 +104,7 @@ struct autofs_info {
#define AUTOFS_INF_EXPIRING (1<<0) /* dentry is in the process of expiring */
#define AUTOFS_INF_MOUNTPOINT (1<<1) /* mountpoint status for direct expire */
#define AUTOFS_INF_PENDING (1<<2) /* dentry pending mount */
+#define AUTOFS_INF_REHASH (1<<3) /* dentry in transit to ->lookup() */
struct autofs_wait_queue {
wait_queue_head_t queue;
diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index a796c94..74bc9aa 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -279,6 +279,7 @@ struct dentry *autofs4_expire_direct(struct super_block *sb,
root->d_mounted--;
}
ino->flags |= AUTOFS_INF_EXPIRING;
+ autofs4_add_expiring(root);
init_completion(&ino->expire_complete);
spin_unlock(&sbi->fs_lock);
return root;
@@ -406,6 +407,7 @@ found:
expired, (int)expired->d_name.len, expired->d_name.name);
ino = autofs4_dentry_ino(expired);
ino->flags |= AUTOFS_INF_EXPIRING;
+ autofs4_add_expiring(expired);
init_completion(&ino->expire_complete);
spin_unlock(&sbi->fs_lock);
spin_lock(&dcache_lock);
@@ -433,7 +435,7 @@ int autofs4_expire_wait(struct dentry *dentry)
DPRINTK("expire done status=%d", status);
- if (d_unhashed(dentry))
+ if (d_unhashed(dentry) && IS_DEADDIR(dentry->d_inode))
return -EAGAIN;
return status;
@@ -473,6 +475,7 @@ int autofs4_expire_run(struct super_block *sb,
spin_lock(&sbi->fs_lock);
ino = autofs4_dentry_ino(dentry);
ino->flags &= ~AUTOFS_INF_EXPIRING;
+ autofs4_del_expiring(dentry);
complete_all(&ino->expire_complete);
spin_unlock(&sbi->fs_lock);
@@ -503,6 +506,7 @@ int autofs4_do_expire_multi(struct super_block *sb, struct vfsmount *mnt,
ino->flags &= ~AUTOFS_INF_MOUNTPOINT;
}
ino->flags &= ~AUTOFS_INF_EXPIRING;
+ autofs4_del_expiring(dentry);
complete_all(&ino->expire_complete);
spin_unlock(&sbi->fs_lock);
dput(dentry);
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index 4670a78..d0a3de2 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -49,6 +49,7 @@ struct autofs_info *autofs4_init_ino(struct autofs_info *ino,
ino->dentry = NULL;
ino->size = 0;
INIT_LIST_HEAD(&ino->active);
+ INIT_LIST_HEAD(&ino->rehash_list);
ino->active_count = 0;
INIT_LIST_HEAD(&ino->expiring);
atomic_set(&ino->count, 0);
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index a015b49..30cc9dd 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -104,6 +104,99 @@ static void autofs4_del_active(struct dentry *dentry)
return;
}
+static void autofs4_add_rehash_entry(struct autofs_info *ino,
+ struct rehash_entry *entry)
+{
+ entry->task = current;
+ INIT_LIST_HEAD(&entry->list);
+ list_add(&entry->list, &ino->rehash_list);
+ return;
+}
+
+static void autofs4_remove_rehash_entry(struct autofs_info *ino)
+{
+ struct list_head *head = &ino->rehash_list;
+ struct rehash_entry *entry;
+ list_for_each_entry(entry, head, list) {
+ if (entry->task == current) {
+ list_del(&entry->list);
+ kfree(entry);
+ break;
+ }
+ }
+ return;
+}
+
+static void autofs4_remove_rehash_entrys(struct autofs_info *ino)
+{
+ struct autofs_sb_info *sbi = ino->sbi;
+ struct rehash_entry *entry, *next;
+ struct list_head *head;
+
+ spin_lock(&sbi->fs_lock);
+ spin_lock(&sbi->lookup_lock);
+ if (!(ino->flags & AUTOFS_INF_REHASH)) {
+ spin_unlock(&sbi->lookup_lock);
+ spin_unlock(&sbi->fs_lock);
+ return;
+ }
+ ino->flags &= ~AUTOFS_INF_REHASH;
+ head = &ino->rehash_list;
+ list_for_each_entry_safe(entry, next, head, list) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
+ spin_unlock(&sbi->lookup_lock);
+ spin_unlock(&sbi->fs_lock);
+ dput(ino->dentry);
+
+ return;
+}
+
+static void autofs4_revalidate_drop(struct dentry *dentry,
+ struct rehash_entry *entry)
+{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ /*
+ * Add to the active list so we can pick this up in
+ * ->lookup(). Also add an entry to a rehash list so
+ * we know when there are no dentrys in flight so we
+ * know when we can rehash the dentry.
+ */
+ spin_lock(&sbi->lookup_lock);
+ if (list_empty(&ino->active))
+ list_add(&ino->active, &sbi->active_list);
+ autofs4_add_rehash_entry(ino, entry);
+ spin_unlock(&sbi->lookup_lock);
+ if (!(ino->flags & AUTOFS_INF_REHASH)) {
+ ino->flags |= AUTOFS_INF_REHASH;
+ dget(dentry);
+ spin_lock(&dentry->d_lock);
+ __d_drop(dentry);
+ spin_unlock(&dentry->d_lock);
+ }
+ return;
+}
+
+static void autofs4_revalidate_rehash(struct dentry *dentry)
+{
+ struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ if (ino->flags & AUTOFS_INF_REHASH) {
+ spin_lock(&sbi->lookup_lock);
+ autofs4_remove_rehash_entry(ino);
+ if (list_empty(&ino->rehash_list)) {
+ spin_unlock(&sbi->lookup_lock);
+ ino->flags &= ~AUTOFS_INF_REHASH;
+ d_rehash(dentry);
+ dput(ino->dentry);
+ } else
+ spin_unlock(&sbi->lookup_lock);
+ }
+ return;
+}
+
static unsigned int autofs4_need_mount(unsigned int flags)
{
unsigned int res = 0;
@@ -143,7 +236,7 @@ out:
return dcache_dir_open(inode, file);
}
-static int try_to_fill_dentry(struct dentry *dentry, int flags)
+static int try_to_fill_dentry(struct dentry *dentry)
{
struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
struct autofs_info *ino = autofs4_dentry_ino(dentry);
@@ -156,55 +249,17 @@ static int try_to_fill_dentry(struct dentry *dentry, int flags)
* Wait for a pending mount, triggering one if there
* isn't one already
*/
- if (dentry->d_inode == NULL) {
- DPRINTK("waiting for mount name=%.*s",
- dentry->d_name.len, dentry->d_name.name);
+ DPRINTK("waiting for mount name=%.*s",
+ dentry->d_name.len, dentry->d_name.name);
- status = autofs4_wait(sbi, dentry, NFY_MOUNT);
+ status = autofs4_wait(sbi, dentry, NFY_MOUNT);
- DPRINTK("mount done status=%d", status);
-
- /* Turn this into a real negative dentry? */
- if (status == -ENOENT) {
- spin_lock(&sbi->fs_lock);
- ino->flags &= ~AUTOFS_INF_PENDING;
- spin_unlock(&sbi->fs_lock);
- return status;
- } else if (status) {
- /* Return a negative dentry, but leave it "pending" */
- return status;
- }
- /* Trigger mount for path component or follow link */
- } else if (ino->flags & AUTOFS_INF_PENDING ||
- autofs4_need_mount(flags) ||
- current->link_count) {
- DPRINTK("waiting for mount name=%.*s",
- dentry->d_name.len, dentry->d_name.name);
+ DPRINTK("mount done status=%d", status);
- spin_lock(&sbi->fs_lock);
- ino->flags |= AUTOFS_INF_PENDING;
- spin_unlock(&sbi->fs_lock);
- status = autofs4_wait(sbi, dentry, NFY_MOUNT);
+ /* Update expiry counter */
+ ino->last_used = jiffies;
- DPRINTK("mount done status=%d", status);
-
- if (status) {
- spin_lock(&sbi->fs_lock);
- ino->flags &= ~AUTOFS_INF_PENDING;
- spin_unlock(&sbi->fs_lock);
- return status;
- }
- }
-
- /* Initialize expiry counter after successful mount */
- if (ino)
- ino->last_used = jiffies;
-
- spin_lock(&sbi->fs_lock);
- ino->flags &= ~AUTOFS_INF_PENDING;
- spin_unlock(&sbi->fs_lock);
-
- return 0;
+ return status;
}
/* For autofs direct mounts the follow link triggers the mount */
@@ -258,10 +313,16 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
*/
if (ino->flags & AUTOFS_INF_PENDING ||
(!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs))) {
+ ino->flags |= AUTOFS_INF_PENDING;
spin_unlock(&dcache_lock);
spin_unlock(&sbi->fs_lock);
- status = try_to_fill_dentry(dentry, 0);
+ status = try_to_fill_dentry(dentry);
+
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
+
if (status)
goto out_error;
@@ -300,18 +361,47 @@ static int autofs4_revalidate(struct dentry *dentry, struct nameidata *nd)
{
struct inode *dir = dentry->d_parent->d_inode;
struct autofs_sb_info *sbi = autofs4_sbi(dir->i_sb);
- int oz_mode = autofs4_oz_mode(sbi);
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ struct rehash_entry *entry;
int flags = nd ? nd->flags : 0;
- int status = 1;
+ unsigned int mutex_aquired;
+
+ DPRINTK("name = %.*s oz_mode = %d",
+ dentry->d_name.len, dentry->d_name.name, oz_mode);
+
+ /* Daemon never causes a mount to trigger */
+ if (autofs4_oz_mode(sbi))
+ return 1;
+
+ entry = kmalloc(sizeof(struct rehash_entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ mutex_aquired = mutex_trylock(&dir->i_mutex);
- /* Pending dentry */
spin_lock(&sbi->fs_lock);
+ spin_lock(&dcache_lock);
+ /* Pending dentry */
if (autofs4_ispending(dentry)) {
- /* The daemon never causes a mount to trigger */
- spin_unlock(&sbi->fs_lock);
+ int status;
- if (oz_mode)
- return 1;
+ /*
+ * We can only unhash and send this to ->lookup() if
+ * the directory mutex is held over d_revalidate() and
+ * ->lookup(). This prevents the VFS from incorrectly
+ * seeing the dentry as non-existent.
+ */
+ ino->flags |= AUTOFS_INF_PENDING;
+ if (!mutex_aquired) {
+ autofs4_revalidate_drop(dentry, entry);
+ spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
+ return 0;
+ }
+ spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
+ mutex_unlock(&dir->i_mutex);
+ kfree(entry);
/*
* If the directory has gone away due to an expire
@@ -325,45 +415,82 @@ static int autofs4_revalidate(struct dentry *dentry, struct nameidata *nd)
* A zero status is success otherwise we have a
* negative error code.
*/
- status = try_to_fill_dentry(dentry, flags);
+ status = try_to_fill_dentry(dentry);
+
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
+
if (status == 0)
return 1;
return status;
}
- spin_unlock(&sbi->fs_lock);
-
- /* Negative dentry.. invalidate if "old" */
- if (dentry->d_inode == NULL)
- return 0;
/* Check for a non-mountpoint directory with no contents */
- spin_lock(&dcache_lock);
if (S_ISDIR(dentry->d_inode->i_mode) &&
!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
DPRINTK("dentry=%p %.*s, emptydir",
dentry, dentry->d_name.len, dentry->d_name.name);
- spin_unlock(&dcache_lock);
- /* The daemon never causes a mount to trigger */
- if (oz_mode)
- return 1;
+ if (autofs4_need_mount(flags) || current->link_count) {
+ int status;
- /*
- * A zero status is success otherwise we have a
- * negative error code.
- */
- status = try_to_fill_dentry(dentry, flags);
- if (status == 0)
- return 1;
+ /*
+ * We can only unhash and send this to ->lookup() if
+ * the directory mutex is held over d_revalidate() and
+ * ->lookup(). This prevents the VFS from incorrectly
+ * seeing the dentry as non-existent.
+ */
+ ino->flags |= AUTOFS_INF_PENDING;
+ if (!mutex_aquired) {
+ autofs4_revalidate_drop(dentry, entry);
+ spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
+ return 0;
+ }
+ spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
+ mutex_unlock(&dir->i_mutex);
+ kfree(entry);
- return status;
+ /*
+ * A zero status is success otherwise we have a
+ * negative error code.
+ */
+ status = try_to_fill_dentry(dentry);
+
+ spin_lock(&sbi->fs_lock);
+ ino->flags &= ~AUTOFS_INF_PENDING;
+ spin_unlock(&sbi->fs_lock);
+
+ if (status == 0)
+ return 1;
+
+ return status;
+ }
}
spin_unlock(&dcache_lock);
+ spin_unlock(&sbi->fs_lock);
+
+ if (mutex_aquired)
+ mutex_unlock(&dir->i_mutex);
+
+ kfree(entry);
return 1;
}
+static void autofs4_free_rehash_entrys(struct autofs_info *inf)
+{
+ struct list_head *head = &inf->rehash_list;
+ struct rehash_entry *entry, *next;
+ list_for_each_entry_safe(entry, next, head, list) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
+}
+
void autofs4_dentry_release(struct dentry *de)
{
struct autofs_info *inf;
@@ -382,6 +509,8 @@ void autofs4_dentry_release(struct dentry *de)
list_del(&inf->active);
if (!list_empty(&inf->expiring))
list_del(&inf->expiring);
+ if (!list_empty(&inf->rehash_list))
+ autofs4_free_rehash_entrys(inf);
spin_unlock(&sbi->lookup_lock);
}
@@ -414,6 +543,7 @@ static struct dentry *autofs4_lookup_active(struct dentry *dentry)
const unsigned char *str = name->name;
struct list_head *p, *head;
+restart:
spin_lock(&dcache_lock);
spin_lock(&sbi->lookup_lock);
head = &sbi->active_list;
@@ -431,6 +561,19 @@ static struct dentry *autofs4_lookup_active(struct dentry *dentry)
if (atomic_read(&active->d_count) == 0)
goto next;
+ if (active->d_inode && IS_DEADDIR(active->d_inode)) {
+ if (!list_empty(&ino->rehash_list)) {
+ dget(active);
+ spin_unlock(&active->d_lock);
+ spin_unlock(&sbi->lookup_lock);
+ spin_unlock(&dcache_lock);
+ autofs4_remove_rehash_entrys(ino);
+ dput(active);
+ goto restart;
+ }
+ goto next;
+ }
+
qstr = &active->d_name;
if (active->d_name.hash != hash)
@@ -443,13 +586,11 @@ static struct dentry *autofs4_lookup_active(struct dentry *dentry)
if (memcmp(qstr->name, str, len))
goto next;
- if (d_unhashed(active)) {
- dget(active);
- spin_unlock(&active->d_lock);
- spin_unlock(&sbi->lookup_lock);
- spin_unlock(&dcache_lock);
- return active;
- }
+ dget(active);
+ spin_unlock(&active->d_lock);
+ spin_unlock(&sbi->lookup_lock);
+ spin_unlock(&dcache_lock);
+ return active;
next:
spin_unlock(&active->d_lock);
}
@@ -498,13 +639,11 @@ static struct dentry *autofs4_lookup_expiring(struct dentry *dentry)
if (memcmp(qstr->name, str, len))
goto next;
- if (d_unhashed(expiring)) {
- dget(expiring);
- spin_unlock(&expiring->d_lock);
- spin_unlock(&sbi->lookup_lock);
- spin_unlock(&dcache_lock);
- return expiring;
- }
+ dget(expiring);
+ spin_unlock(&expiring->d_lock);
+ spin_unlock(&sbi->lookup_lock);
+ spin_unlock(&dcache_lock);
+ return expiring;
next:
spin_unlock(&expiring->d_lock);
}
@@ -514,6 +653,48 @@ next:
return NULL;
}
+static struct autofs_info *init_new_dentry(struct autofs_sb_info *sbi,
+ struct dentry *dentry, int oz_mode)
+{
+ struct autofs_info *ino;
+
+ /*
+ * Mark the dentry incomplete but don't hash it. We do this
+ * to serialize our inode creation operations (symlink and
+ * mkdir) which prevents deadlock during the callback to
+ * the daemon. Subsequent user space lookups for the same
+ * dentry are placed on the wait queue while the daemon
+ * itself is allowed passage unresticted so the create
+ * operation itself can then hash the dentry. Finally,
+ * we check for the hashed dentry and return the newly
+ * hashed dentry.
+ */
+ dentry->d_op = &autofs4_root_dentry_operations;
+
+ /*
+ * And we need to ensure that the same dentry is used for
+ * all following lookup calls until it is hashed so that
+ * the dentry flags are persistent throughout the request.
+ */
+ ino = autofs4_init_ino(NULL, sbi, 0555);
+ if (!ino)
+ return ERR_PTR(-ENOMEM);
+
+ dentry->d_fsdata = ino;
+ ino->dentry = dentry;
+
+ /*
+ * Only set the mount pending flag for new dentrys not created
+ * by the daemon.
+ */
+ if (!oz_mode)
+ ino->flags |= AUTOFS_INF_PENDING;
+
+ d_instantiate(dentry, NULL);
+
+ return ino;
+}
+
/* Lookups in the root directory */
static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
{
@@ -521,6 +702,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
struct autofs_info *ino;
struct dentry *expiring, *active;
int oz_mode;
+ int status = 0;
DPRINTK("name = %.*s",
dentry->d_name.len, dentry->d_name.name);
@@ -535,44 +717,26 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
DPRINTK("pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d",
current->pid, task_pgrp_nr(current), sbi->catatonic, oz_mode);
+ spin_lock(&sbi->fs_lock);
active = autofs4_lookup_active(dentry);
if (active) {
dentry = active;
ino = autofs4_dentry_ino(dentry);
+ /* If this came from revalidate, rehash it */
+ autofs4_revalidate_rehash(dentry);
+ spin_unlock(&sbi->fs_lock);
} else {
- /*
- * Mark the dentry incomplete but don't hash it. We do this
- * to serialize our inode creation operations (symlink and
- * mkdir) which prevents deadlock during the callback to
- * the daemon. Subsequent user space lookups for the same
- * dentry are placed on the wait queue while the daemon
- * itself is allowed passage unresticted so the create
- * operation itself can then hash the dentry. Finally,
- * we check for the hashed dentry and return the newly
- * hashed dentry.
- */
- dentry->d_op = &autofs4_root_dentry_operations;
-
- /*
- * And we need to ensure that the same dentry is used for
- * all following lookup calls until it is hashed so that
- * the dentry flags are persistent throughout the request.
- */
- ino = autofs4_init_ino(NULL, sbi, 0555);
- if (!ino)
- return ERR_PTR(-ENOMEM);
-
- dentry->d_fsdata = ino;
- ino->dentry = dentry;
-
- autofs4_add_active(dentry);
-
- d_instantiate(dentry, NULL);
+ spin_unlock(&sbi->fs_lock);
+ ino = init_new_dentry(sbi, dentry, oz_mode);
+ if (IS_ERR(ino))
+ return (struct dentry *) ino;
}
+ autofs4_add_active(dentry);
+
if (!oz_mode) {
- mutex_unlock(&dir->i_mutex);
expiring = autofs4_lookup_expiring(dentry);
+ mutex_unlock(&dir->i_mutex);
if (expiring) {
/*
* If we are racing with expire the request might not
@@ -580,23 +744,22 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
* so it must have been successful, so just wait for it.
*/
autofs4_expire_wait(expiring);
- autofs4_del_expiring(expiring);
dput(expiring);
}
-
+ status = try_to_fill_dentry(dentry);
+ mutex_lock(&dir->i_mutex);
spin_lock(&sbi->fs_lock);
- ino->flags |= AUTOFS_INF_PENDING;
+ ino->flags &= ~AUTOFS_INF_PENDING;
spin_unlock(&sbi->fs_lock);
- if (dentry->d_op && dentry->d_op->d_revalidate)
- (dentry->d_op->d_revalidate)(dentry, nd);
- mutex_lock(&dir->i_mutex);
}
+ autofs4_del_active(dentry);
+
/*
- * If we are still pending, check if we had to handle
+ * If we had a mount fail, check if we had to handle
* a signal. If so we can force a restart..
*/
- if (ino->flags & AUTOFS_INF_PENDING) {
+ if (status) {
/* See if we were interrupted */
if (signal_pending(current)) {
sigset_t *sigset = ¤t->pending.signal;
@@ -608,43 +771,46 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
return ERR_PTR(-ERESTARTNOINTR);
}
}
- if (!oz_mode) {
- spin_lock(&sbi->fs_lock);
- ino->flags &= ~AUTOFS_INF_PENDING;
- spin_unlock(&sbi->fs_lock);
- }
}
/*
- * If this dentry is unhashed, then we shouldn't honour this
- * lookup. Returning ENOENT here doesn't do the right thing
- * for all system calls, but it should be OK for the operations
- * we permit from an autofs.
+ * User space can (and has done in the past) remove and re-create
+ * this directory during the callback. This can leave us with an
+ * unhashed dentry, but a successful mount! So we need to
+ * perform another cached lookup in case the dentry now exists.
*/
- if (!oz_mode && d_unhashed(dentry)) {
- /*
- * A user space application can (and has done in the past)
- * remove and re-create this directory during the callback.
- * This can leave us with an unhashed dentry, but a
- * successful mount! So we need to perform another
- * cached lookup in case the dentry now exists.
- */
- struct dentry *parent = dentry->d_parent;
- struct dentry *new = d_lookup(parent, &dentry->d_name);
- if (new != NULL)
- dentry = new;
- else
- dentry = ERR_PTR(-ENOENT);
+ if (!oz_mode && !have_submounts(dentry)) {
+ struct dentry *new;
+ new = d_lookup(dentry->d_parent, &dentry->d_name);
+ if (new) {
+ if (active)
+ dput(active);
+ return new;
+ } else {
+ if (!status)
+ status = -ENOENT;
+ }
+ }
+ /*
+ * If we had a mount failure, return status to user space.
+ * If the mount succeeded and we used a dentry from the active queue
+ * return it.
+ */
+ if (status) {
+ dentry = ERR_PTR(status);
if (active)
dput(active);
-
return dentry;
+ } else {
+ /*
+ * Valid successful mount, return active dentry or NULL
+ * for a new dentry.
+ */
+ if (active)
+ return active;
}
- if (active)
- return active;
-
return NULL;
}
@@ -668,8 +834,6 @@ static int autofs4_dir_symlink(struct inode *dir,
if (!ino)
return -ENOMEM;
- autofs4_del_active(dentry);
-
ino->size = strlen(symname);
cp = kmalloc(ino->size + 1, GFP_KERNEL);
if (!cp) {
@@ -746,7 +910,6 @@ static int autofs4_dir_unlink(struct inode *dir, struct dentry *dentry)
dir->i_mtime = CURRENT_TIME;
spin_lock(&dcache_lock);
- autofs4_add_expiring(dentry);
spin_lock(&dentry->d_lock);
__d_drop(dentry);
spin_unlock(&dentry->d_lock);
@@ -772,7 +935,6 @@ static int autofs4_dir_rmdir(struct inode *dir, struct dentry *dentry)
spin_unlock(&dcache_lock);
return -ENOTEMPTY;
}
- autofs4_add_expiring(dentry);
spin_lock(&dentry->d_lock);
__d_drop(dentry);
spin_unlock(&dentry->d_lock);
@@ -810,8 +972,6 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode)
if (!ino)
return -ENOMEM;
- autofs4_del_active(dentry);
-
inode = autofs4_get_inode(dir->i_sb, ino);
if (!inode) {
if (!dentry->d_fsdata)
Ian Kent wrote:
> A change to the VFS path walk locking is needed to resolve an issue
> identified by Sage Weil. This locking change requires significant
> changes to the autofs4 module to allow it to callback to userspace
> without introducing a deadlock.
Oops, meant to say this series is against the current Linus tree and not
the current mm tree. However, I don't expect there will be differences
in the autofs4 patches.
Ian
On Thu, 24 Sep 2009, Ian Kent wrote:
> A change to the VFS path walk locking is needed to resolve an issue
> identified by Sage Weil. This locking change requires significant
> changes to the autofs4 module to allow it to callback to userspace
> without introducing a deadlock.
>
> To cope with the change the autofs4 module needs to redirect mount
> requests from ->d_revalidate() to ->lookup() if the directory
> inode mutex is held when a callback needs to be done. Note that we
> cannot redirect these requests when the mutex is not held because,
> to function correctly, the mutex must be held over both revalidate
> and lookup.
>
> Of the patches in the series most are cleanups and refactoring done
> to keep the real change in "autofs4 - always use lookup for lookup"
> as clean as possible. Unfortuneately, there is still quite a bit
> left in it.
>
> Also, I need confirmation that the patch that changes the VFS path
> walk locking is in fact correct, or at least like for like to what
> will be submitted. I had some difficulty with the original patches
> that were paosted. The patch in question below is "vfs: make
> real_lookup do dentry revalidation with i_mutex held".
It looks identical to be the original two folded into one patch. I'll
repost those two now, freshened against Linus' tree. The first has just
the functional change, and the cleanup is in the second (as per
Christoph's review).
> I've done quite a bit of fairly heavy stress testing of the patch
> series and they (finally) hold up to it. Although I have also
> managed to uncover a locking bug in the user space daemon as a
> result, ;)
I'm glad some other good has come of it. Thanks, Ian, for carrying this
through!
sage
>
> ---
>
> Ian Kent (10):
> autofs4 - always use lookup for lookup
> autofs4 - rename dentry to expiring in autofs4_lookup_expiring()
> autofs4 - rename dentry to active in autofs4_lookup_active()
> autofs4 - eliminate d_unhashed in path walk checks
> autofs4 - cleanup active and expire lookup
> autofs4 - renamer unhashed to active in autofs4_lookup()
> autofs4 - use autofs_info for pending flag
> autofs4 - use macro for need mount check
> autofs4 - use macros for expiring list
> autofs4 - use macros for active list handling
>
> Sage Weil (1):
> Subject: [PATCH] vfs: make real_lookup do dentry revalidation with i_mutex held
>
>
> fs/autofs4/autofs_i.h | 38 +++
> fs/autofs4/expire.c | 8 -
> fs/autofs4/inode.c | 2
> fs/autofs4/root.c | 616 ++++++++++++++++++++++++++++++++-----------------
> fs/namei.c | 58 ++---
> 5 files changed, 480 insertions(+), 242 deletions(-)
>
> --
> Ian
>
>
Sage Weil wrote:
> On Thu, 24 Sep 2009, Ian Kent wrote:
>
>> A change to the VFS path walk locking is needed to resolve an issue
>> identified by Sage Weil. This locking change requires significant
>> changes to the autofs4 module to allow it to callback to userspace
>> without introducing a deadlock.
>>
>> To cope with the change the autofs4 module needs to redirect mount
>> requests from ->d_revalidate() to ->lookup() if the directory
>> inode mutex is held when a callback needs to be done. Note that we
>> cannot redirect these requests when the mutex is not held because,
>> to function correctly, the mutex must be held over both revalidate
>> and lookup.
>>
>> Of the patches in the series most are cleanups and refactoring done
>> to keep the real change in "autofs4 - always use lookup for lookup"
>> as clean as possible. Unfortuneately, there is still quite a bit
>> left in it.
>>
>> Also, I need confirmation that the patch that changes the VFS path
>> walk locking is in fact correct, or at least like for like to what
>> will be submitted. I had some difficulty with the original patches
>> that were paosted. The patch in question below is "vfs: make
>> real_lookup do dentry revalidation with i_mutex held".
>
> It looks identical to be the original two folded into one patch. I'll
> repost those two now, freshened against Linus' tree. The first has just
> the functional change, and the cleanup is in the second (as per
> Christoph's review).
>
>> I've done quite a bit of fairly heavy stress testing of the patch
>> series and they (finally) hold up to it. Although I have also
>> managed to uncover a locking bug in the user space daemon as a
>> result, ;)
>
> I'm glad some other good has come of it. Thanks, Ian, for carrying this
> through!
Not quite so good as I haven't fixed it yet.
But that's user space and it happens occasionally under an extended
heavy load so it will take a while to work out.
Ian
Sage Weil wrote:
> On Thu, 24 Sep 2009, Ian Kent wrote:
>
>> A change to the VFS path walk locking is needed to resolve an issue
>> identified by Sage Weil. This locking change requires significant
>> changes to the autofs4 module to allow it to callback to userspace
>> without introducing a deadlock.
>>
>> To cope with the change the autofs4 module needs to redirect mount
>> requests from ->d_revalidate() to ->lookup() if the directory
>> inode mutex is held when a callback needs to be done. Note that we
>> cannot redirect these requests when the mutex is not held because,
>> to function correctly, the mutex must be held over both revalidate
>> and lookup.
>>
>> Of the patches in the series most are cleanups and refactoring done
>> to keep the real change in "autofs4 - always use lookup for lookup"
>> as clean as possible. Unfortuneately, there is still quite a bit
>> left in it.
>>
>> Also, I need confirmation that the patch that changes the VFS path
>> walk locking is in fact correct, or at least like for like to what
>> will be submitted. I had some difficulty with the original patches
>> that were paosted. The patch in question below is "vfs: make
>> real_lookup do dentry revalidation with i_mutex held".
>
> It looks identical to be the original two folded into one patch. I'll
> repost those two now, freshened against Linus' tree. The first has just
> the functional change, and the cleanup is in the second (as per
> Christoph's review).
Yes, after checking, the patch I was using is functionally the same as
the combination of the two you re-posted. So all is good for may part.
Ian