From: Ian Kent <raven@themaw.net>

Remove BKL from autofs4 module and add spinlock to serialise access to the
automount daemon communication waitq.

Locking requirements are different in 2.6 and so I'm seeking comments and
suggestions on this.  I have taken a rather heavy handed approach to this in
the patch.  For example, the VFS operations that directly change the
filesystem, such as autofs4_mkdir etc, hold the inode semaphore on entry so
the BKL has been removed.  I can't see why two locking mechanisms are needed.
 Rather than add locking all over the place, I'm looking for justification
it's needed, as I don't see it myself.


---

 25-akpm/fs/autofs4/autofs_i.h |    2 +-
 25-akpm/fs/autofs4/root.c     |   38 ++++++--------------------------------
 25-akpm/fs/autofs4/waitq.c    |   32 ++++++++++++++++++++++----------
 3 files changed, 29 insertions(+), 43 deletions(-)

diff -puN fs/autofs4/autofs_i.h~3-autofs4-2.6.0-bkl-20040405 fs/autofs4/autofs_i.h
--- 25/fs/autofs4/autofs_i.h~3-autofs4-2.6.0-bkl-20040405	Tue Apr  6 15:50:30 2004
+++ 25-akpm/fs/autofs4/autofs_i.h	Tue Apr  6 15:50:30 2004
@@ -82,7 +82,7 @@ struct autofs_wait_queue {
 	char *name;
 	/* This is for status reporting upon return */
 	int status;
-	int wait_ctr;
+	atomic_t wait_ctr;
 };
 
 #define AUTOFS_SBI_MAGIC 0x6d4a556d
diff -puN fs/autofs4/root.c~3-autofs4-2.6.0-bkl-20040405 fs/autofs4/root.c
--- 25/fs/autofs4/root.c~3-autofs4-2.6.0-bkl-20040405	Tue Apr  6 15:50:30 2004
+++ 25-akpm/fs/autofs4/root.c	Tue Apr  6 15:50:30 2004
@@ -251,7 +251,6 @@ static struct dentry *autofs4_root_looku
 
 	sbi = autofs4_sbi(dir->i_sb);
 
-	lock_kernel();
 	oz_mode = autofs4_oz_mode(sbi);
 	DPRINTK(("autofs4_lookup: pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d\n",
 		 current->pid, process_group(current), sbi->catatonic, oz_mode));
@@ -290,12 +289,10 @@ static struct dentry *autofs4_root_looku
 			if (sigismember (sigset, SIGKILL) ||
 			    sigismember (sigset, SIGQUIT) ||
 			    sigismember (sigset, SIGINT)) {
-			    unlock_kernel();
 			    return ERR_PTR(-ERESTARTNOINTR);
 			}
 		}
 	}
-	unlock_kernel();
 
 	/*
 	 * If this dentry is unhashed, then we shouldn't honour this
@@ -321,24 +318,18 @@ static int autofs4_dir_symlink(struct in
 	DPRINTK(("autofs4_dir_symlink: %s <- %.*s\n", symname,
 		 dentry->d_name.len, dentry->d_name.name));
 
-	lock_kernel();
-	if (!autofs4_oz_mode(sbi)) {
-		unlock_kernel();
+	if (!autofs4_oz_mode(sbi))
 		return -EACCES;
-	}
 
 	ino = autofs4_init_ino(ino, sbi, S_IFLNK | 0555);
-	if (ino == NULL) {
-		unlock_kernel();
+	if (ino == NULL)
 		return -ENOSPC;
-	}
 
 	ino->size = strlen(symname);
 	ino->u.symlink = cp = kmalloc(ino->size + 1, GFP_KERNEL);
 
 	if (cp == NULL) {
 		kfree(ino);
-		unlock_kernel();
 		return -ENOSPC;
 	}
 
@@ -358,7 +349,6 @@ static int autofs4_dir_symlink(struct in
 
 	dir->i_mtime = CURRENT_TIME;
 
-	unlock_kernel();
 	return 0;
 }
 
@@ -383,11 +373,8 @@ static int autofs4_dir_unlink(struct ino
 	struct autofs_info *ino = autofs4_dentry_ino(dentry);
 	
 	/* This allows root to remove symlinks */
