locks.c: removal of semaphores

Andrew,
I got 5250 Req/s with your locks-sem.patch on normal Apache. It is good performance on normal Apache.

Followings are the recent data with/without serialization.

w/ serialize w/o serialize
240t10pre5 2237 5358
240t10pre5+P2 5253 5355**
240t10pre5+P3 --- NG
240t10pre5+locksem 5250 ---
**: once we found deadlock
NG: cannot complete measurement
--: we've not measured.

Normal apache on various kernel setting as follows:

We also did durability test of 2.4.0-test10-pre5. Unfortunately enough, we didn't successfully complete the test of Apache w/o serialization (-DSINGLE_LISTEN_UNSERIALIZED_ACCEPT), it couldn't continue to run for a night. The kernel got complete deadlock.

The message is:
"Unable to handle kernel NULL pointer dereference NMI watchdog detected LOCKUP on CPU1."

Yes, obviously it's not Andrew's problem, that is genuine test10-pre5.

Hidden bugs are awakened by removing serialization.

If the bug is same as what I observed, It is NULL pointer dereference on run-queue list.
--
Computer Systems Laboratory, Fujitsu Labs. [email protected]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Great. Thanks again. Trond has this patch now for ongoing NFS locking stuff. Hopefully 2.4 will now work OK with "legacy" Apache configurations.

As long as the "new style" Apache configurations work OK. Which leads us to...

(Places fingertips lightly upon spinning disk drive.
Closes eyes. Quietly hums low monotone. Time passes...)

I sense a corrupted struct timer_list!

Your kernel was traversing the tvecs[] array and took a fault. The fault handler called printk() which called the console code which called mod_timer() which deadlocked on timerlist_lock. The NMI handler broke the deadlock and called the console code. The NMI handler also deadlocked on timerlist_lock.

If this theory is correct the below patch should remove the deadlock and thus allow you to get the usual diagnostics.

But I suspect the diagnostics will just show a trace up into cascade_timers() or run_timer_list() which doesn't tell us much.

If that is the case I'm afraid you will have to identify the _exact_ statement where the error occured, put some diagnostic code just prior to it and run the tests a second time.

The interesting piece of information will be the timer's function pointer. So you'll need to add something like:

	struct timer_list *some_timer;	/* The timer which causes the fault */
	...
	if (some_timer->list.next == 0 || some_timer->list.prev == 0)
		printk("Corrupted timer! function=0x%p\n", some_timer->function);
	/* The next statement is where the oops occurs */
	<some statement which uses *some_timer>

Then if you can look the function pointer up in System.map or gdb we will have our culprit.

@@ -396,9 +396,22 @@
 
 __setup("nmi_watchdog=", setup_nmi_watchdog);
 
-extern spinlock_t console_lock;
+extern spinlock_t console_lock, timerlist_lock;
 static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
 
