[PATCH] autofs4: reinstate negatitive timeout of mount fails

[PATCH] autofs4: reinstate negatitive timeout of mount fails

[Ian Kent]

Hi,

Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
hashed dentrys are returned the negative caching of mount failures
stopped working in the autofs4 module for nobrowse mount (ie. directory
created at mount time and removed at umount or following a mount
failure).

This patch keeps track of the dentrys from mount fails in order to be
able check the timeout since the last fail and return the appropriate
status. In addition the timeout value is settable at load time as a
module option and via sysfs using the module
parameter /sys/module/autofs4/parameters/negative_timeout.

Signed-off-by: Ian Kent <raven@themaw.net>

---
--- linux-2.6.23-rc2-mm2/fs/autofs4/init.c.negative-timeout	2007-07-09 07:32:17.000000000 +0800
+++ linux-2.6.23-rc2-mm2/fs/autofs4/init.c	2007-08-21 15:44:34.000000000 +0800
@@ -14,6 +14,10 @@
 #include <linux/init.h>
 #include "autofs_i.h"
 
+unsigned int negative_timeout = AUTOFS_NEGATIVE_TIMEOUT;
+module_param(negative_timeout, uint, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(negative_timeout, "Cache mount fails negatively for this many seconds");
+
 static int autofs_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
--- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout	2007-08-17 11:52:33.000000000 +0800
+++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-21 15:44:34.000000000 +0800
@@ -46,6 +46,7 @@ struct autofs_info *autofs4_init_ino(str
 	ino->inode = NULL;
 	ino->dentry = NULL;
 	ino->size = 0;
+	ino->negative_timeout = negative_timeout;
 
 	INIT_LIST_HEAD(&ino->rehash);
 
@@ -98,11 +99,24 @@ void autofs4_free_ino(struct autofs_info
 static void autofs4_force_release(struct autofs_sb_info *sbi)
 {
 	struct dentry *this_parent = sbi->sb->s_root;
-	struct list_head *next;
+	struct list_head *p, *next;
 
 	if (!sbi->sb->s_root)
 		return;
 
+	/* Cleanup the negative dentry cache */
+	spin_lock(&sbi->rehash_lock);
+	list_for_each_safe(p, next, &sbi->rehash_list) {
+		struct autofs_info *ino;
+		struct dentry *dentry;
+		ino = list_entry(p, struct autofs_info, rehash);
+		dentry = ino->dentry;
+		spin_unlock(&sbi->rehash_lock);
+		dput(ino->dentry);
+		spin_lock(&sbi->rehash_lock);
+	}
+	spin_unlock(&sbi->rehash_lock);
+
 	spin_lock(&dcache_lock);
 repeat:
 	next = this_parent->d_subdirs.next;
--- linux-2.6.23-rc2-mm2/fs/autofs4/autofs_i.h.negative-timeout	2007-08-17 11:52:33.000000000 +0800
+++ linux-2.6.23-rc2-mm2/fs/autofs4/autofs_i.h	2007-08-21 15:44:34.000000000 +0800
@@ -40,6 +40,14 @@
 #define DPRINTK(fmt,args...) do {} while(0)
 #endif
 
+/*
+ * If the daemon returns a negative response (AUTOFS_IOC_FAIL) then we keep
+ * the negative response cached for up to the time given here, although
+ * the time can be shorter if the kernel throws the dcache entry away.
+ */
+#define AUTOFS_NEGATIVE_TIMEOUT	60	/* default 1 minute */
+extern unsigned int negative_timeout;
+
 /* 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
@@ -52,8 +60,16 @@ struct autofs_info {
 
 	int		flags;
 
+	/*
+	 * Two types of unhashed dentry can exist on this list.
+	 * Negative dentrys from failed mounts and positive dentrys
+	 * resulting from a race between expire and mount. This 
+	 * fact is used when looking for dentrys in the list.
+	 */
 	struct list_head rehash;
 
+	unsigned int negative_timeout;
+
 	struct autofs_sb_info *sbi;
 	unsigned long last_used;
 	atomic_t count;
--- linux-2.6.23-rc2-mm2/fs/autofs4/root.c.negative-timeout	2007-08-17 11:53:38.000000000 +0800
+++ linux-2.6.23-rc2-mm2/fs/autofs4/root.c	2007-08-21 15:44:34.000000000 +0800
@@ -238,6 +238,125 @@ out:
 	return dcache_readdir(file, dirent, filldir);
 }
 
+static int autofs4_compare_dentry(struct dentry *parent, struct dentry *dentry, struct qstr *name)
+{
+	unsigned int len = name->len;
+	unsigned int hash = name->hash;
+	const unsigned char *str = name->name;
+	struct qstr *qstr = &dentry->d_name;
+
+	if (dentry->d_name.hash != hash)
+		return 0;
+	if (dentry->d_parent != parent)
+		return 0;
+
+	if (qstr->len != len)
+		return 0;
+	if (memcmp(qstr->name, str, len))
+		return 0;
+
+	return 1;
+}
+
+static struct dentry *autofs4_lookup_dentry(struct autofs_sb_info *sbi, struct dentry *dentry)
+{
+	struct dentry *parent = dentry->d_parent;
+	struct qstr *name = &dentry->d_name;
+	struct list_head *p, *head;
+
+	head = &sbi->rehash_list;
+	list_for_each(p, head) {
+		struct autofs_info *ino;
+		struct dentry *this;
+
+		ino = list_entry(p, struct autofs_info, rehash);
+		this = ino->dentry;
+
+		spin_lock(&this->d_lock);
+
+		if (!autofs4_compare_dentry(parent, this, name))
+			goto next;
+
+		if (d_unhashed(this)) {
+			spin_unlock(&this->d_lock);
+			return this;
+		}
+next:
+		spin_unlock(&this->d_lock);
+	}
+
+	return NULL;
+}
+
+static struct dentry *autofs4_lookup_negative(struct autofs_sb_info *sbi, struct dentry *dentry)
+{
+	struct dentry *parent = dentry->d_parent;
+	struct qstr *name = &dentry->d_name;
+	struct list_head *p, *head;
+
+	spin_lock(&sbi->rehash_lock);
+	head = &sbi->rehash_list;
+	list_for_each(p, head) {
+		struct autofs_info *ino;
+		struct dentry *this;
+
+		ino = list_entry(p, struct autofs_info, rehash);
+		this = ino->dentry;
+
+		spin_lock(&this->d_lock);
+
+		if (this->d_inode)
+			goto next;
+
+		if (!autofs4_compare_dentry(parent, this, name))
+			goto next;
+
+		if (d_unhashed(this)) {
+			if (autofs4_oz_mode(sbi)) {
+				spin_unlock(&this->d_lock);
+				spin_unlock(&sbi->rehash_lock);
+				return this;
+			}
+			/*
+			 * If d_time is non-zero we've had a mount fail.
+			 * Check if it's time to forget about it, return
+			 * fail if not.
+			 */
+			if (this->d_time) {
+				struct autofs_info *p_ino;
+				unsigned long neg_timeout;
+
+				neg_timeout = this->d_time - get_seconds();
+				if (neg_timeout <= ino->negative_timeout) {
+					spin_unlock(&this->d_lock);
+					spin_unlock(&sbi->rehash_lock);
+					return ERR_PTR(-ENOENT);
+				}
+				/* Negative cache timeout */
+				this->d_time = 0;
+				list_del_init(&ino->rehash);
+				spin_unlock(&this->d_lock);
+				spin_unlock(&sbi->rehash_lock);
+				if (atomic_dec_and_test(&ino->count)) {
+					p_ino = autofs4_dentry_ino(this->d_parent);
+					if (p_ino && this->d_parent != this)
+						atomic_dec(&p_ino->count);
+				}
+				dput(this);
+				return NULL;
+			}
+			spin_unlock(&this->d_lock);
+			spin_unlock(&sbi->rehash_lock);
+			return this;
+		}
+next:
+		spin_unlock(&this->d_lock);
+	}
+	spin_unlock(&sbi->rehash_lock);
+
+	return NULL;
+}
+
 static int try_to_fill_dentry(struct dentry *dentry, int flags)
 {
 	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
@@ -282,7 +401,27 @@ static int try_to_fill_dentry(struct den
 
 		/* Turn this into a real negative dentry? */
 		if (status == -ENOENT) {
+			if (!ino) {
+				ino = autofs4_init_ino(ino, sbi, S_IFDIR | 0555);
+				if (ino == NULL)
+					return -ENOSPC;
+				dentry->d_fsdata = ino;
+			}
+			spin_lock(&sbi->rehash_lock);
+			if (!autofs4_lookup_dentry(sbi, dentry)) {
+				struct autofs_info *p_ino;
+				ino->dentry = dget(dentry);
+				atomic_inc(&ino->count);
+				p_ino = autofs4_dentry_ino(dentry->d_parent);
+				if (p_ino && dentry->d_parent != dentry)
+					atomic_inc(&p_ino->count);
+				list_add(&ino->rehash, &sbi->rehash_list);
+			}
+			spin_unlock(&sbi->rehash_lock);
+			/* Set time at which we forget about this mount fial */
 			spin_lock(&dentry->d_lock);
+			ino->negative_timeout = negative_timeout;
+			dentry->d_time = get_seconds() + negative_timeout;
 			dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
 			spin_unlock(&dentry->d_lock);
 			return status;
@@ -305,6 +444,11 @@ static int try_to_fill_dentry(struct den
 
 		if (status) {
 			spin_lock(&dentry->d_lock);
+			/* Set time at which we forget about this mount fial */
+			if (status == -ENOENT) {
+				ino->negative_timeout = negative_timeout;
+				dentry->d_time = get_seconds() + negative_timeout;
+			}
 			dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
 			spin_unlock(&dentry->d_lock);
 			return status;
@@ -316,6 +460,7 @@ static int try_to_fill_dentry(struct den
 		ino->last_used = jiffies;
 
 	spin_lock(&dentry->d_lock);
+	dentry->d_time = 0;
 	dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
 	spin_unlock(&dentry->d_lock);
 	return status;
@@ -339,6 +484,19 @@ static void *autofs4_follow_link(struct 
 	if (oz_mode || !lookup_type)
 		goto done;
 
+	/* Check for cached mount fail, reset d_time if timed out */
+	spin_lock(&dentry->d_lock);
+	if (dentry->d_time) {
+		unsigned long neg_timeout = dentry->d_time - get_seconds();
+		if (neg_timeout <= ino->negative_timeout) {
+			spin_unlock(&dentry->d_lock);
+			status = -ENOENT;
+			goto out_error;
+		}
+		dentry->d_time = 0;
+	}
+	spin_unlock(&dentry->d_lock);
+
 	/* If an expire request is pending wait for it. */
 	if (ino && (ino->flags & AUTOFS_INF_EXPIRING)) {
 		DPRINTK("waiting for active request %p name=%.*s",
@@ -426,9 +584,28 @@ static int autofs4_revalidate(struct den
 		return status;
 	}
 
-	/* Negative dentry.. invalidate if "old" */
-	if (dentry->d_inode == NULL)
-		return 0;
+	/* Check for cached mount fail, reset d_time if timed out */
+	spin_lock(&dentry->d_lock);
+	if (!dentry->d_inode || dentry->d_time) {
+		struct autofs_info *ino = autofs4_dentry_ino(dentry);
+		unsigned long neg_timeout = dentry->d_time - get_seconds();
+		/*
+		 * If d_time is non-zero we've had a mount fail. Check if
+		 * it's time to forget about it, return fail if not.
+		 */
+		if (dentry->d_time && (neg_timeout <= ino->negative_timeout)) {
+			spin_unlock(&dentry->d_lock);
+			return 1;
+		}
+		/* Negative timeout has expired */
+		dentry->d_time = 0;
+
+		if (!dentry->d_inode) {
+			spin_unlock(&dentry->d_lock);
+			return 0;
+		}
+	}
+	spin_unlock(&dentry->d_lock);
 
 	/* Check for a non-mountpoint directory with no contents */
 	spin_lock(&dcache_lock);
@@ -497,9 +674,6 @@ static struct dentry_operations autofs4_
 
 static struct dentry *autofs4_lookup_unhashed(struct autofs_sb_info *sbi, struct dentry *parent, struct qstr *name)
 {
-	unsigned int len = name->len;
-	unsigned int hash = name->hash;
-	const unsigned char *str = name->name;
 	struct list_head *p, *head;
 
 	spin_lock(&dcache_lock);
@@ -508,7 +682,6 @@ static struct dentry *autofs4_lookup_unh
 	list_for_each(p, head) {
 		struct autofs_info *ino;
 		struct dentry *dentry;
-		struct qstr *qstr;
 
 		ino = list_entry(p, struct autofs_info, rehash);
 		dentry = ino->dentry;
@@ -519,20 +692,10 @@ static struct dentry *autofs4_lookup_unh
 		if (!dentry->d_inode)
 			goto next;
 
-		qstr = &dentry->d_name;
-
-		if (dentry->d_name.hash != hash)
-			goto next;
-		if (dentry->d_parent != parent)
-			goto next;
-
-		if (qstr->len != len)
-			goto next;
-		if (memcmp(qstr->name, str, len))
+		if (!autofs4_compare_dentry(parent, dentry, name))
 			goto next;
 
 		if (d_unhashed(dentry)) {
-			struct autofs_info *ino = autofs4_dentry_ino(dentry);
 			struct inode *inode = dentry->d_inode;
 
 			list_del_init(&ino->rehash);
@@ -568,7 +731,7 @@ next:
 static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 {
 	struct autofs_sb_info *sbi;
-	struct dentry *unhashed;
+	struct dentry *unhashed, *negative = NULL;
 	int oz_mode;
 
 	DPRINTK("name = %.*s",
@@ -587,6 +750,22 @@ static struct dentry *autofs4_lookup(str
 	unhashed = autofs4_lookup_unhashed(sbi, dentry->d_parent, &dentry->d_name);
 	if (!unhashed) {
 		/*
+		 * Negative caching of failed "nobrowse" autofs mounts
+		 * require that we keep the unhashed, negative dentrys
+		 * around so we can use them to check failure status. We
+		 * use the rehash_list list in the super bloxk for this
+		 * purpose. This isn't needed for "browse" or direct mounts
+		 * as the dentrys are always positive so the info is always
+		 * available.
+		 */
+		negative = autofs4_lookup_negative(sbi, dentry);
+		if (negative) {
+			if (IS_ERR(negative))
+				return negative;
+			dentry = negative;
+			goto found;
+		}
+		/*
 		 * 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
@@ -599,6 +778,7 @@ static struct dentry *autofs4_lookup(str
 		 */
 		dentry->d_op = &autofs4_root_dentry_operations;
 
+		dentry->d_time = 0;
 		dentry->d_fsdata = NULL;
 		d_instantiate(dentry, NULL);
 	} else {
@@ -621,7 +801,7 @@ static struct dentry *autofs4_lookup(str
 		}
 		dentry = unhashed;
 	}
-
+found:
 	if (!oz_mode) {
 		spin_lock(&dentry->d_lock);
 		dentry->d_flags |= DCACHE_AUTOFS_PENDING;
@@ -682,7 +862,7 @@ static struct dentry *autofs4_lookup(str
 		return dentry;
 	}
 
-	if (unhashed)
+	if (unhashed || negative)
 		return dentry;
 
 	return NULL;
@@ -779,7 +959,9 @@ static int autofs4_dir_unlink(struct ino
 
 	spin_lock(&dcache_lock);
 	spin_lock(&sbi->rehash_lock);
-	list_add(&ino->rehash, &sbi->rehash_list);
+	dentry->d_time = 0;
+	if (!autofs4_lookup_dentry(sbi, dentry))
+		list_add(&ino->rehash, &sbi->rehash_list);
 	spin_unlock(&sbi->rehash_lock);
 	spin_lock(&dentry->d_lock);
 	__d_drop(dentry);
@@ -807,7 +989,9 @@ static int autofs4_dir_rmdir(struct inod
 		return -ENOTEMPTY;
 	}
 	spin_lock(&sbi->rehash_lock);
-	list_add(&ino->rehash, &sbi->rehash_list);
+	dentry->d_time = 0;
+	if (!autofs4_lookup_dentry(sbi, dentry))
+		list_add(&ino->rehash, &sbi->rehash_list);
 	spin_unlock(&sbi->rehash_lock);
 	spin_lock(&dentry->d_lock);
 	__d_drop(dentry);

Re: [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Peter Staubach]

Ian Kent wrote:
> Hi,
>
> Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
> hashed dentrys are returned the negative caching of mount failures
> stopped working in the autofs4 module for nobrowse mount (ie. directory
> created at mount time and removed at umount or following a mount
> failure).
>
> This patch keeps track of the dentrys from mount fails in order to be
> able check the timeout since the last fail and return the appropriate
> status. In addition the timeout value is settable at load time as a
> module option and via sysfs using the module
> parameter /sys/module/autofs4/parameters/negative_timeout.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
>
> ---
> --- linux-2.6.23-rc2-mm2/fs/autofs4/init.c.negative-timeout	2007-07-09 07:32:17.000000000 +0800
> +++ linux-2.6.23-rc2-mm2/fs/autofs4/init.c	2007-08-21 15:44:34.000000000 +0800
> @@ -14,6 +14,10 @@
>  #include <linux/init.h>
>  #include "autofs_i.h"
>  
> +unsigned int negative_timeout = AUTOFS_NEGATIVE_TIMEOUT;
> +module_param(negative_timeout, uint, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(negative_timeout, "Cache mount fails negatively for this many seconds");
> +
>  static int autofs_get_sb(struct file_system_type *fs_type,
>  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
>  {
> --- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout	2007-08-17 11:52:33.000000000 +0800
> +++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-21 15:44:34.000000000 +0800
> @@ -46,6 +46,7 @@ struct autofs_info *autofs4_init_ino(str
>  	ino->inode = NULL;
>  	ino->dentry = NULL;
>  	ino->size = 0;
> +	ino->negative_timeout = negative_timeout;
>  
>  	INIT_LIST_HEAD(&ino->rehash);
>  
> @@ -98,11 +99,24 @@ void autofs4_free_ino(struct autofs_info
>  static void autofs4_force_release(struct autofs_sb_info *sbi)
>  {
>  	struct dentry *this_parent = sbi->sb->s_root;
> -	struct list_head *next;
> +	struct list_head *p, *next;
>  
>  	if (!sbi->sb->s_root)
>  		return;
>  
> +	/* Cleanup the negative dentry cache */
> +	spin_lock(&sbi->rehash_lock);
> +	list_for_each_safe(p, next, &sbi->rehash_list) {
> +		struct autofs_info *ino;
> +		struct dentry *dentry;
> +		ino = list_entry(p, struct autofs_info, rehash);
> +		dentry = ino->dentry;
> +		spin_unlock(&sbi->rehash_lock);
> +		dput(ino->dentry);
>   

Should this be dput(dentry);?

    Thanx...

       ps


> +		spin_lock(&sbi->rehash_lock);
> +	}
> +	spin_unlock(&sbi->rehash_lock);
> +
>  	spin_lock(&dcache_lock);
>  repeat:
>  	next = this_parent->d_subdirs.next;
> --- linux-2.6.23-rc2-mm2/fs/autofs4/autofs_i.h.negative-timeout	2007-08-17 11:52:33.000000000 +0800
> +++ linux-2.6.23-rc2-mm2/fs/autofs4/autofs_i.h	2007-08-21 15:44:34.000000000 +0800
> @@ -40,6 +40,14 @@
>  #define DPRINTK(fmt,args...) do {} while(0)
>  #endif
>  
> +/*
> + * If the daemon returns a negative response (AUTOFS_IOC_FAIL) then we keep
> + * the negative response cached for up to the time given here, although
> + * the time can be shorter if the kernel throws the dcache entry away.
> + */
> +#define AUTOFS_NEGATIVE_TIMEOUT	60	/* default 1 minute */
> +extern unsigned int negative_timeout;
> +
>  /* 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
> @@ -52,8 +60,16 @@ struct autofs_info {
>  
>  	int		flags;
>  
> +	/*
> +	 * Two types of unhashed dentry can exist on this list.
> +	 * Negative dentrys from failed mounts and positive dentrys
> +	 * resulting from a race between expire and mount. This 
> +	 * fact is used when looking for dentrys in the list.
> +	 */
>  	struct list_head rehash;
>  
> +	unsigned int negative_timeout;
> +
>  	struct autofs_sb_info *sbi;
>  	unsigned long last_used;
>  	atomic_t count;
> --- linux-2.6.23-rc2-mm2/fs/autofs4/root.c.negative-timeout	2007-08-17 11:53:38.000000000 +0800
> +++ linux-2.6.23-rc2-mm2/fs/autofs4/root.c	2007-08-21 15:44:34.000000000 +0800
> @@ -238,6 +238,125 @@ out:
>  	return dcache_readdir(file, dirent, filldir);
>  }
>  
> +static int autofs4_compare_dentry(struct dentry *parent, struct dentry *dentry, struct qstr *name)
> +{
> +	unsigned int len = name->len;
> +	unsigned int hash = name->hash;
> +	const unsigned char *str = name->name;
> +	struct qstr *qstr = &dentry->d_name;
> +
> +	if (dentry->d_name.hash != hash)
> +		return 0;
> +	if (dentry->d_parent != parent)
> +		return 0;
> +
> +	if (qstr->len != len)
> +		return 0;
> +	if (memcmp(qstr->name, str, len))
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static struct dentry *autofs4_lookup_dentry(struct autofs_sb_info *sbi, struct dentry *dentry)
> +{
> +	struct dentry *parent = dentry->d_parent;
> +	struct qstr *name = &dentry->d_name;
> +	struct list_head *p, *head;
> +
> +	head = &sbi->rehash_list;
> +	list_for_each(p, head) {
> +		struct autofs_info *ino;
> +		struct dentry *this;
> +
> +		ino = list_entry(p, struct autofs_info, rehash);
> +		this = ino->dentry;
> +
> +		spin_lock(&this->d_lock);
> +
> +		if (!autofs4_compare_dentry(parent, this, name))
> +			goto next;
> +
> +		if (d_unhashed(this)) {
> +			spin_unlock(&this->d_lock);
> +			return this;
> +		}
> +next:
> +		spin_unlock(&this->d_lock);
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct dentry *autofs4_lookup_negative(struct autofs_sb_info *sbi, struct dentry *dentry)
> +{
> +	struct dentry *parent = dentry->d_parent;
> +	struct qstr *name = &dentry->d_name;
> +	struct list_head *p, *head;
> +
> +	spin_lock(&sbi->rehash_lock);
> +	head = &sbi->rehash_list;
> +	list_for_each(p, head) {
> +		struct autofs_info *ino;
> +		struct dentry *this;
> +
> +		ino = list_entry(p, struct autofs_info, rehash);
> +		this = ino->dentry;
> +
> +		spin_lock(&this->d_lock);
> +
> +		if (this->d_inode)
> +			goto next;
> +
> +		if (!autofs4_compare_dentry(parent, this, name))
> +			goto next;
> +
> +		if (d_unhashed(this)) {
> +			if (autofs4_oz_mode(sbi)) {
> +				spin_unlock(&this->d_lock);
> +				spin_unlock(&sbi->rehash_lock);
> +				return this;
> +			}
> +			/*
> +			 * If d_time is non-zero we've had a mount fail.
> +			 * Check if it's time to forget about it, return
> +			 * fail if not.
> +			 */
> +			if (this->d_time) {
> +				struct autofs_info *p_ino;
> +				unsigned long neg_timeout;
> +
> +				neg_timeout = this->d_time - get_seconds();
> +				if (neg_timeout <= ino->negative_timeout) {
> +					spin_unlock(&this->d_lock);
> +					spin_unlock(&sbi->rehash_lock);
> +					return ERR_PTR(-ENOENT);
> +				}
> +				/* Negative cache timeout */
> +				this->d_time = 0;
> +				list_del_init(&ino->rehash);
> +				spin_unlock(&this->d_lock);
> +				spin_unlock(&sbi->rehash_lock);
> +				if (atomic_dec_and_test(&ino->count)) {
> +					p_ino = autofs4_dentry_ino(this->d_parent);
> +					if (p_ino && this->d_parent != this)
> +						atomic_dec(&p_ino->count);
> +				}
> +				dput(this);
> +				return NULL;
> +			}
> +			spin_unlock(&this->d_lock);
> +			spin_unlock(&sbi->rehash_lock);
> +			return this;
> +		}
> +next:
> +		spin_unlock(&this->d_lock);
> +	}
> +	spin_unlock(&sbi->rehash_lock);
> +
> +	return NULL;
> +}
> +
>  static int try_to_fill_dentry(struct dentry *dentry, int flags)
>  {
>  	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
> @@ -282,7 +401,27 @@ static int try_to_fill_dentry(struct den
>  
>  		/* Turn this into a real negative dentry? */
>  		if (status == -ENOENT) {
> +			if (!ino) {
> +				ino = autofs4_init_ino(ino, sbi, S_IFDIR | 0555);
> +				if (ino == NULL)
> +					return -ENOSPC;
> +				dentry->d_fsdata = ino;
> +			}
> +			spin_lock(&sbi->rehash_lock);
> +			if (!autofs4_lookup_dentry(sbi, dentry)) {
> +				struct autofs_info *p_ino;
> +				ino->dentry = dget(dentry);
> +				atomic_inc(&ino->count);
> +				p_ino = autofs4_dentry_ino(dentry->d_parent);
> +				if (p_ino && dentry->d_parent != dentry)
> +					atomic_inc(&p_ino->count);
> +				list_add(&ino->rehash, &sbi->rehash_list);
> +			}
> +			spin_unlock(&sbi->rehash_lock);
> +			/* Set time at which we forget about this mount fial */
>  			spin_lock(&dentry->d_lock);
> +			ino->negative_timeout = negative_timeout;
> +			dentry->d_time = get_seconds() + negative_timeout;
>  			dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
>  			spin_unlock(&dentry->d_lock);
>  			return status;
> @@ -305,6 +444,11 @@ static int try_to_fill_dentry(struct den
>  
>  		if (status) {
>  			spin_lock(&dentry->d_lock);
> +			/* Set time at which we forget about this mount fial */
> +			if (status == -ENOENT) {
> +				ino->negative_timeout = negative_timeout;
> +				dentry->d_time = get_seconds() + negative_timeout;
> +			}
>  			dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
>  			spin_unlock(&dentry->d_lock);
>  			return status;
> @@ -316,6 +460,7 @@ static int try_to_fill_dentry(struct den
>  		ino->last_used = jiffies;
>  
>  	spin_lock(&dentry->d_lock);
> +	dentry->d_time = 0;
>  	dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
>  	spin_unlock(&dentry->d_lock);
>  	return status;
> @@ -339,6 +484,19 @@ static void *autofs4_follow_link(struct 
>  	if (oz_mode || !lookup_type)
>  		goto done;
>  
> +	/* Check for cached mount fail, reset d_time if timed out */
> +	spin_lock(&dentry->d_lock);
> +	if (dentry->d_time) {
> +		unsigned long neg_timeout = dentry->d_time - get_seconds();
> +		if (neg_timeout <= ino->negative_timeout) {
> +			spin_unlock(&dentry->d_lock);
> +			status = -ENOENT;
> +			goto out_error;
> +		}
> +		dentry->d_time = 0;
> +	}
> +	spin_unlock(&dentry->d_lock);
> +
>  	/* If an expire request is pending wait for it. */
>  	if (ino && (ino->flags & AUTOFS_INF_EXPIRING)) {
>  		DPRINTK("waiting for active request %p name=%.*s",
> @@ -426,9 +584,28 @@ static int autofs4_revalidate(struct den
>  		return status;
>  	}
>  
> -	/* Negative dentry.. invalidate if "old" */
> -	if (dentry->d_inode == NULL)
> -		return 0;
> +	/* Check for cached mount fail, reset d_time if timed out */
> +	spin_lock(&dentry->d_lock);
> +	if (!dentry->d_inode || dentry->d_time) {
> +		struct autofs_info *ino = autofs4_dentry_ino(dentry);
> +		unsigned long neg_timeout = dentry->d_time - get_seconds();
> +		/*
> +		 * If d_time is non-zero we've had a mount fail. Check if
> +		 * it's time to forget about it, return fail if not.
> +		 */
> +		if (dentry->d_time && (neg_timeout <= ino->negative_timeout)) {
> +			spin_unlock(&dentry->d_lock);
> +			return 1;
> +		}
> +		/* Negative timeout has expired */
> +		dentry->d_time = 0;
> +
> +		if (!dentry->d_inode) {
> +			spin_unlock(&dentry->d_lock);
> +			return 0;
> +		}
> +	}
> +	spin_unlock(&dentry->d_lock);
>  
>  	/* Check for a non-mountpoint directory with no contents */
>  	spin_lock(&dcache_lock);
> @@ -497,9 +674,6 @@ static struct dentry_operations autofs4_
>  
>  static struct dentry *autofs4_lookup_unhashed(struct autofs_sb_info *sbi, struct dentry *parent, struct qstr *name)
>  {
> -	unsigned int len = name->len;
> -	unsigned int hash = name->hash;
> -	const unsigned char *str = name->name;
>  	struct list_head *p, *head;
>  
>  	spin_lock(&dcache_lock);
> @@ -508,7 +682,6 @@ static struct dentry *autofs4_lookup_unh
>  	list_for_each(p, head) {
>  		struct autofs_info *ino;
>  		struct dentry *dentry;
> -		struct qstr *qstr;
>  
>  		ino = list_entry(p, struct autofs_info, rehash);
>  		dentry = ino->dentry;
> @@ -519,20 +692,10 @@ static struct dentry *autofs4_lookup_unh
>  		if (!dentry->d_inode)
>  			goto next;
>  
> -		qstr = &dentry->d_name;
> -
> -		if (dentry->d_name.hash != hash)
> -			goto next;
> -		if (dentry->d_parent != parent)
> -			goto next;
> -
> -		if (qstr->len != len)
> -			goto next;
> -		if (memcmp(qstr->name, str, len))
> +		if (!autofs4_compare_dentry(parent, dentry, name))
>  			goto next;
>  
>  		if (d_unhashed(dentry)) {
> -			struct autofs_info *ino = autofs4_dentry_ino(dentry);
>  			struct inode *inode = dentry->d_inode;
>  
>  			list_del_init(&ino->rehash);
> @@ -568,7 +731,7 @@ next:
>  static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
>  {
>  	struct autofs_sb_info *sbi;
> -	struct dentry *unhashed;
> +	struct dentry *unhashed, *negative = NULL;
>  	int oz_mode;
>  
>  	DPRINTK("name = %.*s",
> @@ -587,6 +750,22 @@ static struct dentry *autofs4_lookup(str
>  	unhashed = autofs4_lookup_unhashed(sbi, dentry->d_parent, &dentry->d_name);
>  	if (!unhashed) {
>  		/*
> +		 * Negative caching of failed "nobrowse" autofs mounts
> +		 * require that we keep the unhashed, negative dentrys
> +		 * around so we can use them to check failure status. We
> +		 * use the rehash_list list in the super bloxk for this
> +		 * purpose. This isn't needed for "browse" or direct mounts
> +		 * as the dentrys are always positive so the info is always
> +		 * available.
> +		 */
> +		negative = autofs4_lookup_negative(sbi, dentry);
> +		if (negative) {
> +			if (IS_ERR(negative))
> +				return negative;
> +			dentry = negative;
> +			goto found;
> +		}
> +		/*
>  		 * 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
> @@ -599,6 +778,7 @@ static struct dentry *autofs4_lookup(str
>  		 */
>  		dentry->d_op = &autofs4_root_dentry_operations;
>  
> +		dentry->d_time = 0;
>  		dentry->d_fsdata = NULL;
>  		d_instantiate(dentry, NULL);
>  	} else {
> @@ -621,7 +801,7 @@ static struct dentry *autofs4_lookup(str
>  		}
>  		dentry = unhashed;
>  	}
> -
> +found:
>  	if (!oz_mode) {
>  		spin_lock(&dentry->d_lock);
>  		dentry->d_flags |= DCACHE_AUTOFS_PENDING;
> @@ -682,7 +862,7 @@ static struct dentry *autofs4_lookup(str
>  		return dentry;
>  	}
>  
> -	if (unhashed)
> +	if (unhashed || negative)
>  		return dentry;
>  
>  	return NULL;
> @@ -779,7 +959,9 @@ static int autofs4_dir_unlink(struct ino
>  
>  	spin_lock(&dcache_lock);
>  	spin_lock(&sbi->rehash_lock);
> -	list_add(&ino->rehash, &sbi->rehash_list);
> +	dentry->d_time = 0;
> +	if (!autofs4_lookup_dentry(sbi, dentry))
> +		list_add(&ino->rehash, &sbi->rehash_list);
>  	spin_unlock(&sbi->rehash_lock);
>  	spin_lock(&dentry->d_lock);
>  	__d_drop(dentry);
> @@ -807,7 +989,9 @@ static int autofs4_dir_rmdir(struct inod
>  		return -ENOTEMPTY;
>  	}
>  	spin_lock(&sbi->rehash_lock);
> -	list_add(&ino->rehash, &sbi->rehash_list);
> +	dentry->d_time = 0;
> +	if (!autofs4_lookup_dentry(sbi, dentry))
> +		list_add(&ino->rehash, &sbi->rehash_list);
>  	spin_unlock(&sbi->rehash_lock);
>  	spin_lock(&dentry->d_lock);
>  	__d_drop(dentry);
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>   

Re: [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Andrew Morton]

On Tue, 21 Aug 2007 17:26:09 +0800
Ian Kent <raven@themaw.net> wrote:

> Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
> hashed dentrys are returned the negative caching of mount failures
> stopped working in the autofs4 module for nobrowse mount (ie. directory
> created at mount time and removed at umount or following a mount
> failure).
> 
> This patch keeps track of the dentrys from mount fails in order to be
> able check the timeout since the last fail and return the appropriate
> status. In addition the timeout value is settable at load time as a
> module option and via sysfs using the module
> parameter /sys/module/autofs4/parameters/negative_timeout.

Boy, that's a complex-looking patch.  I think I'll sit on this one
for 2.6.24 ;)

It seems to use a lot of list_for_each[_safe] which could
have been coded as list_for_each_entry[_safe], btw.

Re: [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Ian Kent]

On Tue, 2007-08-21 at 10:19 -0400, Peter Staubach wrote:
> Ian Kent wrote:
> > Hi,
> >
> > Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
> > hashed dentrys are returned the negative caching of mount failures
> > stopped working in the autofs4 module for nobrowse mount (ie. directory
> > created at mount time and removed at umount or following a mount
> > failure).
> >
> > This patch keeps track of the dentrys from mount fails in order to be
> > able check the timeout since the last fail and return the appropriate
> > status. In addition the timeout value is settable at load time as a
> > module option and via sysfs using the module
> > parameter /sys/module/autofs4/parameters/negative_timeout.
> >
> > Signed-off-by: Ian Kent <raven@themaw.net>
> >
> > ---
> > --- linux-2.6.23-rc2-mm2/fs/autofs4/init.c.negative-timeout	2007-07-09 07:32:17.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/fs/autofs4/init.c	2007-08-21 15:44:34.000000000 +0800
> > @@ -14,6 +14,10 @@
> >  #include <linux/init.h>
> >  #include "autofs_i.h"
> >  
> > +unsigned int negative_timeout = AUTOFS_NEGATIVE_TIMEOUT;
> > +module_param(negative_timeout, uint, S_IRUGO | S_IWUSR);
> > +MODULE_PARM_DESC(negative_timeout, "Cache mount fails negatively for this many seconds");
> > +
> >  static int autofs_get_sb(struct file_system_type *fs_type,
> >  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
> >  {
> > --- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout	2007-08-17 11:52:33.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-21 15:44:34.000000000 +0800
> > @@ -46,6 +46,7 @@ struct autofs_info *autofs4_init_ino(str
> >  	ino->inode = NULL;
> >  	ino->dentry = NULL;
> >  	ino->size = 0;
> > +	ino->negative_timeout = negative_timeout;
> >  
> >  	INIT_LIST_HEAD(&ino->rehash);
> >  
> > @@ -98,11 +99,24 @@ void autofs4_free_ino(struct autofs_info
> >  static void autofs4_force_release(struct autofs_sb_info *sbi)
> >  {
> >  	struct dentry *this_parent = sbi->sb->s_root;
> > -	struct list_head *next;
> > +	struct list_head *p, *next;
> >  
> >  	if (!sbi->sb->s_root)
> >  		return;
> >  
> > +	/* Cleanup the negative dentry cache */
> > +	spin_lock(&sbi->rehash_lock);
> > +	list_for_each_safe(p, next, &sbi->rehash_list) {
> > +		struct autofs_info *ino;
> > +		struct dentry *dentry;
> > +		ino = list_entry(p, struct autofs_info, rehash);
> > +		dentry = ino->dentry;
> > +		spin_unlock(&sbi->rehash_lock);
> > +		dput(ino->dentry);
> >   
> 
> Should this be dput(dentry);?

It could be since they're the same or maybe I should get rid of the
assignment. Maybe that would save a couple of cpu cycles.

> 
>     Thanx...
> 
>        ps
> 
> 
> > +		spin_lock(&sbi->rehash_lock);
> > +	}
> > +	spin_unlock(&sbi->rehash_lock);
> > +
> >  	spin_lock(&dcache_lock);
> >  repeat:
> >  	next = this_parent->d_subdirs.next;
> > --- linux-2.6.23-rc2-mm2/fs/autofs4/autofs_i.h.negative-timeout	2007-08-17 11:52:33.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/fs/autofs4/autofs_i.h	2007-08-21 15:44:34.000000000 +0800
> > @@ -40,6 +40,14 @@
> >  #define DPRINTK(fmt,args...) do {} while(0)
> >  #endif
> >  
> > +/*
> > + * If the daemon returns a negative response (AUTOFS_IOC_FAIL) then we keep
> > + * the negative response cached for up to the time given here, although
> > + * the time can be shorter if the kernel throws the dcache entry away.
> > + */
> > +#define AUTOFS_NEGATIVE_TIMEOUT	60	/* default 1 minute */
> > +extern unsigned int negative_timeout;
> > +
> >  /* 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
> > @@ -52,8 +60,16 @@ struct autofs_info {
> >  
> >  	int		flags;
> >  
> > +	/*
> > +	 * Two types of unhashed dentry can exist on this list.
> > +	 * Negative dentrys from failed mounts and positive dentrys
> > +	 * resulting from a race between expire and mount. This 
> > +	 * fact is used when looking for dentrys in the list.
> > +	 */
> >  	struct list_head rehash;
> >  
> > +	unsigned int negative_timeout;
> > +
> >  	struct autofs_sb_info *sbi;
> >  	unsigned long last_used;
> >  	atomic_t count;
> > --- linux-2.6.23-rc2-mm2/fs/autofs4/root.c.negative-timeout	2007-08-17 11:53:38.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/fs/autofs4/root.c	2007-08-21 15:44:34.000000000 +0800
> > @@ -238,6 +238,125 @@ out:
> >  	return dcache_readdir(file, dirent, filldir);
> >  }
> >  
> > +static int autofs4_compare_dentry(struct dentry *parent, struct dentry *dentry, struct qstr *name)
> > +{
> > +	unsigned int len = name->len;
> > +	unsigned int hash = name->hash;
> > +	const unsigned char *str = name->name;
> > +	struct qstr *qstr = &dentry->d_name;
> > +
> > +	if (dentry->d_name.hash != hash)
> > +		return 0;
> > +	if (dentry->d_parent != parent)
> > +		return 0;
> > +
> > +	if (qstr->len != len)
> > +		return 0;
> > +	if (memcmp(qstr->name, str, len))
> > +		return 0;
> > +
> > +	return 1;
> > +}
> > +
> > +static struct dentry *autofs4_lookup_dentry(struct autofs_sb_info *sbi, struct dentry *dentry)
> > +{
> > +	struct dentry *parent = dentry->d_parent;
> > +	struct qstr *name = &dentry->d_name;
> > +	struct list_head *p, *head;
> > +
> > +	head = &sbi->rehash_list;
> > +	list_for_each(p, head) {
> > +		struct autofs_info *ino;
> > +		struct dentry *this;
> > +
> > +		ino = list_entry(p, struct autofs_info, rehash);
> > +		this = ino->dentry;
> > +
> > +		spin_lock(&this->d_lock);
> > +
> > +		if (!autofs4_compare_dentry(parent, this, name))
> > +			goto next;
> > +
> > +		if (d_unhashed(this)) {
> > +			spin_unlock(&this->d_lock);
> > +			return this;
> > +		}
> > +next:
> > +		spin_unlock(&this->d_lock);
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static struct dentry *autofs4_lookup_negative(struct autofs_sb_info *sbi, struct dentry *dentry)
> > +{
> > +	struct dentry *parent = dentry->d_parent;
> > +	struct qstr *name = &dentry->d_name;
> > +	struct list_head *p, *head;
> > +
> > +	spin_lock(&sbi->rehash_lock);
> > +	head = &sbi->rehash_list;
> > +	list_for_each(p, head) {
> > +		struct autofs_info *ino;
> > +		struct dentry *this;
> > +
> > +		ino = list_entry(p, struct autofs_info, rehash);
> > +		this = ino->dentry;
> > +
> > +		spin_lock(&this->d_lock);
> > +
> > +		if (this->d_inode)
> > +			goto next;
> > +
> > +		if (!autofs4_compare_dentry(parent, this, name))
> > +			goto next;
> > +
> > +		if (d_unhashed(this)) {
> > +			if (autofs4_oz_mode(sbi)) {
> > +				spin_unlock(&this->d_lock);
> > +				spin_unlock(&sbi->rehash_lock);
> > +				return this;
> > +			}
> > +			/*
> > +			 * If d_time is non-zero we've had a mount fail.
> > +			 * Check if it's time to forget about it, return
> > +			 * fail if not.
> > +			 */
> > +			if (this->d_time) {
> > +				struct autofs_info *p_ino;
> > +				unsigned long neg_timeout;
> > +
> > +				neg_timeout = this->d_time - get_seconds();
> > +				if (neg_timeout <= ino->negative_timeout) {
> > +					spin_unlock(&this->d_lock);
> > +					spin_unlock(&sbi->rehash_lock);
> > +					return ERR_PTR(-ENOENT);
> > +				}
> > +				/* Negative cache timeout */
> > +				this->d_time = 0;
> > +				list_del_init(&ino->rehash);
> > +				spin_unlock(&this->d_lock);
> > +				spin_unlock(&sbi->rehash_lock);
> > +				if (atomic_dec_and_test(&ino->count)) {
> > +					p_ino = autofs4_dentry_ino(this->d_parent);
> > +					if (p_ino && this->d_parent != this)
> > +						atomic_dec(&p_ino->count);
> > +				}
> > +				dput(this);
> > +				return NULL;
> > +			}
> > +			spin_unlock(&this->d_lock);
> > +			spin_unlock(&sbi->rehash_lock);
> > +			return this;
> > +		}
> > +next:
> > +		spin_unlock(&this->d_lock);
> > +	}
> > +	spin_unlock(&sbi->rehash_lock);
> > +
> > +	return NULL;
> > +}
> > +
> >  static int try_to_fill_dentry(struct dentry *dentry, int flags)
> >  {
> >  	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
> > @@ -282,7 +401,27 @@ static int try_to_fill_dentry(struct den
> >  
> >  		/* Turn this into a real negative dentry? */
> >  		if (status == -ENOENT) {
> > +			if (!ino) {
> > +				ino = autofs4_init_ino(ino, sbi, S_IFDIR | 0555);
> > +				if (ino == NULL)
> > +					return -ENOSPC;
> > +				dentry->d_fsdata = ino;
> > +			}
> > +			spin_lock(&sbi->rehash_lock);
> > +			if (!autofs4_lookup_dentry(sbi, dentry)) {
> > +				struct autofs_info *p_ino;
> > +				ino->dentry = dget(dentry);
> > +				atomic_inc(&ino->count);
> > +				p_ino = autofs4_dentry_ino(dentry->d_parent);
> > +				if (p_ino && dentry->d_parent != dentry)
> > +					atomic_inc(&p_ino->count);
> > +				list_add(&ino->rehash, &sbi->rehash_list);
> > +			}
> > +			spin_unlock(&sbi->rehash_lock);
> > +			/* Set time at which we forget about this mount fial */
> >  			spin_lock(&dentry->d_lock);
> > +			ino->negative_timeout = negative_timeout;
> > +			dentry->d_time = get_seconds() + negative_timeout;
> >  			dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
> >  			spin_unlock(&dentry->d_lock);
> >  			return status;
> > @@ -305,6 +444,11 @@ static int try_to_fill_dentry(struct den
> >  
> >  		if (status) {
> >  			spin_lock(&dentry->d_lock);
> > +			/* Set time at which we forget about this mount fial */
> > +			if (status == -ENOENT) {
> > +				ino->negative_timeout = negative_timeout;
> > +				dentry->d_time = get_seconds() + negative_timeout;
> > +			}
> >  			dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
> >  			spin_unlock(&dentry->d_lock);
> >  			return status;
> > @@ -316,6 +460,7 @@ static int try_to_fill_dentry(struct den
> >  		ino->last_used = jiffies;
> >  
> >  	spin_lock(&dentry->d_lock);
> > +	dentry->d_time = 0;
> >  	dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
> >  	spin_unlock(&dentry->d_lock);
> >  	return status;
> > @@ -339,6 +484,19 @@ static void *autofs4_follow_link(struct 
> >  	if (oz_mode || !lookup_type)
> >  		goto done;
> >  
> > +	/* Check for cached mount fail, reset d_time if timed out */
> > +	spin_lock(&dentry->d_lock);
> > +	if (dentry->d_time) {
> > +		unsigned long neg_timeout = dentry->d_time - get_seconds();
> > +		if (neg_timeout <= ino->negative_timeout) {
> > +			spin_unlock(&dentry->d_lock);
> > +			status = -ENOENT;
> > +			goto out_error;
> > +		}
> > +		dentry->d_time = 0;
> > +	}
> > +	spin_unlock(&dentry->d_lock);
> > +
> >  	/* If an expire request is pending wait for it. */
> >  	if (ino && (ino->flags & AUTOFS_INF_EXPIRING)) {
> >  		DPRINTK("waiting for active request %p name=%.*s",
> > @@ -426,9 +584,28 @@ static int autofs4_revalidate(struct den
> >  		return status;
> >  	}
> >  
> > -	/* Negative dentry.. invalidate if "old" */
> > -	if (dentry->d_inode == NULL)
> > -		return 0;
> > +	/* Check for cached mount fail, reset d_time if timed out */
> > +	spin_lock(&dentry->d_lock);
> > +	if (!dentry->d_inode || dentry->d_time) {
> > +		struct autofs_info *ino = autofs4_dentry_ino(dentry);
> > +		unsigned long neg_timeout = dentry->d_time - get_seconds();
> > +		/*
> > +		 * If d_time is non-zero we've had a mount fail. Check if
> > +		 * it's time to forget about it, return fail if not.
> > +		 */
> > +		if (dentry->d_time && (neg_timeout <= ino->negative_timeout)) {
> > +			spin_unlock(&dentry->d_lock);
> > +			return 1;
> > +		}
> > +		/* Negative timeout has expired */
> > +		dentry->d_time = 0;
> > +
> > +		if (!dentry->d_inode) {
> > +			spin_unlock(&dentry->d_lock);
> > +			return 0;
> > +		}
> > +	}
> > +	spin_unlock(&dentry->d_lock);
> >  
> >  	/* Check for a non-mountpoint directory with no contents */
> >  	spin_lock(&dcache_lock);
> > @@ -497,9 +674,6 @@ static struct dentry_operations autofs4_
> >  
> >  static struct dentry *autofs4_lookup_unhashed(struct autofs_sb_info *sbi, struct dentry *parent, struct qstr *name)
> >  {
> > -	unsigned int len = name->len;
> > -	unsigned int hash = name->hash;
> > -	const unsigned char *str = name->name;
> >  	struct list_head *p, *head;
> >  
> >  	spin_lock(&dcache_lock);
> > @@ -508,7 +682,6 @@ static struct dentry *autofs4_lookup_unh
> >  	list_for_each(p, head) {
> >  		struct autofs_info *ino;
> >  		struct dentry *dentry;
> > -		struct qstr *qstr;
> >  
> >  		ino = list_entry(p, struct autofs_info, rehash);
> >  		dentry = ino->dentry;
> > @@ -519,20 +692,10 @@ static struct dentry *autofs4_lookup_unh
> >  		if (!dentry->d_inode)
> >  			goto next;
> >  
> > -		qstr = &dentry->d_name;
> > -
> > -		if (dentry->d_name.hash != hash)
> > -			goto next;
> > -		if (dentry->d_parent != parent)
> > -			goto next;
> > -
> > -		if (qstr->len != len)
> > -			goto next;
> > -		if (memcmp(qstr->name, str, len))
> > +		if (!autofs4_compare_dentry(parent, dentry, name))
> >  			goto next;
> >  
> >  		if (d_unhashed(dentry)) {
> > -			struct autofs_info *ino = autofs4_dentry_ino(dentry);
> >  			struct inode *inode = dentry->d_inode;
> >  
> >  			list_del_init(&ino->rehash);
> > @@ -568,7 +731,7 @@ next:
> >  static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
> >  {
> >  	struct autofs_sb_info *sbi;
> > -	struct dentry *unhashed;
> > +	struct dentry *unhashed, *negative = NULL;
> >  	int oz_mode;
> >  
> >  	DPRINTK("name = %.*s",
> > @@ -587,6 +750,22 @@ static struct dentry *autofs4_lookup(str
> >  	unhashed = autofs4_lookup_unhashed(sbi, dentry->d_parent, &dentry->d_name);
> >  	if (!unhashed) {
> >  		/*
> > +		 * Negative caching of failed "nobrowse" autofs mounts
> > +		 * require that we keep the unhashed, negative dentrys
> > +		 * around so we can use them to check failure status. We
> > +		 * use the rehash_list list in the super bloxk for this
> > +		 * purpose. This isn't needed for "browse" or direct mounts
> > +		 * as the dentrys are always positive so the info is always
> > +		 * available.
> > +		 */
> > +		negative = autofs4_lookup_negative(sbi, dentry);
> > +		if (negative) {
> > +			if (IS_ERR(negative))
> > +				return negative;
> > +			dentry = negative;
> > +			goto found;
> > +		}
> > +		/*
> >  		 * 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
> > @@ -599,6 +778,7 @@ static struct dentry *autofs4_lookup(str
> >  		 */
> >  		dentry->d_op = &autofs4_root_dentry_operations;
> >  
> > +		dentry->d_time = 0;
> >  		dentry->d_fsdata = NULL;
> >  		d_instantiate(dentry, NULL);
> >  	} else {
> > @@ -621,7 +801,7 @@ static struct dentry *autofs4_lookup(str
> >  		}
> >  		dentry = unhashed;
> >  	}
> > -
> > +found:
> >  	if (!oz_mode) {
> >  		spin_lock(&dentry->d_lock);
> >  		dentry->d_flags |= DCACHE_AUTOFS_PENDING;
> > @@ -682,7 +862,7 @@ static struct dentry *autofs4_lookup(str
> >  		return dentry;
> >  	}
> >  
> > -	if (unhashed)
> > +	if (unhashed || negative)
> >  		return dentry;
> >  
> >  	return NULL;
> > @@ -779,7 +959,9 @@ static int autofs4_dir_unlink(struct ino
> >  
> >  	spin_lock(&dcache_lock);
> >  	spin_lock(&sbi->rehash_lock);
> > -	list_add(&ino->rehash, &sbi->rehash_list);
> > +	dentry->d_time = 0;
> > +	if (!autofs4_lookup_dentry(sbi, dentry))
> > +		list_add(&ino->rehash, &sbi->rehash_list);
> >  	spin_unlock(&sbi->rehash_lock);
> >  	spin_lock(&dentry->d_lock);
> >  	__d_drop(dentry);
> > @@ -807,7 +989,9 @@ static int autofs4_dir_rmdir(struct inod
> >  		return -ENOTEMPTY;
> >  	}
> >  	spin_lock(&sbi->rehash_lock);
> > -	list_add(&ino->rehash, &sbi->rehash_list);
> > +	dentry->d_time = 0;
> > +	if (!autofs4_lookup_dentry(sbi, dentry))
> > +		list_add(&ino->rehash, &sbi->rehash_list);
> >  	spin_unlock(&sbi->rehash_lock);
> >  	spin_lock(&dentry->d_lock);
> >  	__d_drop(dentry);
> >
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >   
> 

Re: [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Ian Kent]

On Tue, 2007-08-21 at 13:15 -0700, Andrew Morton wrote:
> On Tue, 21 Aug 2007 17:26:09 +0800
> Ian Kent <raven@themaw.net> wrote:
> 
> > Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
> > hashed dentrys are returned the negative caching of mount failures
> > stopped working in the autofs4 module for nobrowse mount (ie. directory
> > created at mount time and removed at umount or following a mount
> > failure).
> > 
> > This patch keeps track of the dentrys from mount fails in order to be
> > able check the timeout since the last fail and return the appropriate
> > status. In addition the timeout value is settable at load time as a
> > module option and via sysfs using the module
> > parameter /sys/module/autofs4/parameters/negative_timeout.
> 
> Boy, that's a complex-looking patch.  I think I'll sit on this one
> for 2.6.24 ;)

Yes, that's fine .. the principle isn't that complex.

> 
> It seems to use a lot of list_for_each[_safe] which could
> have been coded as list_for_each_entry[_safe], btw.

Mmm .. good point. I've not noticed the list_for_each_entry* macros.

Ian

Re: [autofs] [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Ian Kent]

On Wed, 2007-08-22 at 10:56 +0800, Ian Kent wrote:
> On Tue, 2007-08-21 at 13:15 -0700, Andrew Morton wrote:
> > 
> > It seems to use a lot of list_for_each[_safe] which could
> > have been coded as list_for_each_entry[_safe], btw.
> 
> Mmm .. good point. I've not noticed the list_for_each_entry* macros.

A good idea but that change would cover more than just this patch so I'd
rather leave the patch as is and submit a cleanup patch to cover this
later.

Ian

Re: [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Ian Kent]

On Tue, 2007-08-21 at 10:19 -0400, Peter Staubach wrote:
> Ian Kent wrote:
> > Hi,
> >
> > Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
> > hashed dentrys are returned the negative caching of mount failures
> > stopped working in the autofs4 module for nobrowse mount (ie. directory
> > created at mount time and removed at umount or following a mount
> > failure).
> >
> > This patch keeps track of the dentrys from mount fails in order to be
> > able check the timeout since the last fail and return the appropriate
> > status. In addition the timeout value is settable at load time as a
> > module option and via sysfs using the module
> > parameter /sys/module/autofs4/parameters/negative_timeout.
> >
> > Signed-off-by: Ian Kent <raven@themaw.net>
> >
> > ---
> > --- linux-2.6.23-rc2-mm2/fs/autofs4/init.c.negative-timeout	2007-07-09 07:32:17.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/fs/autofs4/init.c	2007-08-21 15:44:34.000000000 +0800
> > @@ -14,6 +14,10 @@
> >  #include <linux/init.h>
> >  #include "autofs_i.h"
> >  
> > +unsigned int negative_timeout = AUTOFS_NEGATIVE_TIMEOUT;
> > +module_param(negative_timeout, uint, S_IRUGO | S_IWUSR);
> > +MODULE_PARM_DESC(negative_timeout, "Cache mount fails negatively for this many seconds");
> > +
> >  static int autofs_get_sb(struct file_system_type *fs_type,
> >  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
> >  {
> > --- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout	2007-08-17 11:52:33.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-21 15:44:34.000000000 +0800
> > @@ -46,6 +46,7 @@ struct autofs_info *autofs4_init_ino(str
> >  	ino->inode = NULL;
> >  	ino->dentry = NULL;
> >  	ino->size = 0;
> > +	ino->negative_timeout = negative_timeout;
> >  
> >  	INIT_LIST_HEAD(&ino->rehash);
> >  
> > @@ -98,11 +99,24 @@ void autofs4_free_ino(struct autofs_info
> >  static void autofs4_force_release(struct autofs_sb_info *sbi)
> >  {
> >  	struct dentry *this_parent = sbi->sb->s_root;
> > -	struct list_head *next;
> > +	struct list_head *p, *next;
> >  
> >  	if (!sbi->sb->s_root)
> >  		return;
> >  
> > +	/* Cleanup the negative dentry cache */
> > +	spin_lock(&sbi->rehash_lock);
> > +	list_for_each_safe(p, next, &sbi->rehash_list) {
> > +		struct autofs_info *ino;
> > +		struct dentry *dentry;
> > +		ino = list_entry(p, struct autofs_info, rehash);
> > +		dentry = ino->dentry;
> > +		spin_unlock(&sbi->rehash_lock);
> > +		dput(ino->dentry);
> >   
> 
> Should this be dput(dentry);?

Addressed in this simple (first) cleanup patch.

Thanks
Ian

---
--- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout-cleanup	2007-08-22 17:46:33.000000000 +0800
+++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-22 17:47:02.000000000 +0800
@@ -110,7 +110,6 @@ static void autofs4_force_release(struct
 		struct autofs_info *ino;
 		struct dentry *dentry;
 		ino = list_entry(p, struct autofs_info, rehash);
-		dentry = ino->dentry;
 		spin_unlock(&sbi->rehash_lock);
 		dput(ino->dentry);
 		spin_lock(&sbi->rehash_lock);

Re: [autofs] [PATCH] autofs4: reinstate negatitive timeout of mount fails

[Ian Kent]

On Wed, 2007-08-22 at 17:55 +0800, Ian Kent wrote:
> On Tue, 2007-08-21 at 10:19 -0400, Peter Staubach wrote:
> > Ian Kent wrote:
> > > Hi,
> > >
> > > Due to a change to fs/dcache.c:d_lookup() in the 2.6 kernel whereby only
> > > hashed dentrys are returned the negative caching of mount failures
> > > stopped working in the autofs4 module for nobrowse mount (ie. directory
> > > created at mount time and removed at umount or following a mount
> > > failure).
> > >
> > > This patch keeps track of the dentrys from mount fails in order to be
> > > able check the timeout since the last fail and return the appropriate
> > > status. In addition the timeout value is settable at load time as a
> > > module option and via sysfs using the module
> > > parameter /sys/module/autofs4/parameters/negative_timeout.
> > >
> > > Signed-off-by: Ian Kent <raven@themaw.net>
> > >
> > > ---
> > > --- linux-2.6.23-rc2-mm2/fs/autofs4/init.c.negative-timeout	2007-07-09 07:32:17.000000000 +0800
> > > +++ linux-2.6.23-rc2-mm2/fs/autofs4/init.c	2007-08-21 15:44:34.000000000 +0800
> > > @@ -14,6 +14,10 @@
> > >  #include <linux/init.h>
> > >  #include "autofs_i.h"
> > >  
> > > +unsigned int negative_timeout = AUTOFS_NEGATIVE_TIMEOUT;
> > > +module_param(negative_timeout, uint, S_IRUGO | S_IWUSR);
> > > +MODULE_PARM_DESC(negative_timeout, "Cache mount fails negatively for this many seconds");
> > > +
> > >  static int autofs_get_sb(struct file_system_type *fs_type,
> > >  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
> > >  {
> > > --- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout	2007-08-17 11:52:33.000000000 +0800
> > > +++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-21 15:44:34.000000000 +0800
> > > @@ -46,6 +46,7 @@ struct autofs_info *autofs4_init_ino(str
> > >  	ino->inode = NULL;
> > >  	ino->dentry = NULL;
> > >  	ino->size = 0;
> > > +	ino->negative_timeout = negative_timeout;
> > >  
> > >  	INIT_LIST_HEAD(&ino->rehash);
> > >  
> > > @@ -98,11 +99,24 @@ void autofs4_free_ino(struct autofs_info
> > >  static void autofs4_force_release(struct autofs_sb_info *sbi)
> > >  {
> > >  	struct dentry *this_parent = sbi->sb->s_root;
> > > -	struct list_head *next;
> > > +	struct list_head *p, *next;
> > >  
> > >  	if (!sbi->sb->s_root)
> > >  		return;
> > >  
> > > +	/* Cleanup the negative dentry cache */
> > > +	spin_lock(&sbi->rehash_lock);
> > > +	list_for_each_safe(p, next, &sbi->rehash_list) {
> > > +		struct autofs_info *ino;
> > > +		struct dentry *dentry;
> > > +		ino = list_entry(p, struct autofs_info, rehash);
> > > +		dentry = ino->dentry;
> > > +		spin_unlock(&sbi->rehash_lock);
> > > +		dput(ino->dentry);
> > >   
> > 
> > Should this be dput(dentry);?
> 
> Addressed in this simple (first) cleanup patch.
> 

Scratch that first cleanup.
I missed removing the definition of the no longer used dentry.
Please use this instead.
Ian

---
--- linux-2.6.23-rc2-mm2/fs/autofs4/inode.c.negative-timeout-cleanup	2007-08-22 17:46:33.000000000 +0800
+++ linux-2.6.23-rc2-mm2/fs/autofs4/inode.c	2007-08-22 18:57:49.000000000 +0800
@@ -108,9 +108,7 @@ static void autofs4_force_release(struct
 	spin_lock(&sbi->rehash_lock);
 	list_for_each_safe(p, next, &sbi->rehash_list) {
 		struct autofs_info *ino;
-		struct dentry *dentry;
 		ino = list_entry(p, struct autofs_info, rehash);
-		dentry = ino->dentry;
 		spin_unlock(&sbi->rehash_lock);
 		dput(ino->dentry);
 		spin_lock(&sbi->rehash_lock);