-	lock_kernel();
-	if ( !autofs4_oz_mode(sbi) && !capable(CAP_SYS_ADMIN) ) {
-		unlock_kernel();
+	if ( !autofs4_oz_mode(sbi) && !capable(CAP_SYS_ADMIN) )
 		return -EACCES;
-	}
 
 	dput(ino->dentry);
 
@@ -398,8 +385,6 @@ static int autofs4_dir_unlink(struct ino
 
 	d_drop(dentry);
 
-	unlock_kernel();
-	
 	return 0;
 }
 
@@ -408,16 +393,12 @@ static int autofs4_dir_rmdir(struct inod
 	struct autofs_sb_info *sbi = autofs4_sbi(dir->i_sb);
 	struct autofs_info *ino = autofs4_dentry_ino(dentry);
 	
-	lock_kernel();
-	if (!autofs4_oz_mode(sbi)) {
-		unlock_kernel();
+	if (!autofs4_oz_mode(sbi))
 		return -EACCES;
-	}
 
 	spin_lock(&dcache_lock);
 	if (!list_empty(&dentry->d_subdirs)) {
 		spin_unlock(&dcache_lock);
-		unlock_kernel();
 		return -ENOTEMPTY;
 	}
 	__d_drop(dentry);
@@ -431,7 +412,6 @@ static int autofs4_dir_rmdir(struct inod
 	if (dir->i_nlink)
 		dir->i_nlink--;
 
-	unlock_kernel();
 	return 0;
 }
 
@@ -443,20 +423,15 @@ static int autofs4_dir_mkdir(struct inod
 	struct autofs_info *ino = autofs4_dentry_ino(dentry);
 	struct inode *inode;
 
-	lock_kernel();
-	if ( !autofs4_oz_mode(sbi) ) {
-		unlock_kernel();
+	if ( !autofs4_oz_mode(sbi) )
 		return -EACCES;
-	}
 
 	DPRINTK(("autofs4_dir_mkdir: dentry %p, creating %.*s\n",
 		 dentry, dentry->d_name.len, dentry->d_name.name));
 
 	ino = autofs4_init_ino(ino, sbi, S_IFDIR | 0555);
-	if (ino == NULL) {
-		unlock_kernel();
+	if (ino == NULL)
 		return -ENOSPC;
-	}
 
 	inode = autofs4_get_inode(dir->i_sb, ino);
 	d_instantiate(dentry, inode);
@@ -472,7 +447,6 @@ static int autofs4_dir_mkdir(struct inod
 	dir->i_nlink++;
 	dir->i_mtime = CURRENT_TIME;
 
-	unlock_kernel();
 	return 0;
 }
 
diff -puN fs/autofs4/waitq.c~3-autofs4-2.6.0-bkl-20040405 fs/autofs4/waitq.c
--- 25/fs/autofs4/waitq.c~3-autofs4-2.6.0-bkl-20040405	Tue Apr  6 15:50:30 2004
+++ 25-akpm/fs/autofs4/waitq.c	Tue Apr  6 15:50:30 2004
@@ -16,6 +16,8 @@
 #include <linux/file.h>
 #include "autofs_i.h"
 
+static spinlock_t waitq_lock = SPIN_LOCK_UNLOCKED;
+
 /* We make this a static variable rather than a part of the superblock; it
    is better if we don't reassign numbers easily even across filesystems */
 static autofs_wqt_t autofs4_next_wait_queue = 1;
@@ -37,7 +39,7 @@ void autofs4_catatonic_mode(struct autof
 		wq->status = -ENOENT; /* Magic is gone - report failure */
 		kfree(wq->name);
 		wq->name = NULL;
-		wake_up(&wq->queue);
+		wake_up_interruptible(&wq->queue);
 		wq = nwq;
 	}
 	if (sbi->pipe) {
@@ -138,12 +140,14 @@ int autofs4_wait(struct autofs_sb_info *
 	if ( name->len > NAME_MAX )
 		return -ENOENT;
 
+	spin_lock(&waitq_lock);
 	for ( wq = sbi->queues ; wq ; wq = wq->next ) {
 		if ( wq->hash == name->hash &&
 		     wq->len == name->len &&
 		     wq->name && !memcmp(wq->name,name->name,name->len) )
 			break;
 	}
+	spin_unlock(&waitq_lock);
 	
 	if ( !wq ) {
 		/* Create a new wait queue */
@@ -156,21 +160,24 @@ int autofs4_wait(struct autofs_sb_info *
 			kfree(wq);
 			return -ENOMEM;
 		}
+
+		spin_lock(&waitq_lock);
 		wq->wait_queue_token = autofs4_next_wait_queue;
 		if (++autofs4_next_wait_queue == 0)
 			autofs4_next_wait_queue = 1;
+		wq->next = sbi->queues;
+		sbi->queues = wq;
+		spin_unlock(&waitq_lock);
 		init_waitqueue_head(&wq->queue);
 		wq->hash = name->hash;
 		wq->len = name->len;
 		wq->status = -EINTR; /* Status return if interrupted */
 		memcpy(wq->name, name->name, name->len);
-		wq->next = sbi->queues;
-		sbi->queues = wq;
 
 		DPRINTK(("autofs4_wait: new wait id = 0x%08lx, name = %.*s, nfy=%d\n",
 			 (unsigned long) wq->wait_queue_token, wq->len, wq->name, notify));
 		/* autofs4_notify_daemon() may block */
-		wq->wait_ctr = 2;
+		atomic_set(&wq->wait_ctr, 2);
 		if (notify != NFY_NONE) {
 			autofs4_notify_daemon(sbi,wq, 
 					notify == NFY_MOUNT ?
@@ -178,7 +185,7 @@ int autofs4_wait(struct autofs_sb_info *
 						  autofs_ptype_expire_multi);
 		}
 	} else {
-		wq->wait_ctr++;
+		atomic_inc(&wq->wait_ctr);
 		DPRINTK(("autofs4_wait: existing wait id = 0x%08lx, name = %.*s, nfy=%d\n",
 			 (unsigned long) wq->wait_queue_token, wq->len, wq->name, notify));
 	}
@@ -205,7 +212,7 @@ int autofs4_wait(struct autofs_sb_info *
 		recalc_sigpending();
 		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
 
-		interruptible_sleep_on(&wq->queue);
+		wait_event_interruptible(wq->queue, wq->name == NULL);
 
 		spin_lock_irqsave(&current->sighand->siglock, irqflags);
 		current->blocked = oldset;
@@ -217,7 +224,7 @@ int autofs4_wait(struct autofs_sb_info *
 
 	status = wq->status;
 
-	if (--wq->wait_ctr == 0)	/* Are we the last process to need status? */
+	if (atomic_dec_and_test(&wq->wait_ctr))	/* Are we the last process to need status? */
 		kfree(wq);
 
 	return status;
@@ -228,23 +235,28 @@ int autofs4_wait_release(struct autofs_s
 {
 	struct autofs_wait_queue *wq, **wql;
 
+	spin_lock(&waitq_lock);
 	for ( wql = &sbi->queues ; (wq = *wql) ; wql = &wq->next ) {
 		if ( wq->wait_queue_token == wait_queue_token )
 			break;
 	}
-	if ( !wq )
+
+	if ( !wq ) {
+		spin_unlock(&waitq_lock);
 		return -EINVAL;
+	}
 
 	*wql = wq->next;	/* Unlink from chain */
+	spin_unlock(&waitq_lock);
 	kfree(wq->name);
 	wq->name = NULL;	/* Do not wait on this queue */
 
 	wq->status = status;
 
-	if (--wq->wait_ctr == 0)	/* Is anyone still waiting for this guy? */
+	if (atomic_dec_and_test(&wq->wait_ctr))	/* Is anyone still waiting for this guy? */
 		kfree(wq);
 	else
-		wake_up(&wq->queue);
+		wake_up_interruptible(&wq->queue);
 
 	return 0;
 }

_