+/*
+ * Unlock any spinlocks which will prevent us from getting the
+ * message out
+ */
+
+void bust_spinlocks(void)
+{
+	spin_trylock(&console_lock);
+	spin_unlock(&console_lock);
+	spin_trylock(&timerlist_lock);
+	spin_unlock(&timerlist_lock);
+}
+
 inline void nmi_watchdog_tick(struct pt_regs * regs)
 {
 	/*

@@ -439,8 +452,7 @@
 			 * We are in trouble anyway, lets at least try
 			 * to get a message out.
 			 */
-			spin_trylock(&console_lock);
-			spin_unlock(&console_lock);
+			bust_spinlocks();
 			printk("NMI Watchdog detected LOCKUP on CPU%d, registers:\n", cpu);
 			show_registers(regs);
 			printk("console shuts up ...\n");

@@ -24,6 +24,7 @@
 #include <asm/hardirq.h>
 
 extern void die(const char *,struct pt_regs *,long);
+extern void bust_spinlocks(void);
 
 /*
  * Ugly, ugly, but the goto's result in better assembly..

@@ -250,6 +251,8 @@
  * Oops. The kernel tried to access some bad page. We'll have to
  * terminate things with extreme prejudice.
  */
+
+	bust_spinlocks();
 
 	if (address < PAGE_SIZE)
 		printk(KERN_ALERT "Unable to handle kernel NULL pointer dereference");

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Here's a slightly modified version of Andrew's patch: the version he sent me on Sunday contained an incomplete merge of my fix to locks_remove_posix(), meaning that the host server was not being notified that posix locks were cleared in the call to close().

The following patch contains his patch + the above correction + removal of the now-redundant BKLs in fl_notify & friends. It's been tested. Amongst other things, we now pass all the Connectathon tests.

Remaining issues on NFS locking: just the lock reclaiming fixes, which as I said earlier are non-critical for purposes of the 2.4.0 release.

fs/locks.c

- Remove semaphore file_lock_sem, and revert to BKL locking in order to fix various races and deadlocks.

- locks_delete_lock() should not call file->f_op->lock(). The latter notifies the *server*, hence calling it from routines like posix_lock_file() while merging/unmerging locks in our internal representation of the locks is a bug.

- A new function locks_unlock_delete() takes care of the special case when we're freeing all locks upon close().

- Straighten out lock notification. Don't bother waking up task when we have a notify function. Don't wake up task in posix_unblock_lock().

fs/lockd/clntlock.c
- Remove semaphore file_lock_sem. We're now protected by the BKL.

fs/lockd/clntproc, fs/lockd/svclock.c
- Remove redundant BKL. The caller already holds it.

Cheers,
Trond

diff -u --recursive --new-file linux-2.4.0-test10/fs/lockd/clntlock.c linux-2.4.0-lockd/fs/lockd/clntlock.c
@@ -168,7 +168,6 @@
 	 * reclaim is in progress */
 	lock_kernel();
 	lockd_up();
-	down(&file_lock_sem);
 
 	/* First, reclaim all locks that have been granted previously. */
 restart:

@@ -181,12 +180,11 @@
 				fl->fl_u.nfs_fl.state != host->h_state &&
 				(fl->fl_u.nfs_fl.flags & NFS_LCK_GRANTED)) {
 			fl->fl_u.nfs_fl.flags &= ~ NFS_LCK_GRANTED;
-			nlmclnt_reclaim(host, fl);
+			nlmclnt_reclaim(host, fl);	/* This sleeps */
 			goto restart;
 		}
 		tmp = tmp->next;
 	}
-	up(&file_lock_sem);
 
 	host->h_reclaiming = 0;
 	wake_up(&host->h_gracewait);

diff -u --recursive --new-file linux-2.4.0-test10/fs/lockd/clntproc.c linux-2.4.0-lockd/fs/lockd/clntproc.c
@@ -407,19 +407,15 @@
 static
 void nlmclnt_insert_lock_callback(struct file_lock *fl)
 {
-	lock_kernel();
 	nlm_get_host(fl->fl_u.nfs_fl.host);
-	unlock_kernel();
 }
 static
 void nlmclnt_remove_lock_callback(struct file_lock *fl)
 {
-	lock_kernel();
 	if (fl->fl_u.nfs_fl.host) {
 		nlm_release_host(fl->fl_u.nfs_fl.host);
 		fl->fl_u.nfs_fl.host = NULL;
 	}
-	unlock_kernel();
 }
 
 /*

diff -u --recursive --new-file linux-2.4.0-test10/fs/lockd/svclock.c linux-2.4.0-lockd/fs/lockd/svclock.c
@@ -468,16 +468,13 @@
 
 	dprintk("lockd: VFS unblock notification for block %p\n", fl);
 	posix_unblock_lock(fl);
-	lock_kernel();
 	for (bp = &nlm_blocked; (block = *bp); bp = &block->b_next) {
 		if (nlm_compare_locks(&block->b_call.a_args.lock.fl, fl)) {
 			nlmsvc_insert_block(block, 0);
 			svc_wake_up(block->b_daemon);
-			unlock_kernel();
 			return;
 		}
 	}
-	unlock_kernel();
 
 	printk(KERN_WARNING "lockd: notification for unknown block!\n");
 }

diff -u --recursive --new-file linux-2.4.0-test10/include/linux/fs.h linux-2.4.0-lockd/include/linux/fs.h
@@ -546,7 +546,6 @@
 #endif
 
 extern struct list_head file_lock_list;
-extern struct semaphore file_lock_sem;
 
 #include <linux/fcntl.h>
 

diff -u --recursive --new-file linux-2.4.0-test10/kernel/ksyms.c linux-2.4.0-lockd/kernel/ksyms.c
@@ -215,7 +215,6 @@
 EXPORT_SYMBOL(page_hash_bits);
 EXPORT_SYMBOL(page_hash_table);
 EXPORT_SYMBOL(file_lock_list);
-EXPORT_SYMBOL(file_lock_sem);
 EXPORT_SYMBOL(locks_init_lock);
 EXPORT_SYMBOL(locks_copy_lock);
 EXPORT_SYMBOL(posix_lock_file);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/