linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] remove BKL from drivers' release functions
@ 2001-11-28 23:05 David C. Hansen
  2001-11-28 23:29 ` Andrew Morton
  2001-11-28 23:36 ` Alan Cox
  0 siblings, 2 replies; 26+ messages in thread
From: David C. Hansen @ 2001-11-28 23:05 UTC (permalink / raw)
  To: linux-kernel

This version fixes a non-release of a spinlock before a return on error in rd.c.

The following is a patch which removes the BKL from quite a few 
drivers' release functions.  The release functions are already 
serialized in the VFS code by an atomic_t which guarantees that each 
function will be called only once, after all file descriptors have been 
closed.  In addition, in these drivers, the BKL was _only_ held in the 
release function and nowhere else in the driver where it might be needed.

Many of these patches simply remove the BKL from the file.  This causes 
no harm because the BKL was not really protecting anything, anyway.  
Other patches try to actually fix the locking.  Some do this by making 
use of atomic operations with the atomic_* functions, or the 
(test|set)_bit functions.  Most of these patches replace uses of normal 
integers which were used to keep open counts in the drivers.  In other 
some cases, a spinlock was added when the atomic operations could not 
guarantee proper serialization by themselves.  And, in very few cases, 
the existing locking was extended to protect more things.  These cases 
are very uncommon because locking is very uncommon in most of these drivers.

Special care has been taken not to introduce more locking issues into 
the drivers (do no harm).

The patch applies cleanly against 2.5.1-pre2. 

diff -ur linux-2.5.1-pre2-clean/arch/i386/kernel/apm.c linux/arch/i386/kernel/apm.c
--- linux-2.5.1-pre2-clean/arch/i386/kernel/apm.c	Fri Nov  9 13:58:02 2001
+++ linux/arch/i386/kernel/apm.c	Wed Nov 28 11:08:06 2001
@@ -387,6 +387,7 @@
 static DECLARE_WAIT_QUEUE_HEAD(apm_waitqueue);
 static DECLARE_WAIT_QUEUE_HEAD(apm_suspend_waitqueue);
 static struct apm_user *	user_list;
+static spinlock_t		user_list_lock;
 
 static char			driver_version[] = "1.15";	/* no spaces */
 
@@ -1053,6 +1054,7 @@
 {
 	struct apm_user *	as;
 
+	spin_lock( &user_list_lock );
 	if (user_list == NULL)
 		return;
 	for (as = user_list; as != NULL; as = as->next) {
@@ -1083,6 +1085,7 @@
 			break;
 		}
 	}
+	spin_unlock( &user_list_lock );
 	wake_up_interruptible(&apm_waitqueue);
 }
 
@@ -1179,10 +1182,12 @@
 	send_event(APM_NORMAL_RESUME);
 	sti();
 	queue_event(APM_NORMAL_RESUME, NULL);
+	spin_lock( &user_list_lock );
 	for (as = user_list; as != NULL; as = as->next) {
 		as->suspend_wait = 0;
 		as->suspend_result = ((err == APM_SUCCESS) ? 0 : -EIO);
 	}
+	spin_unlock( &user_list_lock );
 	ignore_normal_resume = 1;
 	wake_up_interruptible(&apm_suspend_waitqueue);
 	return err;
@@ -1519,7 +1524,6 @@
 	if (check_apm_user(as, "release"))
 		return 0;
 	filp->private_data = NULL;
-	lock_kernel();
 	if (as->standbys_pending > 0) {
 		standbys_pending -= as->standbys_pending;
 		if (standbys_pending <= 0)
@@ -1530,6 +1534,7 @@
 		if (suspends_pending <= 0)
 			(void) suspend();
 	}
+  	spin_lock( &user_list_lock );
 	if (user_list == as)
 		user_list = as->next;
 	else {
@@ -1544,7 +1549,7 @@
 		else
 			as1->next = as->next;
 	}
-	unlock_kernel();
+	spin_unlock( &user_list_lock );
 	kfree(as);
 	return 0;
 }
@@ -1573,8 +1578,10 @@
 	as->suser = capable(CAP_SYS_ADMIN);
 	as->writer = (filp->f_mode & FMODE_WRITE) == FMODE_WRITE;
 	as->reader = (filp->f_mode & FMODE_READ) == FMODE_READ;
+	spin_lock( &user_list_lock );
 	as->next = user_list;
 	user_list = as;
+	spin_unlock( &user_list_lock );
 	filp->private_data = as;
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/arch/i386/kernel/mtrr.c linux/arch/i386/kernel/mtrr.c
--- linux-2.5.1-pre2-clean/arch/i386/kernel/mtrr.c	Fri Nov  9 13:58:02 2001
+++ linux/arch/i386/kernel/mtrr.c	Wed Nov 28 11:08:06 2001
@@ -1788,7 +1788,6 @@
     unsigned int *fcount = file->private_data;
 
     if (fcount == NULL) return 0;
-    lock_kernel();
     max = get_num_var_ranges ();
     for (i = 0; i < max; ++i)
     {
@@ -1798,7 +1797,6 @@
 	    --fcount[i];
 	}
     }
-    unlock_kernel();
     kfree (fcount);
     file->private_data = NULL;
     return 0;
