From: Ian Kent 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 #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(¤t->sighand->siglock, irqflags); - interruptible_sleep_on(&wq->queue); + wait_event_interruptible(wq->queue, wq->name == NULL); spin_lock_irqsave(¤t->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; } _