On Wed, 2009-03-25 at 20:53 -0700, Sage Weil wrote:
> So, it sounds like this fix would need to go in along with an autofs patch
> that moves the upcall back into lookup exclusively, now that a revalidate
> failure does the right thing (always calls lookup). Hopefully that would
> mean a net simplification on the autofs side as well?
Here is a quick and totally untested patch.
I doubt very much that I've got this right yet but it is something like
what I will need to do.
I can easily build this as a module and install it.
I wonder what would happen if I was to use this without your two
patches. That would give me some quick feedback on potential problems.
Thoughts?
Ian
autofs4 - always use lookup for mount
From: Ian Kent <raven@xxxxxxxxxx>
---
fs/autofs4/autofs_i.h | 15 ----
fs/autofs4/root.c | 199 ++++++++++++++++++++++++++-----------------------
2 files changed, 106 insertions(+), 108 deletions(-)
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index b7ff33c..1fd9183 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -95,6 +95,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_REVALIDATE (1<<2) /* revalidate sent this mount request */
struct autofs_wait_queue {
wait_queue_head_t queue;
@@ -156,20 +157,6 @@ static inline int autofs4_oz_mode(struct autofs_sb_info *sbi) {
return sbi->catatonic || task_pgrp_nr(current) == sbi->oz_pgrp;
}
-/* Does a dentry have some pending activity? */
-static inline int autofs4_ispending(struct dentry *dentry)
-{
- struct autofs_info *inf = autofs4_dentry_ino(dentry);
-
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING)
- return 1;
-
- if (inf->flags & AUTOFS_INF_EXPIRING)
- return 1;
-
- return 0;
-}
-
static inline void autofs4_copy_atime(struct file *src, struct file *dst)
{
dst->f_path.dentry->d_inode->i_atime =
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index e8c55d2..397ca66 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -72,6 +72,14 @@ const struct inode_operations autofs4_dir_inode_operations = {
.rmdir = autofs4_dir_rmdir,
};
+static unsigned int autofs4_need_lookup(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;
@@ -103,7 +111,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);
@@ -116,55 +124,18 @@ 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(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_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 ||
- flags & (TRIGGER_FLAGS | TRIGGER_INTENTS) ||
- 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);
- 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);
- return status;
- }
- }
+ DPRINTK("mount done status=%d", status);
/* Initialize expiry counter after successful mount */
- if (ino)
+ if (ino && !status)
ino->last_used = jiffies;
- spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
-
- return 0;
+ return status;
}
/* For autofs direct mounts the follow link triggers the mount */
@@ -216,7 +187,16 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
(!d_mountpoint(dentry) && __simple_empty(dentry))) {
spin_unlock(&dcache_lock);
- status = try_to_fill_dentry(dentry, 0);
+ spin_lock(&dentry->d_lock);
+ dentry->d_flags |= DCACHE_AUTOFS_PENDING;
+ spin_unlock(&dentry->d_lock);
+
+ status = try_to_fill_dentry(dentry);
+
+ spin_lock(&dentry->d_lock);
+ dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
+ spin_unlock(&dentry->d_lock);
+
if (status)
goto out_error;
@@ -255,38 +235,39 @@ 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);
- int flags = nd ? nd->flags : 0;
+ unsigned int flags = nd ? nd->flags : 0;
int status = 1;
- /* Pending dentry */
- spin_lock(&sbi->fs_lock);
- if (autofs4_ispending(dentry)) {
- /* The daemon never causes a mount to trigger */
- spin_unlock(&sbi->fs_lock);
-
- if (oz_mode)
- return 1;
+ DPRINTK("name = %.*s oz_mode = %d flags = %d",
+ dentry->d_name.len, dentry->d_name.name,
+ autofs4_oz_mode(sbi), nd ? nd->flags : 0);
- /*
- * If the directory has gone away due to an expire
- * we have been called as ->d_revalidate() and so
- * we need to return false and proceed to ->lookup().
- */
- if (autofs4_expire_wait(dentry) == -EAGAIN)
- return 0;
+ /* The daemon never causes a mount to trigger */
+ if (autofs4_oz_mode(sbi))
+ return 1;
+ spin_lock(&dentry->d_lock);
+ if (dentry->d_flags & DCACHE_AUTOFS_PENDING) {
+ spin_unlock(&dentry->d_lock);
/*
- * A zero status is success otherwise we have a
- * negative error code.
+ * If the directory mutex lock is held we must hide the
+ * dentry and proceed to ->lookup() to wait for mount
+ * completion since we cannot know whether the lock was
+ * taken by this code path or is held by another.
+ * Otherwise we can just wait for mount completion.
*/
- status = try_to_fill_dentry(dentry, flags);
- if (status == 0)
- return 1;
+ status = 0;
+ if (mutex_is_locked(&dir->i_mutex)) {
+ /* Already pending, send to ->lookup() */
+ d_drop(dentry);
+ } else {
+ if (autofs4_expire_wait(dentry) != -EAGAIN)
+ status = try_to_fill_dentry(dentry);
+ }
return status;
}
- spin_unlock(&sbi->fs_lock);
+ spin_unlock(&dentry->d_lock);
/* Negative dentry.. invalidate if "old" */
if (dentry->d_inode == NULL)
@@ -299,19 +280,23 @@ static int autofs4_revalidate(struct dentry *dentry, struct nameidata *nd)
__simple_empty(dentry)) {
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;
- /*
- * A zero status is success otherwise we have a
- * negative error code.
- */
- status = try_to_fill_dentry(dentry, flags);
- if (status == 0)
- return 1;
+ /* Always send new mount requests to lookup */
+ status = 1;
+ spin_lock(&sbi->fs_lock);
+ if (autofs4_need_lookup(flags) || current->link_count) {
+ struct autofs_info *ino = autofs4_dentry_ino(dentry);
+ ino->flags |= AUTOFS_INF_REVALIDATE;
+ spin_lock(&sbi->lookup_lock);
+ list_add(&ino->active, &sbi->active_list);
+ spin_unlock(&sbi->lookup_lock);
+ spin_lock(&dentry->d_lock);
+ __d_drop(dentry);
+ spin_unlock(&dentry->d_lock);
+ status = 0;
+ }
+ spin_unlock(&sbi->fs_lock);
+ spin_unlock(&dcache_lock);
return status;
}
@@ -471,6 +456,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
struct autofs_info *ino;
struct dentry *expiring, *unhashed;
int oz_mode;
+ int status = 0;
DPRINTK("name = %.*s",
dentry->d_name.len, dentry->d_name.name);
@@ -486,9 +472,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
@@ -522,6 +509,10 @@ 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);
+
mutex_unlock(&dir->i_mutex);
expiring = autofs4_lookup_expiring(sbi,
dentry->d_parent,
@@ -541,19 +532,34 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
dput(expiring);
}
+ status = try_to_fill_dentry(dentry);
+
+ mutex_lock(&dir->i_mutex);
+
spin_lock(&dentry->d_lock);
- dentry->d_flags |= DCACHE_AUTOFS_PENDING;
+ dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
spin_unlock(&dentry->d_lock);
- if (dentry->d_op && dentry->d_op->d_revalidate)
- (dentry->d_op->d_revalidate)(dentry, nd);
- mutex_lock(&dir->i_mutex);
+
+ /*
+ * If we got a new lookup request from revaidate, remove
+ * it from the active queue and rehash the dentry.
+ */
+ spin_lock(&sbi->fs_lock);
+ if (ino->flags & AUTOFS_INF_REVALIDATE) {
+ spin_lock(&sbi->lookup_lock);
+ if (!list_empty(&ino->active))
+ list_del_init(&ino->active);
+ spin_unlock(&sbi->lookup_lock);
+ d_rehash(dentry);
+ }
+ spin_unlock(&sbi->fs_lock);
}
/*
- * 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 (dentry->d_flags & DCACHE_AUTOFS_PENDING) {
+ if (status) {
/* See if we were interrupted */
if (signal_pending(current)) {
sigset_t *sigset = ¤t->pending.signal;
@@ -565,11 +571,6 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
return ERR_PTR(-ERESTARTNOINTR);
}
}
- if (!oz_mode) {
- spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
- spin_unlock(&dentry->d_lock);
- }
}
/*
@@ -599,7 +600,17 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
return dentry;
}
- if (unhashed)
+ /*
+ * 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 (unhashed)
+ dput(unhashed);
+ return dentry;
+ } else if (unhashed)
return unhashed;
return NULL;
--
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