diff -ur linux-2.5.1-pre2-clean/arch/m68k/atari/joystick.c linux/arch/m68k/atari/joystick.c
--- linux-2.5.1-pre2-clean/arch/m68k/atari/joystick.c	Wed Jul 12 21:58:42 2000
+++ linux/arch/m68k/atari/joystick.c	Wed Nov 28 11:08:06 2001
@@ -61,13 +61,11 @@
 {
     int minor = DEVICE_NR(inode->i_rdev);
 
-    lock_kernel();
     joystick[minor].active = 0;
     joystick[minor].ready = 0;
 
     if ((joystick[0].active == 0) && (joystick[1].active == 0))
 	ikbd_joystick_disable();
-    unlock_kernel();
     return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/arch/m68k/mvme16x/rtc.c linux/arch/m68k/mvme16x/rtc.c
--- linux-2.5.1-pre2-clean/arch/m68k/mvme16x/rtc.c	Mon Jun 11 19:15:27 2001
+++ linux/arch/m68k/mvme16x/rtc.c	Wed Nov 28 11:08:06 2001
@@ -36,7 +36,7 @@
 static unsigned char days_in_mo[] =
 {0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
 
-static char rtc_status = 0;
+static atomic_t rtc_ready = ATOMIC_INIT(1);
 
 static int rtc_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 		     unsigned long arg)
@@ -130,18 +130,18 @@
 
 static int rtc_open(struct inode *inode, struct file *file)
 {
-	if(rtc_status)
+	if( !atomic_dec_and_test(&rtc_ready) )
+	{
+		atomic_inc( &rtc_ready );
 		return -EBUSY;
+	}
 
-	rtc_status = 1;
 	return 0;
 }
 
 static int rtc_release(struct inode *inode, struct file *file)
 {
-	lock_kernel();
-	rtc_status = 0;
-	unlock_kernel();
+	atomic_inc( &rtc_ready );
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/arch/mips64/sgi-ip27/ip27-rtc.c linux/arch/mips64/sgi-ip27/ip27-rtc.c
--- linux-2.5.1-pre2-clean/arch/mips64/sgi-ip27/ip27-rtc.c	Tue Nov 28 21:42:04 2000
+++ linux/arch/mips64/sgi-ip27/ip27-rtc.c	Wed Nov 28 11:08:06 2001
@@ -53,14 +53,7 @@
 
 static void get_rtc_time(struct rtc_time *rtc_tm);
 
-/*
- *	Bits in rtc_status. (6 bits of room for future expansion)
- */
-
-#define RTC_IS_OPEN		0x01	/* means /dev/rtc is in use	*/
-#define RTC_TIMER_ON		0x02	/* missed irq timer active	*/
-
-static unsigned char rtc_status;	/* bitmapped status byte.	*/
+static atomic_t rtc_ready = ATOMIC_INIT(1);
 static unsigned long rtc_freq;	/* Current periodic IRQ rate	*/
 static struct m48t35_rtc *rtc;
 
@@ -166,23 +159,17 @@
 
 static int rtc_open(struct inode *inode, struct file *file)
 {
-	if(rtc_status & RTC_IS_OPEN)
+	if( atomic_dec_and_test( &rtc_ready ) ) 
+	{
+		atomic_inc( &rtc_ready );
 		return -EBUSY;
-
-	rtc_status |= RTC_IS_OPEN;
+	}
 	return 0;
 }
 
 static int rtc_release(struct inode *inode, struct file *file)
 {
-	/*
-	 * Turn off all interrupts once the device is no longer
-	 * in use, and clear the data.
-	 */
-
-	lock_kernel();
-	rtc_status &= ~RTC_IS_OPEN;
-	unlock_kernel();
+	atomic_inc( &rtc_ready );
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/arch/sparc64/solaris/socksys.c linux/arch/sparc64/solaris/socksys.c
--- linux-2.5.1-pre2-clean/arch/sparc64/solaris/socksys.c	Sun Feb 18 19:49:54 2001
+++ linux/arch/sparc64/solaris/socksys.c	Wed Nov 28 11:08:06 2001
@@ -118,7 +118,6 @@
         struct T_primsg *it;
 
 	/* XXX: check this */
-	lock_kernel();
 	sock = (struct sol_socket_struct *)filp->private_data;
 	SOLDD(("sock release %016lx(%016lx)\n", sock, filp));
 	it = sock->pfirst;
@@ -132,7 +131,6 @@
 	filp->private_data = NULL;
 	SOLDD(("socksys_release %016lx\n", sock));
 	mykfree((char*)sock);
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/block/acsi_slm.c linux/drivers/block/acsi_slm.c
--- linux-2.5.1-pre2-clean/drivers/block/acsi_slm.c	Tue May 22 10:23:16 2001
+++ linux/drivers/block/acsi_slm.c	Wed Nov 28 11:08:07 2001
@@ -121,8 +121,8 @@
 static struct slm {
 	unsigned	target;			/* target number */
 	unsigned	lun;			/* LUN in target controller */
-	unsigned	wbusy : 1;		/* output part busy */
-	unsigned	rbusy : 1;		/* status part busy */
+	atomic_t	wr_ok; 			/* set to 0 if output part busy */
+	atomic_t	rd_ok;			/* set to 0 if status part busy */
 } slm_info[MAX_SLM];
 
 int N_SLM_Printers = 0;
@@ -778,15 +778,17 @@
 
 	if (file->f_mode & 2) {
 		/* open for writing is exclusive */
-		if (sip->wbusy)
+		if ( !atomic_dec_and_test(&sip->wr_ok) ) {
+			atomic_inc(&sip->wr_ok);	
 			return( -EBUSY );
-		sip->wbusy = 1;
+		}
 	}
 	if (file->f_mode & 1) {
-		/* open for writing is exclusive */
-		if (sip->rbusy)
-			return( -EBUSY );
-		sip->rbusy = 1;
+		/* open for reading is exclusive */
+                if ( !atomic_dec_and_test(&sip->rd_ok) ) {
+                        atomic_inc(&sip->rd_ok);
+                        return( -EBUSY );
+                }
 	}
 
 	return( 0 );
@@ -801,12 +803,10 @@
 	device = MINOR(inode->i_rdev);
 	sip = &slm_info[device];
 
-	lock_kernel();
 	if (file->f_mode & 2)
-		sip->wbusy = 0;
+		atomic_inc( &sip->wr_ok );
 	if (file->f_mode & 1)
-		sip->rbusy = 0;
-	unlock_kernel();
+		atomic_inc( &sip->rd_ok );
 	
 	return( 0 );
 }
@@ -983,8 +983,8 @@
 
 	slm_info[N_SLM_Printers].target = target;
 	slm_info[N_SLM_Printers].lun    = lun;
-	slm_info[N_SLM_Printers].wbusy  = 0;
-	slm_info[N_SLM_Printers].rbusy  = 0;
+	atomic_set(&slm_info[N_SLM_Printers].wr_ok, 1 ); 
+	atomic_set(&slm_info[N_SLM_Printers].rd_ok, 1 );
 	
 	printk( KERN_INFO "  Printer: %s\n", SLMBuffer );
 	printk( KERN_INFO "Detected slm%d at id %d lun %d\n",
diff -ur linux-2.5.1-pre2-clean/drivers/block/paride/pg.c linux/drivers/block/paride/pg.c
--- linux-2.5.1-pre2-clean/drivers/block/paride/pg.c	Thu Oct 11 09:04:57 2001
+++ linux/drivers/block/paride/pg.c	Wed Nov 28 11:08:07 2001
@@ -276,7 +276,7 @@
 	pg_drive_count = 0;
 	for (unit=0;unit<PG_UNITS;unit++) {
 		PG.pi = & PG.pia;
-		PG.access = 0;
+		set_bit( 0, &PG.access );
 		PG.busy = 0;
 		PG.present = 0;
 		PG.bufptr = NULL;
@@ -573,10 +573,7 @@
 
 	if ((unit >= PG_UNITS) || (!PG.present)) return -ENODEV;
 
-	PG.access++;
-
-	if (PG.access > 1) {
-		PG.access--;
+	if ( test_and_set_bit(0, &PG.access) ) {
 		return -EBUSY;
 	}
 
@@ -590,7 +587,7 @@
 
 	PG.bufptr = kmalloc(PG_MAX_DATA,GFP_KERNEL);
 	if (PG.bufptr == NULL) {
-		PG.access--;
+		clear_bit( 0, &PG.access ) ;
 		printk("%s: buffer allocation failed\n",PG.name);
 		return -ENOMEM;
 	}
@@ -602,15 +599,13 @@
 {
 	int	unit = DEVICE_NR(inode->i_rdev);
 
-	if ((unit >= PG_UNITS) || (PG.access <= 0)) 
+	if ( unit >= PG_UNITS || !test_bit(0,&PG.access) )
 		return -EINVAL;
 
-	lock_kernel();
-	PG.access--;
+	clear_bit( 0, &PG.access);
 
 	kfree(PG.bufptr);
 	PG.bufptr = NULL;
-	unlock_kernel();
 
 	return 0;
 
diff -ur linux-2.5.1-pre2-clean/drivers/block/paride/pt.c linux/drivers/block/paride/pt.c
--- linux-2.5.1-pre2-clean/drivers/block/paride/pt.c	Thu Oct 11 09:04:57 2001
+++ linux/drivers/block/paride/pt.c	Wed Nov 28 11:08:07 2001
@@ -244,7 +244,7 @@
 	int flags;        	  /* various state flags */
 	int last_sense;		  /* result of last request sense */
 	int drive;		  /* drive */
-	int access;               /* count of active opens ... */
+	atomic_t available;       /* 1 if access is available 0 otherwise */
 	int bs;			  /* block size */
 	int capacity;             /* Size of tape in KB */
 	int present;		  /* device present ? */
@@ -279,7 +279,7 @@
         pt_drive_count = 0;
         for (unit=0;unit<PT_UNITS;unit++) {
                 PT.pi = & PT.pia;
-                PT.access = 0;
+                atomic_set( &PT.available, 1 );
                 PT.flags = 0;
 		PT.last_sense = 0;
                 PT.present = 0;
@@ -696,22 +696,20 @@
 
         if ((unit >= PT_UNITS) || (!PT.present)) return -ENODEV;
 
-        PT.access++;
-
-	if (PT.access > 1) {
-		PT.access--;
+	if ( !atomic_dec_and_test(&PT.available) ) {
+		atomic_inc( &PT.available );
 		return -EBUSY;
 	}
 
 	pt_identify(unit);
 
 	if (!PT.flags & PT_MEDIA) {
-		PT.access--;
+		atomic_inc( &PT.available );
 		return -ENODEV;
 		}
 
 	if ((!PT.flags & PT_WRITE_OK) && (file ->f_mode & 2)) {
-		PT.access--;
+		atomic_inc( &PT.available );
 		return -EROFS;
 		}
 
@@ -720,7 +718,7 @@
 
 	PT.bufptr = kmalloc(PT_BUFSIZE,GFP_KERNEL);
 	if (PT.bufptr == NULL) {
-		PT.access--;
+		atomic_inc( &PT.available );
 		printk("%s: buffer allocation failed\n",PT.name);
 		return -ENOMEM;
 	}
@@ -775,20 +773,18 @@
 {
         int	unit = DEVICE_NR(inode->i_rdev);
 
-        if ((unit >= PT_UNITS) || (PT.access <= 0)) 
+        if ((unit >= PT_UNITS) || (atomic_read(&PT.available) > 1)) 
                 return -EINVAL;
 
-	lock_kernel();
 	if (PT.flags & PT_WRITING) pt_write_fm(unit);
 
 	if (PT.flags & PT_REWIND) pt_rewind(unit);	
 
-	PT.access--;
-
 	kfree(PT.bufptr);
 	PT.bufptr = NULL;
-	unlock_kernel();
 
+	atomic_inc( &PT.available );
+	
 	return 0;
 
 }
diff -ur linux-2.5.1-pre2-clean/drivers/block/rd.c linux/drivers/block/rd.c
--- linux-2.5.1-pre2-clean/drivers/block/rd.c	Wed Nov 28 11:07:19 2001
+++ linux/drivers/block/rd.c	Wed Nov 28 11:08:07 2001
@@ -89,6 +89,7 @@
 
 #ifdef CONFIG_BLK_DEV_INITRD
 static int initrd_users;
+static spinlock_t initrd_users_lock = SPIN_LOCK_UNLOCKED;
 #endif
 #endif
 
@@ -408,12 +409,15 @@
 {
 	extern void free_initrd_mem(unsigned long, unsigned long);
 
-	lock_kernel();
+	spin_lock( &initrd_users_lock );
 	if (!--initrd_users) {
+		spin_unlock( &initrd_users_lock );
 		free_initrd_mem(initrd_start, initrd_end);
 		initrd_start = 0;
+	} else {
+		spin_unlock( &initrd_users_lock );
 	}
-	unlock_kernel();
+		
 	blkdev_put(inode->i_bdev, BDEV_FILE);
 	return 0;
 }
@@ -433,8 +437,10 @@
 
 #ifdef CONFIG_BLK_DEV_INITRD
 	if (unit == INITRD_MINOR) {
-		if (!initrd_start) return -ENODEV;
+		spin_lock( &initrd_users_lock );
 		initrd_users++;
+		if (!initrd_start) return -ENODEV;
+		spin_unlock( &initrd_users_lock );
 		filp->f_op = &initrd_fops;
 		return 0;
 	}
diff -ur linux-2.5.1-pre2-clean/drivers/char/acquirewdt.c linux/drivers/char/acquirewdt.c
--- linux-2.5.1-pre2-clean/drivers/char/acquirewdt.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/acquirewdt.c	Wed Nov 28 11:08:07 2001
@@ -141,7 +141,6 @@
 
 static int acq_close(struct inode *inode, struct file *file)
 {
-	lock_kernel();
 	if(MINOR(inode->i_rdev)==WATCHDOG_MINOR)
 	{
 		spin_lock(&acq_lock);
@@ -151,7 +150,6 @@
 		acq_is_open=0;
 		spin_unlock(&acq_lock);
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/advantechwdt.c linux/drivers/char/advantechwdt.c
--- linux-2.5.1-pre2-clean/drivers/char/advantechwdt.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/advantechwdt.c	Wed Nov 28 11:08:07 2001
@@ -151,7 +151,6 @@
 static int
 advwdt_close(struct inode *inode, struct file *file)
 {
-	lock_kernel();
 	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR) {
 		spin_lock(&advwdt_lock);
 #ifndef CONFIG_WATCHDOG_NOWAYOUT	
@@ -160,7 +159,6 @@
 		advwdt_is_open = 0;
 		spin_unlock(&advwdt_lock);
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/agp/agpgart_fe.c linux/drivers/char/agp/agpgart_fe.c
--- linux-2.5.1-pre2-clean/drivers/char/agp/agpgart_fe.c	Sun Aug 12 10:38:48 2001
+++ linux/drivers/char/agp/agpgart_fe.c	Wed Nov 28 11:08:07 2001
@@ -606,17 +606,14 @@
 	agp_file_private *priv = (agp_file_private *) file->private_data;
 	agp_kern_info kerninfo;
 
-	lock_kernel();
 	AGP_LOCK();
 
 	if (agp_fe.backend_acquired != TRUE) {
 		AGP_UNLOCK();
-		unlock_kernel();
 		return -EPERM;
 	}
 	if (!(test_bit(AGP_FF_IS_VALID, &priv->access_flags))) {
 		AGP_UNLOCK();
-		unlock_kernel();
 		return -EPERM;
 	}
 	agp_copy_info(&kerninfo);
@@ -628,51 +625,42 @@
 	if (test_bit(AGP_FF_IS_CLIENT, &priv->access_flags)) {
 		if ((size + offset) > current_size) {
 			AGP_UNLOCK();
-			unlock_kernel();
 			return -EINVAL;
 		}
 		client = agp_find_client_by_pid(current->pid);
 
 		if (client == NULL) {
 			AGP_UNLOCK();
-			unlock_kernel();
 			return -EPERM;
 		}
 		if (!agp_find_seg_in_client(client, offset,
 					    size, vma->vm_page_prot)) {
 			AGP_UNLOCK();
-			unlock_kernel();
 			return -EINVAL;
 		}
 		if (remap_page_range(vma->vm_start,
 				     (kerninfo.aper_base + offset),
 				     size, vma->vm_page_prot)) {
 			AGP_UNLOCK();
-			unlock_kernel();
 			return -EAGAIN;
 		}
 		AGP_UNLOCK();
-		unlock_kernel();
 		return 0;
 	}
 	if (test_bit(AGP_FF_IS_CONTROLLER, &priv->access_flags)) {
 		if (size != current_size) {
 			AGP_UNLOCK();
-			unlock_kernel();
 			return -EINVAL;
 		}
 		if (remap_page_range(vma->vm_start, kerninfo.aper_base,
 				     size, vma->vm_page_prot)) {
 			AGP_UNLOCK();
-			unlock_kernel();
 			return -EAGAIN;
 		}
 		AGP_UNLOCK();
-		unlock_kernel();
 		return 0;
 	}
 	AGP_UNLOCK();
-	unlock_kernel();
 	return -EPERM;
 }
 
@@ -680,7 +668,6 @@
 {
 	agp_file_private *priv = (agp_file_private *) file->private_data;
 
-	lock_kernel();
 	AGP_LOCK();
 
 	if (test_bit(AGP_FF_IS_CONTROLLER, &priv->access_flags)) {
@@ -702,7 +689,6 @@
 	agp_remove_file_private(priv);
 	kfree(priv);
 	AGP_UNLOCK();
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/busmouse.c linux/drivers/char/busmouse.c
--- linux-2.5.1-pre2-clean/drivers/char/busmouse.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/busmouse.c	Wed Nov 28 11:08:07 2001
@@ -171,6 +171,7 @@
 	lock_kernel();
 	busmouse_fasync(-1, file, 0);
 
+	down(&mouse_sem); /* to protect mse->active */
 	if (--mse->active == 0) {
 		if (mse->ops->release)
 			ret = mse->ops->release(inode, file);
@@ -179,7 +180,8 @@
 		mse->ready = 0;
 	}
 	unlock_kernel();
-
+	up( &mouse_sem);
+	
 	return ret;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/dtlk.c linux/drivers/char/dtlk.c
--- linux-2.5.1-pre2-clean/drivers/char/dtlk.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/dtlk.c	Wed Nov 28 11:08:07 2001
@@ -328,10 +328,8 @@
 		break;
 	}
 	TRACE_RET;
-
-	lock_kernel();
+	
 	del_timer(&dtlk_timer);
-	unlock_kernel();
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/char/ftape/zftape/zftape-init.c linux/drivers/char/ftape/zftape/zftape-init.c
--- linux-2.5.1-pre2-clean/drivers/char/ftape/zftape/zftape-init.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/ftape/zftape/zftape-init.c	Wed Nov 28 11:08:07 2001
@@ -68,7 +68,8 @@
 
 /*      Local vars.
  */
-static int busy_flag = 0;
+static int busy_flag;
+
 static sigset_t orig_sigmask;
 
 /*  the interface to the kernel vfs layer
@@ -113,14 +114,13 @@
 	TRACE_FUN(ft_t_flow);
 
 	TRACE(ft_t_flow, "called for minor %d", MINOR(ino->i_rdev));
-	if (busy_flag) {
+	if ( test_and_set_bit(0,&busy_flag) )) {
 		TRACE_ABORT(-EBUSY, ft_t_warn, "failed: already busy");
 	}
-	busy_flag = 1;
 	if ((MINOR(ino->i_rdev) & ~(ZFT_MINOR_OP_MASK | FTAPE_NO_REWIND))
 	     > 
 	    FTAPE_SEL_D) {
-		busy_flag = 0;
+		clear_bit(0,&busy_flag);
 		TRACE_ABORT(-ENXIO, ft_t_err, "failed: illegal unit nr");
 	}
 	orig_sigmask = current->blocked;
@@ -128,7 +128,7 @@
 	result = _zft_open(MINOR(ino->i_rdev), filep->f_flags & O_ACCMODE);
 	if (result < 0) {
 		current->blocked = orig_sigmask; /* restore mask */
-		busy_flag = 0;
+		clear_bit(0,&busy_flag);
 		TRACE_ABORT(result, ft_t_err, "_ftape_open failed");
 	} else {
 		/* Mask signals that will disturb proper operation of the
@@ -147,10 +147,8 @@
 	int result;
 	TRACE_FUN(ft_t_flow);
 
-	lock_kernel();
-	if (!busy_flag || MINOR(ino->i_rdev) != zft_unit) {
+	if ( !test_bit(0,&busy_flag) || MINOR(ino->i_rdev) != zft_unit) {
 		TRACE(ft_t_err, "failed: not busy or wrong unit");
-		unlock_kernel();
 		TRACE_EXIT 0;
 	}
 	sigfillset(&current->blocked);
@@ -159,8 +157,7 @@
 		TRACE(ft_t_err, "_zft_close failed");
 	}
 	current->blocked = orig_sigmask; /* restore before open state */
-	busy_flag = 0;
-	unlock_kernel();
+	clear_bit(0,&busy_flag);
 	TRACE_EXIT 0;
 }
 
@@ -173,7 +170,7 @@
 	sigset_t old_sigmask;
 	TRACE_FUN(ft_t_flow);
 
-	if (!busy_flag || MINOR(ino->i_rdev) != zft_unit || ft_failure) {
+	if ( !test_bit(0,&busy_flag) || MINOR(ino->i_rdev) != zft_unit || ft_failure) {
 		TRACE_ABORT(-EIO, ft_t_err,
 			    "failed: not busy, failure or wrong unit");
 	}
@@ -193,7 +190,7 @@
 	sigset_t old_sigmask;
 	TRACE_FUN(ft_t_flow);
 
-	if (!busy_flag || 
+	if ( !test_bit(0,&busy_flag) || 
 	    MINOR(filep->f_dentry->d_inode->i_rdev) != zft_unit || 
 	    ft_failure)
 	{
@@ -202,14 +199,12 @@
 	}
 	old_sigmask = current->blocked; /* save mask */
 	sigfillset(&current->blocked);
-	lock_kernel();
 	if ((result = ftape_mmap(vma)) >= 0) {
 #ifndef MSYNC_BUG_WAS_FIXED
 		static struct vm_operations_struct dummy = { NULL, };
 		vma->vm_ops = &dummy;
 #endif
 	}
-	unlock_kernel();
 	current->blocked = old_sigmask; /* restore mask */
 	TRACE_EXIT result;
 }
@@ -225,7 +220,7 @@
 	TRACE_FUN(ft_t_flow);
 
 	TRACE(ft_t_data_flow, "called with count: %ld", (unsigned long)req_len);
-	if (!busy_flag || MINOR(ino->i_rdev) != zft_unit || ft_failure) {
+	if (!test_bit(0,&busy_flag)  || MINOR(ino->i_rdev) != zft_unit || ft_failure) {
 		TRACE_ABORT(-EIO, ft_t_err,
 			    "failed: not busy, failure or wrong unit");
 	}
@@ -248,7 +243,7 @@
 	TRACE_FUN(ft_t_flow);
 
 	TRACE(ft_t_flow, "called with count: %ld", (unsigned long)req_len);
-	if (!busy_flag || MINOR(ino->i_rdev) != zft_unit || ft_failure) {
+	if (!test_bit(0,&busy_flag) || MINOR(ino->i_rdev) != zft_unit || ft_failure) {
 		TRACE_ABORT(-EIO, ft_t_err,
 			    "failed: not busy, failure or wrong unit");
 	}
@@ -403,7 +398,7 @@
  */
 static int can_unload(void)
 {
-	return (GET_USE_COUNT(THIS_MODULE)||zft_dirty()||busy_flag)?-EBUSY:0;
+	return (GET_USE_COUNT(THIS_MODULE)||zft_dirty()||test_bit(0,&busy_flag))?-EBUSY:0;
 }
 /* Called by modules package when installing the driver
  */
diff -ur linux-2.5.1-pre2-clean/drivers/char/lp.c linux/drivers/char/lp.c
--- linux-2.5.1-pre2-clean/drivers/char/lp.c	Thu Oct 25 00:07:39 2001
+++ linux/drivers/char/lp.c	Wed Nov 28 11:08:07 2001
@@ -494,11 +494,9 @@
 	parport_negotiate (lp_table[minor].dev->port, IEEE1284_MODE_COMPAT);
 	lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
 	lp_release_parport (&lp_table[minor]);
-	lock_kernel();
 	kfree(lp_table[minor].lp_buffer);
 	lp_table[minor].lp_buffer = NULL;
 	LP_F(minor) &= ~LP_BUSY;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/mixcomwd.c linux/drivers/char/mixcomwd.c
--- linux-2.5.1-pre2-clean/drivers/char/mixcomwd.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/mixcomwd.c	Wed Nov 28 11:08:07 2001
@@ -101,11 +101,9 @@
 static int mixcomwd_release(struct inode *inode, struct file *file)
 {
 
-	lock_kernel();
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
 	if(mixcomwd_timer_alive) {
 		printk(KERN_ERR "mixcomwd: release called while internal timer alive");
-		unlock_kernel();
 		return -EBUSY;
 	}
 	init_timer(&mixcomwd_timer);
@@ -117,7 +115,6 @@
 #endif
 
 	clear_bit(0,&mixcomwd_opened);
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/nvram.c linux/drivers/char/nvram.c
--- linux-2.5.1-pre2-clean/drivers/char/nvram.c	Fri Sep 14 14:40:00 2001
+++ linux/drivers/char/nvram.c	Wed Nov 28 11:08:07 2001
@@ -108,7 +108,10 @@
 #include <asm/system.h>
 
 static int nvram_open_cnt;	/* #times opened */
-static int nvram_open_mode;		/* special open modes */
+static int nvram_open_mode;	/* special open modes */
+static spinlock_t nvram_open_lock = SPIN_LOCK_UNLOCKED;
+                               /* guards nvram_open_cnt and
+                                         nvram_open_mode */
 #define	NVRAM_WRITE		1		/* opened for writing (exclusive) */
 #define	NVRAM_EXCL		2		/* opened with O_EXCL */
 
@@ -326,29 +329,33 @@
 
 static int nvram_open( struct inode *inode, struct file *file )
 {
+	spin_lock( &nvram_open_lock );
 	if ((nvram_open_cnt && (file->f_flags & O_EXCL)) ||
 		(nvram_open_mode & NVRAM_EXCL) ||
 		((file->f_mode & 2) && (nvram_open_mode & NVRAM_WRITE)))
+	{	
+		spin_unlock( &nvram_open_lock );
 		return( -EBUSY );
+	}
 
 	if (file->f_flags & O_EXCL)
 		nvram_open_mode |= NVRAM_EXCL;
 	if (file->f_mode & 2)
 		nvram_open_mode |= NVRAM_WRITE;
 	nvram_open_cnt++;
+	spin_unlock( &nvram_open_lock );
 	return( 0 );
 }
 
 static int nvram_release( struct inode *inode, struct file *file )
 {
-	lock_kernel();
+	spin_lock( &nvram_open_lock );
 	nvram_open_cnt--;
 	if (file->f_flags & O_EXCL)
 		nvram_open_mode &= ~NVRAM_EXCL;
 	if (file->f_mode & 2)
 		nvram_open_mode &= ~NVRAM_WRITE;
-	unlock_kernel();
-
+	spin_unlock( &nvram_open_lock );
 	return( 0 );
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/pc110pad.c linux/drivers/char/pc110pad.c
--- linux-2.5.1-pre2-clean/drivers/char/pc110pad.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/pc110pad.c	Wed Nov 28 11:08:07 2001
@@ -68,7 +68,9 @@
 /* driver/filesystem interface management */
 static wait_queue_head_t queue;
 static struct fasync_struct *asyncptr;
-static int active;	/* number of concurrent open()s */
+static active_count = 0; 	/* number of concurrent open()s */
+static spinlock_t active_lock = SPIN_LOCK_UNLOCKED;
+/* this lock should be held when referencing active_count */
 static struct semaphore reader_lock;
 
 /**
@@ -584,11 +586,11 @@
  
 static int close_pad(struct inode * inode, struct file * file)
 {
-	lock_kernel();
 	fasync_pad(-1, file, 0);
-	if (!--active)
+	spin_lock( &active_lock );
+	if (!--active_count)
 		outb(0x30, current_params.io+2);  /* switch off digitiser */
-	unlock_kernel();
+	spin_unlock( &active_lock );	
 	return 0;
 }
 
@@ -608,8 +610,13 @@
 {
 	unsigned long flags;
 	
-	if (active++)
+	spin_lock( &active_lock );
+	if (active_count++)
+        {
+		spin_unlock( &active_lock );
 		return 0;
+	}
+	spin_unlock( &active_lock );
 
 	save_flags(flags);
 	cli();
diff -ur linux-2.5.1-pre2-clean/drivers/char/pc_keyb.c linux/drivers/char/pc_keyb.c
--- linux-2.5.1-pre2-clean/drivers/char/pc_keyb.c	Fri Nov  9 14:01:21 2001
+++ linux/drivers/char/pc_keyb.c	Wed Nov 28 11:08:07 2001
@@ -90,6 +90,7 @@
  
 static struct aux_queue *queue;	/* Mouse data buffer. */
 static int aux_count;
+static spinlock_t aux_count_lock = SPIN_LOCK_UNLOCKED;
 /* used when we send commands to the mouse that expect an ACK. */
 static unsigned char mouse_reply_expected;
 
@@ -405,8 +406,9 @@
 
        if (rqst == PM_RESUME) {
                if (queue) {                    /* Aux port detected */
-                       if (aux_count == 0) {   /* Mouse not in use */ 
-                               spin_lock_irqsave(&kbd_controller_lock, flags);
+		       spin_lock_irqsave( &aux_count_lock, flags);
+              	       if ( aux_count == 0) {   /* Mouse not in use */ 
+                               spin_lock(&kbd_controller_lock);
 			       /*
 				* Dell Lat. C600 A06 enables mouse after resume.
 				* When user touches the pad, it posts IRQ 12
@@ -418,8 +420,9 @@
 			       kbd_write_command(KBD_CCMD_WRITE_MODE);
 			       kb_wait();
 			       kbd_write_output(AUX_INTS_OFF);
-			       spin_unlock_irqrestore(&kbd_controller_lock, flags);
+			       spin_unlock(&kbd_controller_lock, flags);
 		       }
+		       spin_unlock_irqrestore( &aux_count_lock,flags );
 	       }
        }
 #endif
@@ -430,6 +433,7 @@
 static inline void handle_mouse_event(unsigned char scancode)
 {
 #ifdef CONFIG_PSMOUSE
+	int flags;
 	static unsigned char prev_code;
 	if (mouse_reply_expected) {
 		if (scancode == AUX_ACK) {
@@ -448,7 +452,8 @@
 
 	prev_code = scancode;
 	add_mouse_randomness(scancode);
-	if (aux_count) {
+	spin_lock_irqsave( &aux_count_lock, flags);
+	if ( aux_count ) {
 		int head = queue->head;
 
 		queue->buf[head] = scancode;
@@ -459,6 +464,7 @@
 			wake_up_interruptible(&queue->proc_list);
 		}
 	}
+	spin_unlock_irqrestore( &aux_count_lock, flags);
 #endif
 }
 
@@ -1046,16 +1052,17 @@
 
 static int release_aux(struct inode * inode, struct file * file)
 {
-	lock_kernel();
+	int flags;
 	fasync_aux(-1, file, 0);
-	if (--aux_count) {
-		unlock_kernel();
+	spin_lock_irqsave( &aux_count, flags );
+	if ( --aux_count ) {
+		spin_unlock_irqrestore( &aux_count_lock );
 		return 0;
 	}
+	spin_unlock_irqrestore( &aux_count_lock, flags );
 	kbd_write_cmd(AUX_INTS_OFF);			    /* Disable controller ints */
 	kbd_write_command_w(KBD_CCMD_MOUSE_DISABLE);
 	aux_free_irq(AUX_DEV);
-	unlock_kernel();
 	return 0;
 }
 
@@ -1066,12 +1073,16 @@
 
 static int open_aux(struct inode * inode, struct file * file)
 {
-	if (aux_count++) {
+	int flags;
+	spin_lock_irqsave( &aux_count_lock, flags );
+	if ( aux_count++ ) {
+		spin_unlock_irqrestore( &aux_count_lock );
 		return 0;
 	}
 	queue->head = queue->tail = 0;		/* Flush input queue */
 	if (aux_request_irq(keyboard_interrupt, AUX_DEV)) {
 		aux_count--;
+		spin_unlock_irqrestore( &aux_count_lock, flags );
 		return -EBUSY;
 	}
 	kbd_write_command_w(KBD_CCMD_MOUSE_ENABLE);	/* Enable the
@@ -1083,7 +1094,7 @@
 	mdelay(2);			/* Ensure we follow the kbc access delay rules.. */
 
 	send_data(KBD_CMD_ENABLE);	/* try to workaround toshiba4030cdt problem */
-
+	spin_unlock_irqrestore( &aux_count_lock, flags );
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/pcwd.c linux/drivers/char/pcwd.c
--- linux-2.5.1-pre2-clean/drivers/char/pcwd.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/pcwd.c	Wed Nov 28 11:08:07 2001
@@ -100,7 +100,8 @@
 #define WD_SRLY2                0x80	/* Software external relay triggered */
 
 static int current_readport, revision, temp_panic;
-static int is_open, initial_status, supports_temp, mode_debug;
+static atomic_t open_allowed = ATOMIC_INIT(1);
+static int initial_status, supports_temp, mode_debug;
 static spinlock_t io_lock;
 
 /*
@@ -402,9 +403,12 @@
         switch (MINOR(ino->i_rdev))
         {
                 case WATCHDOG_MINOR:
-                    if (is_open)
+                    if ( !atomic_dec_and_test(&open_allowed) )
+		    {
+			atomic_inc( &open_allowed );
                         return -EBUSY;
-                    MOD_INC_USE_COUNT;
+                    }
+		    MOD_INC_USE_COUNT;
                     /*  Enable the port  */
                     if (revision == PCWD_REVISION_C)
                     {
@@ -412,7 +416,6 @@
                     	outb_p(0x00, current_readport + 3);
                     	spin_unlock(&io_lock);
                     }
-                    is_open = 1;
                     return(0);
                 case TEMP_MINOR:
                     return(0);
@@ -452,8 +455,6 @@
 {
 	if (MINOR(ino->i_rdev)==WATCHDOG_MINOR)
 	{
-		lock_kernel();
-	        is_open = 0;
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
 		/*  Disable the board  */
 		if (revision == PCWD_REVISION_C) {
@@ -462,8 +463,8 @@
 			outb_p(0xA5, current_readport + 3);
 			spin_unlock(&io_lock);
 		}
+	        atomic_inc( &open_allowed );
 #endif
-		unlock_kernel();
 	}
 	return 0;
 }
@@ -574,7 +575,7 @@
 	printk("pcwd: v%s Ken Hollis (kenji@bitgate.com)\n", WD_VER);
 
 	/* Initial variables */
-	is_open = 0;
+	set_bit( 0, &open_allowed );
 	supports_temp = 0;
 	mode_debug = 0;
 	temp_panic = 0;
diff -ur linux-2.5.1-pre2-clean/drivers/char/ppdev.c linux/drivers/char/ppdev.c
--- linux-2.5.1-pre2-clean/drivers/char/ppdev.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/ppdev.c	Wed Nov 28 11:08:07 2001
@@ -646,7 +646,6 @@
 	struct pp_struct *pp = file->private_data;
 	int compat_negot;
 
-	lock_kernel();
 	compat_negot = 0;
 	if (!(pp->flags & PP_CLAIMED) && pp->pdev &&
 	    (pp->state.mode != IEEE1284_MODE_COMPAT)) {
@@ -695,7 +694,6 @@
 		printk (KERN_DEBUG CHRDEV "%x: unregistered pardevice\n",
 			minor);
 	}
-	unlock_kernel();
 
 	kfree (pp);
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/qpmouse.c linux/drivers/char/qpmouse.c
--- linux-2.5.1-pre2-clean/drivers/char/qpmouse.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/qpmouse.c	Wed Nov 28 11:08:07 2001
@@ -111,6 +111,7 @@
 
 static int qp_present;
 static int qp_count;
+static spinlock_t qp_count_lock = SPIN_LOCK_UNLOCKED;
 static int qp_data = QP_DATA;
 static int qp_status = QP_STATUS;
 
@@ -141,8 +142,8 @@
 {
 	unsigned char status;
 
-	lock_kernel();
 	fasync_qp(-1, file, 0);
+ 	spin_lock( &qp_count_lock );	
 	if (!--qp_count) {
 		if (!poll_qp_status())
 			printk(KERN_WARNING "Warning: Mouse device busy in release_qp()\n");
@@ -152,7 +153,7 @@
 			printk(KERN_WARNING "Warning: Mouse device busy in release_qp()\n");
 		free_irq(QP_IRQ, NULL);
 	}
-	unlock_kernel();
+	spin_unlock( &qp_count_lock );
 	return 0;
 }
 
@@ -168,8 +169,13 @@
 	if (!qp_present)
 		return -EINVAL;
 
+	spin_lock( &qp_count_lock );
 	if (qp_count++)
+	{
+		spin_unlock( &qp_count_lock );
 		return 0;
+	}
+	spin_unlock( &qp_count_lock );
 
 	if (request_irq(QP_IRQ, qp_interrupt, 0, "PS/2 Mouse", NULL)) {
 		qp_count--;
diff -ur linux-2.5.1-pre2-clean/drivers/char/qtronix.c linux/drivers/char/qtronix.c
--- linux-2.5.1-pre2-clean/drivers/char/qtronix.c	Sun Sep  9 10:43:02 2001
+++ linux/drivers/char/qtronix.c	Wed Nov 28 11:08:07 2001
@@ -492,10 +492,8 @@
 
 static int release_aux(struct inode * inode, struct file * file)
 {
-	lock_kernel();
 	fasync_aux(-1, file, 0);
 	aux_count--;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/sbc60xxwdt.c linux/drivers/char/sbc60xxwdt.c
--- linux-2.5.1-pre2-clean/drivers/char/sbc60xxwdt.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/sbc60xxwdt.c	Wed Nov 28 11:08:07 2001
@@ -214,7 +214,6 @@
 
 static int fop_close(struct inode * inode, struct file * file)
 {
-	lock_kernel();
 	if(MINOR(inode->i_rdev) == WATCHDOG_MINOR) 
 	{
 		if(wdt_expect_close)
@@ -225,7 +224,6 @@
 		}
 	}
 	wdt_is_open = 0;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/softdog.c linux/drivers/char/softdog.c
--- linux-2.5.1-pre2-clean/drivers/char/softdog.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/char/softdog.c	Wed Nov 28 11:08:08 2001
@@ -100,12 +100,10 @@
 	 *	Shut off the timer.
 	 * 	Lock it in if it's a module and we defined ...NOWAYOUT
 	 */
-	 lock_kernel();
 #ifndef CONFIG_WATCHDOG_NOWAYOUT	 
 	del_timer(&watchdog_ticktock);
 #endif	
 	timer_alive=0;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/tpqic02.c linux/drivers/char/tpqic02.c
--- linux-2.5.1-pre2-clean/drivers/char/tpqic02.c	Fri Sep 14 14:40:00 2001
+++ linux/drivers/char/tpqic02.c	Wed Nov 28 11:08:08 2001
@@ -2395,7 +2395,6 @@
 {
 	kdev_t dev = inode->i_rdev;
 
-	lock_kernel();
 	if (TP_DIAGS(dev)) {
 		printk("qic02_tape_release: dev=%s\n", kdevname(dev));
 	}
@@ -2419,7 +2418,6 @@
 			(void) do_qic_cmd(QCMD_REWIND, TIM_R);
 		}
 	}
-	unlock_kernel();
 	return 0;
 }				/* qic02_tape_release */
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/w83877f_wdt.c linux/drivers/char/w83877f_wdt.c
--- linux-2.5.1-pre2-clean/drivers/char/w83877f_wdt.c	Thu Sep 13 15:21:32 2001
+++ linux/drivers/char/w83877f_wdt.c	Wed Nov 28 11:08:08 2001
@@ -214,7 +214,6 @@
 
 static int fop_close(struct inode * inode, struct file * file)
 {
-	lock_kernel();
 	if(MINOR(inode->i_rdev) == WATCHDOG_MINOR) 
 	{
 		if(wdt_expect_close)
@@ -225,7 +224,6 @@
 		}
 	}
 	wdt_is_open = 0;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/wdt.c linux/drivers/char/wdt.c
--- linux-2.5.1-pre2-clean/drivers/char/wdt.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/wdt.c	Wed Nov 28 11:08:08 2001
@@ -374,7 +374,6 @@
  
 static int wdt_release(struct inode *inode, struct file *file)
 {
-	lock_kernel();
 	if(MINOR(inode->i_rdev)==WATCHDOG_MINOR)
 	{
 #ifndef CONFIG_WATCHDOG_NOWAYOUT	
@@ -383,7 +382,6 @@
 #endif		
 		wdt_is_open=0;
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/char/wdt285.c linux/drivers/char/wdt285.c
--- linux-2.5.1-pre2-clean/drivers/char/wdt285.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/wdt285.c	Wed Nov 28 11:08:08 2001
@@ -94,10 +94,8 @@
 static int watchdog_release(struct inode *inode, struct file *file)
 {
 #ifdef ONLY_TESTING
-	lock_kernel();
 	free_irq(IRQ_TIMER4, NULL);
 	timer_alive = 0;
-	unlock_kernel();
 #else
 	/*
 	 *	It's irreversible!
diff -ur linux-2.5.1-pre2-clean/drivers/char/wdt977.c linux/drivers/char/wdt977.c
--- linux-2.5.1-pre2-clean/drivers/char/wdt977.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/wdt977.c	Wed Nov 28 11:08:08 2001
@@ -92,7 +92,6 @@
 	 * 	Lock it in if it's a module and we defined ...NOWAYOUT
 	 */
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
-	lock_kernel();
 
 	// unlock the SuperIO chip
 	outb(0x87,0x370); 
@@ -124,7 +123,6 @@
 	outb(0xAA,0x370);
 
 	timer_alive=0;
-	unlock_kernel();
 
 	printk(KERN_INFO "Watchdog: shutdown.\n");
 #endif
diff -ur linux-2.5.1-pre2-clean/drivers/char/wdt_pci.c linux/drivers/char/wdt_pci.c
--- linux-2.5.1-pre2-clean/drivers/char/wdt_pci.c	Fri Sep  7 09:28:38 2001
+++ linux/drivers/char/wdt_pci.c	Wed Nov 28 11:08:08 2001
@@ -353,16 +353,16 @@
 	switch(MINOR(inode->i_rdev))
 	{
 		case WATCHDOG_MINOR:
-			if(wdt_is_open)
+			if( test_and_set_bit(0,&wdt_is_open) ) 
+			{
 				return -EBUSY;
+			}
 #ifdef CONFIG_WATCHDOG_NOWAYOUT	
 			MOD_INC_USE_COUNT;
 #endif
 			/*
 			 *	Activate 
 			 */
-	 
-			wdt_is_open=1;
 
 			inb_p(WDT_DC);		/* Disable */
 
@@ -412,13 +412,11 @@
 {
 	if(MINOR(inode->i_rdev)==WATCHDOG_MINOR)
 	{
-		lock_kernel();
 #ifndef CONFIG_WATCHDOG_NOWAYOUT	
 		inb_p(WDT_DC);		/* Disable counters */
 		wdtpci_ctr_load(2,0);	/* 0 length reset pulses now */
 #endif		
-		wdt_is_open=0;
-		unlock_kernel();
+		clear_bit(0, &wdt_is_open );
 	}
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/i2c/i2c-dev.c linux/drivers/i2c/i2c-dev.c
--- linux-2.5.1-pre2-clean/drivers/i2c/i2c-dev.c	Thu Oct 11 08:05:47 2001
+++ linux/drivers/i2c/i2c-dev.c	Wed Nov 28 11:08:08 2001
@@ -423,14 +423,9 @@
 #endif
 #if LINUX_KERNEL_VERSION < KERNEL_VERSION(2,4,0)
 	MOD_DEC_USE_COUNT;
-#else /* LINUX_KERNEL_VERSION >= KERNEL_VERSION(2,4,0) */
-	lock_kernel();
 #endif /* LINUX_KERNEL_VERSION < KERNEL_VERSION(2,4,0) */
 	if (i2cdev_adaps[minor]->dec_use)
 		i2cdev_adaps[minor]->dec_use(i2cdev_adaps[minor]);
-#if LINUX_KERNEL_VERSION >= KERNEL_VERSION(2,4,0)
-	unlock_kernel();
-#endif /* LINUX_KERNEL_VERSION >= KERNEL_VERSION(2,4,0) */
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/ide/ide-tape.c linux/drivers/ide/ide-tape.c
--- linux-2.5.1-pre2-clean/drivers/ide/ide-tape.c	Wed Nov 28 11:07:20 2001
+++ linux/drivers/ide/ide-tape.c	Wed Nov 28 11:08:08 2001
@@ -5486,7 +5486,6 @@
 	idetape_pc_t pc;
 	unsigned int minor=MINOR (inode->i_rdev);
 
-	lock_kernel();
 	tape = drive->driver_data;
 #if IDETAPE_DEBUG_LOG
 	if (tape->debug_level >= 3)
@@ -5517,7 +5516,6 @@
 		MOD_DEC_USE_COUNT;
 	}
 	clear_bit (IDETAPE_BUSY, &tape->flags);
-	unlock_kernel();
 	return 0;
 }
 
Only in linux/drivers/ide: ide-tape.c.orig
diff -ur linux-2.5.1-pre2-clean/drivers/ieee1394/raw1394.c linux/drivers/ieee1394/raw1394.c
--- linux-2.5.1-pre2-clean/drivers/ieee1394/raw1394.c	Mon Oct  1 21:24:24 2001
+++ linux/drivers/ieee1394/raw1394.c	Wed Nov 28 11:08:08 2001
@@ -949,7 +949,6 @@
         struct pending_request *req;
         int done = 0, i;
 
-        lock_kernel();
         for (i = 0; i < 64; i++) {
                 if (fi->listen_channels & (1ULL << i)) {
                         hpsb_unlisten_channel(hl_handle, fi->host, i);
@@ -990,7 +989,6 @@
         kfree(fi);
 
         V22_COMPAT_MOD_DEC_USE_COUNT;
-        unlock_kernel();
         return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/input/evdev.c linux/drivers/input/evdev.c
--- linux-2.5.1-pre2-clean/drivers/input/evdev.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/input/evdev.c	Wed Nov 28 11:08:08 2001
@@ -94,7 +94,6 @@
 	struct evdev_list *list = file->private_data;
 	struct evdev_list **listptr;
 
-	lock_kernel();
 	listptr = &list->evdev->list;
 	evdev_fasync(-1, file, 0);
 
@@ -113,7 +112,6 @@
 	}
 
 	kfree(list);
-	unlock_kernel();
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/input/input.c linux/drivers/input/input.c
--- linux-2.5.1-pre2-clean/drivers/input/input.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/input/input.c	Wed Nov 28 11:08:08 2001
@@ -387,9 +387,7 @@
 	old_fops = file->f_op;
 	file->f_op = new_fops;
 
-	lock_kernel();
 	err = new_fops->open(inode, file);
-	unlock_kernel();
 
 	if (err) {
 		fops_put(file->f_op);
diff -ur linux-2.5.1-pre2-clean/drivers/input/joydev.c linux/drivers/input/joydev.c
--- linux-2.5.1-pre2-clean/drivers/input/joydev.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/input/joydev.c	Wed Nov 28 11:08:08 2001
@@ -164,8 +164,7 @@
 {
 	struct joydev_list *list = file->private_data;
 	struct joydev_list **listptr;
-
-	lock_kernel();
+	
 	listptr = &list->joydev->list;
 	joydev_fasync(-1, file, 0);
 
@@ -184,7 +183,6 @@
 	}
 
 	kfree(list);
-	unlock_kernel();
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/input/mousedev.c linux/drivers/input/mousedev.c
--- linux-2.5.1-pre2-clean/drivers/input/mousedev.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/input/mousedev.c	Wed Nov 28 11:08:08 2001
@@ -169,8 +169,7 @@
 {
 	struct mousedev_list *list = file->private_data;
 	struct mousedev_list **listptr;
-
-	lock_kernel();
+	
 	listptr = &list->mousedev->list;
 	mousedev_fasync(-1, file, 0);
 
@@ -208,7 +207,6 @@
 	}
 	
 	kfree(list);
-	unlock_kernel();
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/isdn/avmb1/capi.c linux/drivers/isdn/avmb1/capi.c
--- linux-2.5.1-pre2-clean/drivers/isdn/avmb1/capi.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/isdn/avmb1/capi.c	Wed Nov 28 11:08:08 2001
@@ -1055,7 +1055,6 @@
 {
 	struct capidev *cdev = (struct capidev *)file->private_data;
 
-	lock_kernel();
 	capincci_free(cdev, 0xffffffff);
 	capidev_free(cdev);
 	file->private_data = NULL;
@@ -1064,7 +1063,6 @@
 #ifdef _DEBUG_REFCOUNT
 	printk(KERN_DEBUG "capi_release %d\n", GET_USE_COUNT(THIS_MODULE));
 #endif
-	unlock_kernel();
 	return 0;
 }
 
@@ -1243,13 +1241,11 @@
 	struct capiminor *mp = (struct capiminor *)file->private_data;
 
 	if (mp) {
-		lock_kernel();
 		mp->file = 0;
 		if (mp->nccip == 0) {
 			capiminor_free(mp);
 			file->private_data = NULL;
 		}
-		unlock_kernel();
 	}
 
 #ifdef _DEBUG_REFCOUNT
diff -ur linux-2.5.1-pre2-clean/drivers/isdn/divert/divert_procfs.c linux/drivers/isdn/divert/divert_procfs.c
--- linux-2.5.1-pre2-clean/drivers/isdn/divert/divert_procfs.c	Sun Sep 30 12:26:05 2001
+++ linux/drivers/isdn/divert/divert_procfs.c	Wed Nov 28 11:08:08 2001
@@ -29,6 +29,7 @@
 ulong if_used = 0;		/* number of interface users */
 static struct divert_info *divert_info_head = NULL;	/* head of queue */
 static struct divert_info *divert_info_tail = NULL;	/* pointer to last entry */
+static spinlock_t divert_info_lock = SPIN_LOCK_UNLOCKED;/* lock for queue */
 static wait_queue_head_t rd_queue;
 
 /*********************************/
@@ -50,8 +51,7 @@
 		 return;	/* no memory */
 	strcpy(ib->info_start, cp);	/* set output string */
 	ib->next = NULL;
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave( &divert_info_lock, flags );
 	ib->usage_cnt = if_used;
 	if (!divert_info_head)
 		divert_info_head = ib;	/* new head */
@@ -70,6 +70,7 @@
 		} else
 			break;
 	}			/* divert_info_head->next */
+	spin_lock_irqrestore( &divert_info_lock, flags );
 	wake_up_interruptible(&(rd_queue));
 }				/* put_info_buffer */
 
@@ -135,17 +136,14 @@
 {
 	unsigned long flags;
 
-	lock_kernel();
-	save_flags(flags);
-	cli();
-	if_used++;
+	spin_lock_irqsave( &divert_info_lock, flags );
+ 	if_used++;
 	if (divert_info_head)
 		(struct divert_info **) filep->private_data = &(divert_info_tail->next);
 	else
 		(struct divert_info **) filep->private_data = &divert_info_head;
-	restore_flags(flags);
+	spin_unlock_irqrestore( &divert_info_lock, flags );
 	/*  start_divert(); */
-	unlock_kernel();
 	return (0);
 }				/* isdn_divert_open */
 
@@ -158,9 +156,7 @@
 	struct divert_info *inf;
 	unsigned long flags;
 
-	lock_kernel();
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave( &divert_info_lock, flags );
 	if_used--;
 	inf = *((struct divert_info **) filep->private_data);
 	while (inf) {
@@ -174,7 +170,7 @@
 			divert_info_head = divert_info_head->next;
 			kfree(inf);
 		}
-	unlock_kernel();
+	spin_unlock_irq( &divert_info_lock, flags );
 	return (0);
 }				/* isdn_divert_close */
 
diff -ur linux-2.5.1-pre2-clean/drivers/macintosh/adb.c linux/drivers/macintosh/adb.c
--- linux-2.5.1-pre2-clean/drivers/macintosh/adb.c	Tue Sep 18 14:23:14 2001
+++ linux/drivers/macintosh/adb.c	Wed Nov 28 11:08:08 2001
@@ -672,7 +672,6 @@
 	struct adbdev_state *state = file->private_data;
 	unsigned long flags;
 
-	lock_kernel();
 	if (state) {
 		file->private_data = NULL;
 		spin_lock_irqsave(&state->lock, flags);
@@ -685,7 +684,6 @@
 			spin_unlock_irqrestore(&state->lock, flags);
 		}
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/macintosh/via-pmu.c linux/drivers/macintosh/via-pmu.c
--- linux-2.5.1-pre2-clean/drivers/macintosh/via-pmu.c	Sat Sep  8 12:38:42 2001
+++ linux/drivers/macintosh/via-pmu.c	Wed Nov 28 11:08:08 2001
@@ -1983,7 +1983,6 @@
 	struct pmu_private *pp = file->private_data;
 	unsigned long flags;
 
-	lock_kernel();
 	if (pp != 0) {
 		file->private_data = 0;
 		spin_lock_irqsave(&all_pvt_lock, flags);
@@ -1991,7 +1990,6 @@
 		spin_unlock_irqrestore(&all_pvt_lock, flags);
 		kfree(pp);
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/message/i2o/i2o_config.c linux/drivers/message/i2o/i2o_config.c
--- linux-2.5.1-pre2-clean/drivers/message/i2o/i2o_config.c	Mon Oct 22 08:39:56 2001
+++ linux/drivers/message/i2o/i2o_config.c	Wed Nov 28 11:08:08 2001
@@ -847,7 +847,6 @@
 	struct i2o_cfg_info *p1, *p2;
 	unsigned long flags;
 
-	lock_kernel();
 	p1 = p2 = NULL;
 
 	spin_lock_irqsave(&i2o_config_lock, flags);
@@ -870,7 +869,6 @@
 		p1 = p1->next;
 	}
 	spin_unlock_irqrestore(&i2o_config_lock, flags);
-	unlock_kernel();
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/net/ppp_generic.c linux/drivers/net/ppp_generic.c
--- linux-2.5.1-pre2-clean/drivers/net/ppp_generic.c	Thu Oct 11 09:18:31 2001
+++ linux/drivers/net/ppp_generic.c	Wed Nov 28 11:08:09 2001
@@ -325,7 +325,6 @@
 {
 	struct ppp_file *pf = (struct ppp_file *) file->private_data;
 
-	lock_kernel();
 	if (pf != 0) {
 		file->private_data = 0;
 		if (atomic_dec_and_test(&pf->refcnt)) {
@@ -339,7 +338,6 @@
 			}
 		}
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/net/wan/cosa.c linux/drivers/net/wan/cosa.c
--- linux-2.5.1-pre2-clean/drivers/net/wan/cosa.c	Thu Sep 13 16:04:43 2001
+++ linux/drivers/net/wan/cosa.c	Wed Nov 28 11:08:09 2001
@@ -974,13 +974,11 @@
 	struct cosa_data *cosa;
 	unsigned long flags;
 
-	lock_kernel();
 	cosa = channel->cosa;
 	spin_lock_irqsave(&cosa->lock, flags);
 	cosa->usage--;
 	channel->usage--;
 	spin_unlock_irqrestore(&cosa->lock, flags);
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/pcmcia/ds.c linux/drivers/pcmcia/ds.c
--- linux-2.5.1-pre2-clean/drivers/pcmcia/ds.c	Mon Nov 12 09:39:01 2001
+++ linux/drivers/pcmcia/ds.c	Wed Nov 28 11:08:09 2001
@@ -597,7 +597,6 @@
     DEBUG(0, "ds_release(socket %d)\n", i);
     if ((i >= sockets) || (sockets == 0))
 	return 0;
-    lock_kernel();
     s = &socket_table[i];
     user = file->private_data;
     if (CHECK_USER(user))
@@ -615,7 +614,6 @@
     user->user_magic = 0;
     kfree(user);
 out:
-    unlock_kernel();
     return 0;
 } /* ds_release */
 
diff -ur linux-2.5.1-pre2-clean/drivers/pnp/isapnp_proc.c linux/drivers/pnp/isapnp_proc.c
--- linux-2.5.1-pre2-clean/drivers/pnp/isapnp_proc.c	Wed Jan 17 13:29:14 2001
+++ linux/drivers/pnp/isapnp_proc.c	Wed Nov 28 11:08:09 2001
@@ -170,12 +170,10 @@
 		kfree(buffer);
 		return -ENOMEM;
 	}
-	lock_kernel();
 	buffer->curr = buffer->buffer;
 	file->private_data = buffer;
 	if (mode == O_RDONLY)
 		isapnp_info_read(buffer);
-	unlock_kernel();
 	return 0;
 }
 
@@ -187,12 +185,10 @@
 	if ((buffer = (isapnp_info_buffer_t *) file->private_data) == NULL)
 		return -EINVAL;
 	mode = file->f_flags & O_ACCMODE;
-	lock_kernel();
 	if (mode == O_WRONLY)
 		isapnp_info_write(buffer);
 	vfree(buffer->buffer);
 	kfree(buffer);
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/scsi/sg.c linux/drivers/scsi/sg.c
--- linux-2.5.1-pre2-clean/drivers/scsi/sg.c	Sun Nov  4 09:31:57 2001
+++ linux/drivers/scsi/sg.c	Wed Nov 28 11:08:09 2001
@@ -336,9 +336,7 @@
     Sg_device * sdp;
     Sg_fd * sfp;
 
-    lock_kernel();
     if ((! (sfp = (Sg_fd *)filp->private_data)) || (! (sdp = sfp->parentdp))) {
-	unlock_kernel();
         return -ENXIO;
     }
     SCSI_LOG_TIMEOUT(3, printk("sg_release: dev=%d\n", MINOR(sdp->i_rdev)));
@@ -352,7 +350,6 @@
 	sdp->exclude = 0;
 	wake_up_interruptible(&sdp->o_excl_wait);
     }
-    unlock_kernel();
     return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/sgi/char/graphics.c linux/drivers/sgi/char/graphics.c
--- linux-2.5.1-pre2-clean/drivers/sgi/char/graphics.c	Thu Oct 11 09:43:30 2001
+++ linux/drivers/sgi/char/graphics.c	Wed Nov 28 11:08:09 2001
@@ -196,7 +196,6 @@
 	int board = GRAPHICS_CARD (inode->i_rdev);
 
 	/* Tell the rendering manager that one client is going away */
-	lock_kernel();
 	rrm_close (inode, file);
 
 	/* Was this file handle from the board owner?, clear it */
@@ -206,7 +205,6 @@
 			(*cards [board].g_reset_console)();
 		enable_gconsole ();
 	}
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/sgi/char/shmiq.c linux/drivers/sgi/char/shmiq.c
--- linux-2.5.1-pre2-clean/drivers/sgi/char/shmiq.c	Mon Aug 27 08:56:31 2001
+++ linux/drivers/sgi/char/shmiq.c	Wed Nov 28 11:08:09 2001
@@ -79,11 +79,13 @@
 
 /* /dev/qcntlN attached memory regions, location and size of the event queue */
 static struct {
-	int    opened;		/* if this device has been opened */
-	void   *shmiq_vaddr;	/* mapping in kernel-land */
-	int    tail;		/* our copy of the shmiq->tail */
-	int    events;
-	int    mapped;
+	int		opened;
+	void 	 	*shmiq_vaddr;		/* mapping in kernel-land */
+	spinlock_t	shmiq_lock:SPIN_LOCK_UNLOCKED;
+					 	/* protects vaddr and opened */
+	int   		tail;			/* our copy of the shmiq->tail */
+	int    		events;
+	int    		mapped;
 	
 	wait_queue_head_t    proc_list;
 	struct fasync_struct *fasync;
@@ -328,10 +330,10 @@
 
 	size  = vma->vm_end - vma->vm_start;
 	start = vma->vm_start; 
-	lock_kernel();
+	spin_lock( &shmiqs [minor].shmiq_lock );
 	mem = (unsigned long) shmiqs [minor].shmiq_vaddr =  vmalloc_uncached (size);
 	if (!mem) {
-		unlock_kernel();
+		spin_unlock( &shmiqs [minor].shmiq_lock );
 		return -EINVAL;
 	}
 
@@ -347,8 +349,7 @@
 	shmiqs [minor].tail = 0;
 	/* Init the shared memory input queue */
 	memset (shmiqs [minor].shmiq_vaddr, 0, size);
-	unlock_kernel();
-	
+	spin_unlock( &shmiqs [minor].shmiq_lock );
 	return error;
 }
 
@@ -393,13 +394,16 @@
 	minor--;
 	if (minor > MAX_SHMI_QUEUES)
 		return -EINVAL;
+	spin_lock( &shmiqs [minor].shmiq_lock );
 	if (shmiqs [minor].opened)
+	{
+		spin_unlock( &shmiqs [minor].shmiq_lock );
 		return -EBUSY;
+	}
 
-	lock_kernel ();
 	shmiqs [minor].opened      = 1;
 	shmiqs [minor].shmiq_vaddr = 0;
-	unlock_kernel ();
+	spin_unlock( &shmiqs [minor].shmiq_lock );
 
 	return 0;
 }
@@ -429,18 +433,19 @@
 
 	if (minor > MAX_SHMI_QUEUES)
 		return -EINVAL;
+
+	spin_lock( &shmiqs [minor].shmiq_lock );
 	if (shmiqs [minor].opened == 0)
 		return -EINVAL;
 
-	lock_kernel ();
 	shmiq_qcntl_fasync (-1, filp, 0);
-	shmiqs [minor].opened      = 0;
+	shmiqs [minor].opened 	   = 0;
 	shmiqs [minor].mapped      = 0;
 	shmiqs [minor].events      = 0;
 	shmiqs [minor].fasync      = 0;
 	vfree (shmiqs [minor].shmiq_vaddr);
 	shmiqs [minor].shmiq_vaddr = 0;
-	unlock_kernel ();
+	spin_unlock( &shmiqs [minor].shmiq_lock );
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/drivers/sgi/char/streamable.c linux/drivers/sgi/char/streamable.c
--- linux-2.5.1-pre2-clean/drivers/sgi/char/streamable.c	Wed Jul 12 21:58:43 2000
+++ linux/drivers/sgi/char/streamable.c	Wed Nov 28 11:08:09 2001
@@ -221,9 +221,7 @@
 static int
 sgi_mouse_close (struct inode *inode, struct file *filp)
 {
-	lock_kernel();
 	mouse_opened = 0;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/usb/dabusb.c linux/drivers/usb/dabusb.c
--- linux-2.5.1-pre2-clean/drivers/usb/dabusb.c	Fri Sep 14 14:04:07 2001
+++ linux/drivers/usb/dabusb.c	Wed Nov 28 11:08:09 2001
@@ -622,7 +622,6 @@
 
 	dbg("dabusb_release");
 
-	lock_kernel();
 	down (&s->mutex);
 	dabusb_stop (s);
 	dabusb_free_buffers (s);
@@ -636,7 +635,6 @@
 		wake_up (&s->remove_ok);
 
 	s->opened = 0;
-	unlock_kernel();
 	return 0;
 }
 
diff -ur linux-2.5.1-pre2-clean/drivers/usb/hiddev.c linux/drivers/usb/hiddev.c
--- linux-2.5.1-pre2-clean/drivers/usb/hiddev.c	Sat Oct 20 19:13:11 2001
+++ linux/drivers/usb/hiddev.c	Wed Nov 28 11:08:09 2001
@@ -193,7 +193,6 @@
 	struct hiddev_list *list = file->private_data;
 	struct hiddev_list **listptr;
 
-	lock_kernel();
 	listptr = &list->hiddev->list;
 	hiddev_fasync(-1, file, 0);
 
@@ -209,7 +208,6 @@
 	}
 
 	kfree(list);
-	unlock_kernel();
 
 	return 0;
 }
diff -ur linux-2.5.1-pre2-clean/fs/autofs4/root.c linux/fs/autofs4/root.c
--- linux-2.5.1-pre2-clean/fs/autofs4/root.c	Mon Oct 23 21:57:38 2000
+++ linux/fs/autofs4/root.c	Wed Nov 28 11:08:09 2001
@@ -202,8 +202,6 @@
 {
 	struct autofs_info *inf;
 
-	lock_kernel();
-
 	DPRINTK(("autofs4_dentry_release: releasing %p\n", de));
 
 	inf = autofs4_dentry_ino(de);
@@ -215,8 +213,6 @@
 
 		autofs4_free_ino(inf);
 	}
-
-	unlock_kernel();
 }
 
 /* For dentries of directories in the root dir */

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:05 [PATCH] remove BKL from drivers' release functions David C. Hansen
@ 2001-11-28 23:29 ` Andrew Morton
  2001-11-28 23:42   ` David C. Hansen
  2001-11-28 23:50   ` Robert Love
  2001-11-28 23:36 ` Alan Cox
  1 sibling, 2 replies; 26+ messages in thread
From: Andrew Morton @ 2001-11-28 23:29 UTC (permalink / raw)
  To: David C. Hansen; +Cc: linux-kernel

"David C. Hansen" wrote:
> 
> +static spinlock_t              user_list_lock;

	= SPIN_LOCK_UNLOCKED;

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:36 ` Alan Cox
@ 2001-11-28 23:32   ` David C. Hansen
  2001-11-28 23:45     ` Russell King
  2001-11-30  9:57     ` Alexander Viro
  0 siblings, 2 replies; 26+ messages in thread
From: David C. Hansen @ 2001-11-28 23:32 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel

Alan Cox wrote:

>>drivers' release functions.  The release functions are already 
>>serialized in the VFS code by an atomic_t which guarantees that each 
>>
>The release function was only partially serialized - what stops a release
>and an open racing each other ? That in several cases was why the lock
>was there. 
>
Nothing, because the BKL is not held for all opens anymore.  In most of 
the cases that we addressed, the BKL was in release _only_, not in open 
at all.  There were quite a few drivers where we added a spinlock, or 
used atomic operations to keep open from racing with release.  



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:05 [PATCH] remove BKL from drivers' release functions David C. Hansen
  2001-11-28 23:29 ` Andrew Morton
@ 2001-11-28 23:36 ` Alan Cox
  2001-11-28 23:32   ` David C. Hansen
  1 sibling, 1 reply; 26+ messages in thread
From: Alan Cox @ 2001-11-28 23:36 UTC (permalink / raw)
  To: David C. Hansen; +Cc: linux-kernel

> drivers' release functions.  The release functions are already 
> serialized in the VFS code by an atomic_t which guarantees that each 

The release function was only partially serialized - what stops a release
and an open racing each other ? That in several cases was why the lock
was there. 

I'm not saying your patch isnt a good thing. General rule of thumb, any user
of lock_kernel has a race 8)



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:29 ` Andrew Morton
@ 2001-11-28 23:42   ` David C. Hansen
  2001-11-28 23:50   ` Robert Love
  1 sibling, 0 replies; 26+ messages in thread
From: David C. Hansen @ 2001-11-28 23:42 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Andrew Morton wrote:

>"David C. Hansen" wrote:
>
>>+static spinlock_t              user_list_lock;
>>= SPIN_LOCK_UNLOCKED;
>>
Whoops.   Here's another patch to throw on top of the first one.

--- linux-2.5.1-pre2-dirty/arch/i386/kernel/apm.c    Wed Nov 28 15:37:59 
2001
+++ linux/arch/i386/kernelapm.c    Wed Nov 28 15:37:41 2001
@@ -387,7 +387,7 @@
 static DECLARE_WAIT_QUEUE_HEAD(apm_waitqueue);
 static DECLARE_WAIT_QUEUE_HEAD(apm_suspend_waitqueue);
 static struct apm_user *    user_list;
+static spinlock_t        user_list_lock = SPIN_LOCK_UNLOCKED;
-static spinlock_t        user_list_lock;
 
 static char            driver_version[] = "1.15";    /* no spaces */
 



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:32   ` David C. Hansen
@ 2001-11-28 23:45     ` Russell King
  2001-11-29  0:26       ` David C. Hansen
  2001-11-29 13:55       ` BALBIR SINGH
  2001-11-30  9:57     ` Alexander Viro
  1 sibling, 2 replies; 26+ messages in thread
From: Russell King @ 2001-11-28 23:45 UTC (permalink / raw)
  To: David C. Hansen; +Cc: Alan Cox, linux-kernel

On Wed, Nov 28, 2001 at 03:32:32PM -0800, David C. Hansen wrote:
> Nothing, because the BKL is not held for all opens anymore.  In most of 
> the cases that we addressed, the BKL was in release _only_, not in open 
> at all.  There were quite a few drivers where we added a spinlock, or 
> used atomic operations to keep open from racing with release.  

All char and block devs are opened with the BKL held - see chrdev_open in
fs/devices.c and do_open in fs/block_dev.c

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:29 ` Andrew Morton
  2001-11-28 23:42   ` David C. Hansen
@ 2001-11-28 23:50   ` Robert Love
  1 sibling, 0 replies; 26+ messages in thread
From: Robert Love @ 2001-11-28 23:50 UTC (permalink / raw)
  To: David C. Hansen; +Cc: linux-kernel

On Wed, 2001-11-28 at 18:42, David C. Hansen wrote:

> Whoops.   Here's another patch to throw on top of the first one.

You guys still looking to make global spinlocks static (i.e. not moving
to finer-grained locking but just making sure local locks are properly
static) ?

My tree has some work to this effect, perhaps I can toss you a patch ...

	Robert Love




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:45     ` Russell King
@ 2001-11-29  0:26       ` David C. Hansen
  2001-11-29  0:37         ` Jeff Garzik
  2001-11-29  0:41         ` Russell King
  2001-11-29 13:55       ` BALBIR SINGH
  1 sibling, 2 replies; 26+ messages in thread
From: David C. Hansen @ 2001-11-29  0:26 UTC (permalink / raw)
  To: Russell King; +Cc: Alan Cox, linux-kernel

Russell King wrote:

>On Wed, Nov 28, 2001 at 03:32:32PM -0800, David C. Hansen wrote:
>
>>Nothing, because the BKL is not held for all opens anymore.  In most of 
>>the cases that we addressed, the BKL was in release _only_, not in open 
>>at all.  There were quite a few drivers where we added a spinlock, or 
>>used atomic operations to keep open from racing with release.  
>>
>
>All char and block devs are opened with the BKL held - see chrdev_open in
>fs/devices.c and do_open in fs/block_dev.c
>
I wrote a quick and dirty char device driver to see if this happened. 
 If I run two tasks doing a bunch of opens and closes, the -EBUSY 
condition in the open function does happen.  Is my driver doing 
something wrong?

Here is the meat of the driver:

static int Device_Open = 0;

int testdev_open(struct inode *inode,  struct file *file)
{
  if ( test_and_set_bit(0,&Device_Open) )    {
      printk( "attempt to open testdev more than once\n" );
      return -EBUSY;
    }
  MOD_INC_USE_COUNT;
  return SUCCESS;
}

int testdev_release(struct inode *inode,  struct file *file)
{
  clear_bit(0,&Device_Open);
  MOD_DEC_USE_COUNT;
  return 0;
}

static int Major;
struct file_operations Fops = {
     open: testdev_open,
  release: testdev_release,
};

/* Initialize the module - Register the character device */
int init_module(void)
{
  Major = register_chrdev(0,
                          DEVICE_NAME,
                          &Fops);

  /* Negative values signify an error */
  if (Major < 0) {
    printk ("%s: %s device failed with %d\n",MODULE_NAME,
            "Sorry, registering the character",
            Major);
    return Major;
  }
  printk( "%s: loaded successfully on Major:%d\n",MODULE_NAME,Major);
  return 0;
}

void cleanup_module(void)
{
  int ret;
  ret = unregister_chrdev(Major, DEVICE_NAME);
  if (ret < 0)
    printk("%s: Error in unregister_MODULE_NAME: %d\n",
       MODULE_NAME, ret);
}





^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  0:26       ` David C. Hansen
@ 2001-11-29  0:37         ` Jeff Garzik
  2001-11-29  0:41         ` Russell King
  1 sibling, 0 replies; 26+ messages in thread
From: Jeff Garzik @ 2001-11-29  0:37 UTC (permalink / raw)
  To: David C. Hansen; +Cc: Russell King, Alan Cox, linux-kernel

"David C. Hansen" wrote:
> 
> Russell King wrote:
> 
> >On Wed, Nov 28, 2001 at 03:32:32PM -0800, David C. Hansen wrote:
> >
> >>Nothing, because the BKL is not held for all opens anymore.  In most of
> >>the cases that we addressed, the BKL was in release _only_, not in open
> >>at all.  There were quite a few drivers where we added a spinlock, or
> >>used atomic operations to keep open from racing with release.
> >>
> >
> >All char and block devs are opened with the BKL held - see chrdev_open in
> >fs/devices.c and do_open in fs/block_dev.c
> >
> I wrote a quick and dirty char device driver to see if this happened.
>  If I run two tasks doing a bunch of opens and closes, the -EBUSY
> condition in the open function does happen.  Is my driver doing
> something wrong?
> 
> Here is the meat of the driver:
> 
> static int Device_Open = 0;
> 
> int testdev_open(struct inode *inode,  struct file *file)
> {
>   if ( test_and_set_bit(0,&Device_Open) )    {
>       printk( "attempt to open testdev more than once\n" );
>       return -EBUSY;
>     }
>   MOD_INC_USE_COUNT;
>   return SUCCESS;
> }
> 
> int testdev_release(struct inode *inode,  struct file *file)
> {
>   clear_bit(0,&Device_Open);
>   MOD_DEC_USE_COUNT;
>   return 0;
> }

it is still racy, that's why struct file_operations and other structs
have an 'owner' member......

-- 
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  0:26       ` David C. Hansen
  2001-11-29  0:37         ` Jeff Garzik
@ 2001-11-29  0:41         ` Russell King
  2001-11-29  1:33           ` David C. Hansen
  1 sibling, 1 reply; 26+ messages in thread
From: Russell King @ 2001-11-29  0:41 UTC (permalink / raw)
  To: David C. Hansen; +Cc: linux-kernel

On Wed, Nov 28, 2001 at 04:26:16PM -0800, David C. Hansen wrote:
> I wrote a quick and dirty char device driver to see if this happened. 
>  If I run two tasks doing a bunch of opens and closes, the -EBUSY 
> condition in the open function does happen.  Is my driver doing 
> something wrong?

What's happening is:

task 1				task 2
-> sys_open
 -> lock_kernel
  -> testdev_open
   -> test_and_set_bit
     return success
    unlock kernel
				-> sys_open
				 -> lock_kernel (blocks on it)
				  -> testdev_open
				   -> test_and_set_bit
				     return -EBUSY
				    unlock kernel
				   return
   return

The BKL is only held for the duration of the open, not until you close the
device.

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  0:41         ` Russell King
@ 2001-11-29  1:33           ` David C. Hansen
  2001-11-29  1:42             ` Jeff Garzik
                               ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: David C. Hansen @ 2001-11-29  1:33 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel

Russell King wrote:

>The BKL is only held for the duration of the open, not until you close the
>device.
>
I'll need to go back and review the changes to the char and block 
devices.  I believe that a big chunk of the drivers that we changed were 
misc drivers, so I might still be in luck.

Does everyone agree that we need to get the BKL out of common areas like 
this?  For starters, what about adding a pair of spinlocks for block 
devices and character devices to take the place of the BKL in 
serializing opens?  Or, should we make it the driver's responsibility 
completely?

Dave Hansen
haveblue@us.ibm.com


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  1:33           ` David C. Hansen
@ 2001-11-29  1:42             ` Jeff Garzik
  2001-11-29  7:17               ` Oliver Neukum
  2001-11-29  1:47             ` Alan Cox
  2001-11-29  9:15             ` Russell King
  2 siblings, 1 reply; 26+ messages in thread
From: Jeff Garzik @ 2001-11-29  1:42 UTC (permalink / raw)
  To: David C. Hansen; +Cc: Russell King, linux-kernel

"David C. Hansen" wrote:
> 
> Russell King wrote:
> 
> >The BKL is only held for the duration of the open, not until you close the
> >device.
> >
> I'll need to go back and review the changes to the char and block
> devices.  I believe that a big chunk of the drivers that we changed were
> misc drivers, so I might still be in luck.
> 
> Does everyone agree that we need to get the BKL out of common areas like
> this?  For starters, what about adding a pair of spinlocks for block
> devices and character devices to take the place of the BKL in
> serializing opens?  Or, should we make it the driver's responsibility
> completely?

Ideally we should eliminate the BKL completely...  but you need to
review tons of code when messing around with it, which makes removal
annoying.  Al Viro seems to know this stuff pretty well, and possibly
has plans to remove BKL from various bits of the fs's.

Alan Cox made the comment on IRC that BKL was put into the release
method in 2.4 to synchronize open-close-power management.  Don't forget
to take that third into account either, in the cases where such applies.

	Jeff


-- 
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  1:33           ` David C. Hansen
  2001-11-29  1:42             ` Jeff Garzik
@ 2001-11-29  1:47             ` Alan Cox
  2001-11-29  9:15             ` Russell King
  2 siblings, 0 replies; 26+ messages in thread
From: Alan Cox @ 2001-11-29  1:47 UTC (permalink / raw)
  To: David C. Hansen; +Cc: Russell King, linux-kernel

> Does everyone agree that we need to get the BKL out of common areas like 
> this?  For starters, what about adding a pair of spinlocks for block 

Ideally the BKL itself should die.

> devices and character devices to take the place of the BKL in 
> serializing opens?  Or, should we make it the driver's responsibility 
> completely?

It needs to be the drivers job, to be documented as such and to be
implemented properly in some drivers. In paticular there are some extremely
interesting closedown races in existing drivers where it goes


		CPU1			CPU2
	release
	[do slow thing]
					open
					ioctl
						setting stuff up
	slow thing done
	trash the chip setup
	turn the chip off
	return
					ouch bang splat Oops!!!

Alan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  1:42             ` Jeff Garzik
@ 2001-11-29  7:17               ` Oliver Neukum
  0 siblings, 0 replies; 26+ messages in thread
From: Oliver Neukum @ 2001-11-29  7:17 UTC (permalink / raw)
  To: Jeff Garzik, David C. Hansen; +Cc: Russell King, linux-kernel


> Alan Cox made the comment on IRC that BKL was put into the release
> method in 2.4 to synchronize open-close-power management.  Don't forget
> to take that third into account either, in the cases where such applies.

And device unplugging please.

	Regards
		Oliver

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29  1:33           ` David C. Hansen
  2001-11-29  1:42             ` Jeff Garzik
  2001-11-29  1:47             ` Alan Cox
@ 2001-11-29  9:15             ` Russell King
  2 siblings, 0 replies; 26+ messages in thread
From: Russell King @ 2001-11-29  9:15 UTC (permalink / raw)
  To: David C. Hansen; +Cc: linux-kernel

On Wed, Nov 28, 2001 at 05:33:59PM -0800, David C. Hansen wrote:
> Does everyone agree that we need to get the BKL out of common areas like 
> this?  For starters, what about adding a pair of spinlocks for block 
> devices and character devices to take the place of the BKL in 
> serializing opens?  Or, should we make it the driver's responsibility 
> completely?

If you do happen to look at the serial driver, please note that I'm
currently working on a replacement driver, in cvs at:

   :pserver:cvs@pubcvs.arm.linux.org.uk:/mnt/src/cvsroot, module serial
   (no password)

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:45     ` Russell King
  2001-11-29  0:26       ` David C. Hansen
@ 2001-11-29 13:55       ` BALBIR SINGH
  2001-11-30 19:30         ` Rick Lindsley
  1 sibling, 1 reply; 26+ messages in thread
From: BALBIR SINGH @ 2001-11-29 13:55 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

Even register_blkdev(), etc hold BKL, without these there will be
a lot of problems, all these need to be taken care of if BKL is
ever replaced.

Just adding what I know,
Balbir  


Russell King wrote:

>On Wed, Nov 28, 2001 at 03:32:32PM -0800, David C. Hansen wrote:
>
>>Nothing, because the BKL is not held for all opens anymore.  In most of 
>>the cases that we addressed, the BKL was in release _only_, not in open 
>>at all.  There were quite a few drivers where we added a spinlock, or 
>>used atomic operations to keep open from racing with release.  
>>
>
>All char and block devs are opened with the BKL held - see chrdev_open in
>fs/devices.c and do_open in fs/block_dev.c
>
>--
>Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
>             http://www.arm.linux.org.uk/personal/aboutme.html
>
>-
>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/
>



[-- Attachment #2: InterScan_Disclaimer.txt --]
[-- Type: text/plain, Size: 855 bytes --]

-------------------------------------------------------------------------------------------------------------------------
Information transmitted by this E-MAIL is proprietary to Wipro and/or its Customers and
is intended for use only by the individual or entity to which it is
addressed, and may contain information that is privileged, confidential or
exempt from disclosure under applicable law. If you are not the intended
recipient or it appears that this mail has been forwarded to you without
proper authority, you are notified that any use or dissemination of this
information in any manner is strictly prohibited. In such cases, please
notify us immediately at mailto:mailadmin@wipro.com and delete this mail
from your records.
----------------------------------------------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-28 23:32   ` David C. Hansen
  2001-11-28 23:45     ` Russell King
@ 2001-11-30  9:57     ` Alexander Viro
  2001-11-30 12:41       ` Victor Yodaiken
                         ` (2 more replies)
  1 sibling, 3 replies; 26+ messages in thread
From: Alexander Viro @ 2001-11-30  9:57 UTC (permalink / raw)
  To: David C. Hansen; +Cc: Linus Torvalds, Alan Cox, linux-kernel



On Wed, 28 Nov 2001, David C. Hansen wrote:

> Alan Cox wrote:
> 
> >The release function was only partially serialized - what stops a release
> >and an open racing each other ? That in several cases was why the lock
> >was there. 

->release() is not serialized AT ALL.  It is serialized for given struct file,
but call open(2) twice and you've got two struct file for the same device.
close() both and you've got two calls of ->release(), quite possibly -
simultaneous.

> Nothing, because the BKL is not held for all opens anymore.  In most of 

RTFS.  fs/devices.c::chrdev_open().

> the cases that we addressed, the BKL was in release _only_, not in open 
> at all.  There were quite a few drivers where we added a spinlock, or 

See above.

> used atomic operations to keep open from racing with release.  

Right.  Including quite a few where you schedule under that spinlock.

In other words, patch is completely bogus.  BKL removal may be a good
idea, but you really need to audit the code.  Which requires at least
some understanding of the things you are doing.  There _are_ races
and they need to be dealt with.  But blind BKL removal doesn't fix any
and breaks quite a few places where the code was actually correct.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-30  9:57     ` Alexander Viro
@ 2001-11-30 12:41       ` Victor Yodaiken
  2001-11-30 20:02         ` Rick Lindsley
  2001-11-30 19:38       ` David C. Hansen
  2001-11-30 20:11       ` Rick Lindsley
  2 siblings, 1 reply; 26+ messages in thread
From: Victor Yodaiken @ 2001-11-30 12:41 UTC (permalink / raw)
  To: Alexander Viro; +Cc: David C. Hansen, Linus Torvalds, Alan Cox, linux-kernel

On Fri, Nov 30, 2001 at 04:57:28AM -0500, Alexander Viro wrote:
> In other words, patch is completely bogus.  BKL removal may be a good
> idea, but you really need to audit the code.  Which requires at least
> some understanding of the things you are doing.  There _are_ races
> and they need to be dealt with.  But blind BKL removal doesn't fix any
> and breaks quite a few places where the code was actually correct.

People seem to believe that BKL is worse than added spin-locks, but I
very much doubt this is always true.

> 
> -
> 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/

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-29 13:55       ` BALBIR SINGH
@ 2001-11-30 19:30         ` Rick Lindsley
  0 siblings, 0 replies; 26+ messages in thread
From: Rick Lindsley @ 2001-11-30 19:30 UTC (permalink / raw)
  To: BALBIR SINGH; +Cc: linux-kernel

    Even register_blkdev(), etc hold BKL, without these there will be
    a lot of problems, all these need to be taken care of if BKL is
    ever replaced.

I think it's pretty clear that the BKL has become so, um, multi-functional
over the years that any replacement will not involve a single new variable
(or algorithm).  On the plus side, it also means it can be a stepwise
project. If we carefully identify each function and replace it or obsolete
it, we can gradually deprecate the BKL at a safe and sane pace.

Everybody keep track of these usages; there will come a day where somebody
is asking you about it again :) open/release is the tip of the iceberg,
but it takes a long time for an iceberg to melt ....

Rick

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-30  9:57     ` Alexander Viro
  2001-11-30 12:41       ` Victor Yodaiken
@ 2001-11-30 19:38       ` David C. Hansen
  2001-11-30 23:12         ` Alexander Viro
  2001-11-30 20:11       ` Rick Lindsley
  2 siblings, 1 reply; 26+ messages in thread
From: David C. Hansen @ 2001-11-30 19:38 UTC (permalink / raw)
  To: Alexander Viro; +Cc: Alan Cox, linux-kernel, Rick Lindsley

Alexander Viro wrote:
 > ->release() is not serialized AT ALL.  It is serialized for given
 > struct file, but call open(2) twice and you've got two struct file
 > for the same device. close() both and you've got two calls of
 > ->release(), quite possibly - simultaneous.
OK, that clears some things up.  So, the file->fcount is only used in 
cases where the file descriptor was dup'd, right?

As Rick Lindsley pointed out to me:
 > In cases where we removed BKL from release() and left obvious locking
 > issues as "an exercise to the reader", we MAY have broken things
 > because the BKL (we now know) may have been serializing opens and
 > closes.
 > In cases where we replaced it with atomic locking or a spinlock, we've
 > done nothing but replace one lock with another (unless there are
 > subtleties

back to Alexander  Viro:
 > In other words, patch is completely bogus.
No, not completely.  In a lot of cases we just replaced some regular 
arithmetic with atomic instructions of some sort.  These changes are 
still completely valid.  But, in the cases where we added locking, we 
need to reevaluate them for potential problem.  In the cases where we 
just removed the BKL, we really need to check them to make sure that we 
didn't introduce anything.
Thanks for the feedback, Al!  This has been very helpful!

--
Dave Hansen
dave@sr71.net


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-30 12:41       ` Victor Yodaiken
@ 2001-11-30 20:02         ` Rick Lindsley
  0 siblings, 0 replies; 26+ messages in thread
From: Rick Lindsley @ 2001-11-30 20:02 UTC (permalink / raw)
  To: Victor Yodaiken
  Cc: Alexander Viro, David C. Hansen, Linus Torvalds, Alan Cox, linux-kernel

    People seem to believe that BKL is worse than added spin-locks, but
    I very much doubt this is always true.

Depends, to some extent, on your definition of "worse". To address
two: the BKL (in general) encourages "worse" design and promotes
"worse" performance. Combine these with a general lack of comments
where clever and subtle code is being employed and the pervasive use of
the BKL can make debugging (or design change) difficult.

The BKL *is* a spinlock, with some questionable special handling in
other parts of the code which permits and encourages poor programming
(like automatically releasing it before you go to sleep, and allowing
you to "acquire" it multiple times).  In general, if you do not need
the special handling, there are compelling reasons to replace it with a
purpose-specific lock (or locking mechanism).

Setting aside architectural and academic issues of "locking design",
the main thing the current practice does, from a performance
standpoint, is create a generic bottleneck.  "One lock to rule them all
..." It creates strange effects like heavy activity in one subsystem
suddenly bogging down a completely disjoint subsystem.  Two separate
locks, in that case, would allow one subsystem to be very busy without
affecting the other.

The advantages gained become more evident on multiple-cpu machines.
Since this is not your most common desktop variant :) the trick is to
introduce changes which allow that hardware to excel while not
impacting the more common case: the single cpu desktop or laptop.  This
is sometimes easy; often, however, it is not, and so the work goes
slowly.

Locks should have clearly defined purpose and scope, just like any
other component of a software system.  The BKL has evolved/mutated
beyond its original scope and needs to be trimmed back.  When it is
trimmed back to either one or zero purposes then, I believe, we can put
down the weed-whackers and stop trimming.

Rick

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-30  9:57     ` Alexander Viro
  2001-11-30 12:41       ` Victor Yodaiken
  2001-11-30 19:38       ` David C. Hansen
@ 2001-11-30 20:11       ` Rick Lindsley
  2 siblings, 0 replies; 26+ messages in thread
From: Rick Lindsley @ 2001-11-30 20:11 UTC (permalink / raw)
  To: Alexander Viro; +Cc: David C. Hansen, Linus Torvalds, Alan Cox, linux-kernel

I think you're exactly the right person to be helping out with this;
glad to see you join the discussion.

The biggest flaw in the patch was not realizing that chrdev_open held
the BKL for us during opens as well.  I don't think that invalidates it
in its entirety though.  In cases where the patch replaced BKL with
some other form of locking, we should be no worse off (but for your
concerns about scheduling) because we added it in the open routines
too.  In cases where we removed BKL from release but noted there were
"still" locking issues, yes, we need to examine those closely and fix
them in case we've created a locking issue where none existed before.

In most (all?) cases, the spinlocks are held briefly and (we believe)
not across scheduling opportunities.  If you see areas of the patch
where that is not true, I agree they should be addressed and please point
them out.

Rick

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-30 19:38       ` David C. Hansen
@ 2001-11-30 23:12         ` Alexander Viro
  2001-12-01  0:47           ` Rick Lindsley
  0 siblings, 1 reply; 26+ messages in thread
From: Alexander Viro @ 2001-11-30 23:12 UTC (permalink / raw)
  To: David C. Hansen; +Cc: Alan Cox, linux-kernel, Rick Lindsley



On Fri, 30 Nov 2001, David C. Hansen wrote:

> Alexander Viro wrote:
>  > ->release() is not serialized AT ALL.  It is serialized for given
>  > struct file, but call open(2) twice and you've got two struct file
>  > for the same device. close() both and you've got two calls of
>  > ->release(), quite possibly - simultaneous.
> OK, that clears some things up.  So, the file->fcount is only used in 
> cases where the file descriptor was dup'd, right?

Think for a minute - current IO position is in file->f_pos.  So if two
descriptors have independent current positions (lseek() on one of them
doesn't affect another) they have to have different instances of
struct file behind them.

struct file is an opened channel
descriptor refers to opened channel
file->f_count is the number of references to _this_ channel
dup() creates a descriptor refering to the same channel
so does fork()
open() creates a new channel and descriptor refering to it.

That difference between descriptors and opened channels exists in any
UNIX - otherwise you would either have lseek() done in one program
screwing everybody else or (if fork would create new channels instead
of new references to channels) have (ls; ls) > foo break (shell opens
foo and both ls(1) inherit it over fork(); you _really_ want changes
in current positions resulting from the first ls to affect the
stdout of the second one).
 
> back to Alexander  Viro:
>  > In other words, patch is completely bogus.
> No, not completely.  In a lot of cases we just replaced some regular 
> arithmetic with atomic instructions of some sort.  These changes are 
> still completely valid.  But, in the cases where we added locking, we 

Valid, but not necessary a good idea.  Notice that there are very real
races in ->release() and ->open() instances.  Removing BKL from these
(or replacing it with atomic operations) doesn't fix the existing race.
Moreover, it creates a false impression that thing had been reviewed
and fixed.

> need to reevaluate them for potential problem.  In the cases where we 
> just removed the BKL, we really need to check them to make sure that we 
> didn't introduce anything.

You really need that in all cases.  Sorry for "told you so", but that's
what I'd been trying to explain from the very beginning - back when the
idea of that patch had been discussed the first time.  That work really
has to be done driver-by-driver...


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-11-30 23:12         ` Alexander Viro
@ 2001-12-01  0:47           ` Rick Lindsley
  2001-12-01  9:52             ` Alan Cox
  0 siblings, 1 reply; 26+ messages in thread
From: Rick Lindsley @ 2001-12-01  0:47 UTC (permalink / raw)
  To: Alexander Viro; +Cc: David C. Hansen, Alan Cox, linux-kernel

    > back to Alexander  Viro:
    >  > In other words, patch is completely bogus.
    > No, not completely.  In a lot of cases we just replaced some regular 
    > arithmetic with atomic instructions of some sort.  These changes are 
    > still completely valid.  But, in the cases where we added locking, we 
    
    Valid, but not necessary a good idea.  Notice that there are very
    real races in ->release() and ->open() instances.  Removing BKL
    from these (or replacing it with atomic operations) doesn't fix the
    existing race.  Moreover, it creates a false impression that thing
    had been reviewed and fixed.

Well, but for the revelation that the BKL is held in chrdev_open(),
offering implicit serialization for all open routines, these *were*
reviewed and thought fixed.  Atomic operations *do* fill the bill when
we are looking at counts used to guarantee exclusive opens.  They
generally *don't* fill the bill when you want to take virtually any
other action based on those counts.

    Sorry for "told you so", but that's what I'd been trying to explain
    from the very beginning - back when the idea of that patch had been
    discussed the first time.  That work really has to be done
    driver-by-driver...

It actually *was* in this case.  This may look like an afternoon of
editor exercise, but each of the cases was inspected and reviewed
before recommending the patch, and collectively represents several
weeks' worth of work.  In early November I posted a note to
linux-kernel which said, in part:

    Before we post these patches, I thought I'd ask if in the time
    since Al Viro moved this out here (July 2000) if anybody
    (especially him!) has found a *legitimate* use of the BKL in the
    release() functions. (We have not found one.)

There was no response.

Given the miss on chrdev_open(), we've got some patch patches to come
up with.  However, this isn't a dump-and-run job.  We accept the
responsibility for those patches, and we'll patch 'em up.  While we
can't then miraculously announce it's safe to remove lock_kernel around
opens, we CAN announce we are closer, and nothing else will be any more
broken than it was before we started. As I said in an earlier post (and
I don't think anybody disagrees): this is an incremental task. Any
assistance you can provide in reviewing this work, including all to
date, is welcome.

Rick

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-12-01  0:47           ` Rick Lindsley
@ 2001-12-01  9:52             ` Alan Cox
  2001-12-01 10:06               ` Rick Lindsley
  0 siblings, 1 reply; 26+ messages in thread
From: Alan Cox @ 2001-12-01  9:52 UTC (permalink / raw)
  To: Rick Lindsley; +Cc: Alexander Viro, David C. Hansen, Alan Cox, linux-kernel

> opens, we CAN announce we are closer, and nothing else will be any more
> broken than it was before we started. As I said in an earlier post (and
> I don't think anybody disagrees): this is an incremental task. Any

This is why we have a development tree. Its moving things in the right
direction which is important. I suspect many drivers will want to use
semaphores rather than atomic counts however, to ensure that an open doesn't
complete while a previous release is still shutting down hardware

Alan


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] remove BKL from drivers' release functions
  2001-12-01  9:52             ` Alan Cox
@ 2001-12-01 10:06               ` Rick Lindsley
  0 siblings, 0 replies; 26+ messages in thread
From: Rick Lindsley @ 2001-12-01 10:06 UTC (permalink / raw)
  To: Alan Cox; +Cc: Alexander Viro, David C. Hansen, linux-kernel

    This is why we have a development tree. Its moving things in the
    right direction which is important. I suspect many drivers will
    want to use semaphores rather than atomic counts however, to ensure
    that an open doesn't complete while a previous release is still
    shutting down hardware

Yes, the only successful application for atomic counts that I've seen
(in this context) is for exclusive open code that looks like

    if (count++) {
	count--;
	return -EBUSY;
    }

in the open routine and

    count--;

in the release.  If you want to do anything else as a result of that
count, you'll need additional locking because it could have changed a
nanosecond after it was (safely) incremented or decremented. You can't
count on it remaining that value after you check it.

release()s that want to shutdown the device, free memory, or take other
actions will want to employ either a spinlock (plain or r/w as
appropriate) or a sleeping semaphore to insure things remain stable
until those actions are complete.

Rick

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2001-12-01 10:07 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-11-28 23:05 [PATCH] remove BKL from drivers' release functions David C. Hansen
2001-11-28 23:29 ` Andrew Morton
2001-11-28 23:42   ` David C. Hansen
2001-11-28 23:50   ` Robert Love
2001-11-28 23:36 ` Alan Cox
2001-11-28 23:32   ` David C. Hansen
2001-11-28 23:45     ` Russell King
2001-11-29  0:26       ` David C. Hansen
2001-11-29  0:37         ` Jeff Garzik
2001-11-29  0:41         ` Russell King
2001-11-29  1:33           ` David C. Hansen
2001-11-29  1:42             ` Jeff Garzik
2001-11-29  7:17               ` Oliver Neukum
2001-11-29  1:47             ` Alan Cox
2001-11-29  9:15             ` Russell King
2001-11-29 13:55       ` BALBIR SINGH
2001-11-30 19:30         ` Rick Lindsley
2001-11-30  9:57     ` Alexander Viro
2001-11-30 12:41       ` Victor Yodaiken
2001-11-30 20:02         ` Rick Lindsley
2001-11-30 19:38       ` David C. Hansen
2001-11-30 23:12         ` Alexander Viro
2001-12-01  0:47           ` Rick Lindsley
2001-12-01  9:52             ` Alan Cox
2001-12-01 10:06               ` Rick Lindsley
2001-11-30 20:11       ` Rick Lindsley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).