BKL in tiglusb release function

BKL in tiglusb release function

[Dave Hansen]

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

Is there a reason for the BKL to be used in tiglusb_release()?  Are you 
worried about a race between open and release, or were you just 
following examples from other code?

I'm sure we can remove it safely.  We might need another lock, but it 
won't be much.

-- 
Dave Hansen
haveblue@us.ibm.com

[-- Attachment #2: tiglusb-bkl_remove-2.5.8-pre1.patch --]
[-- Type: text/plain, Size: 389 bytes --]

--- linux-2.5.8-pre1-clean/drivers/usb/tiglusb.c	Thu Apr  4 08:58:26 2002
+++ linux/drivers/usb/tiglusb.c	Thu Apr  4 16:29:31 2002
@@ -128,7 +128,6 @@
 {
 	ptiglusb_t s = (ptiglusb_t) file->private_data;
 
-	lock_kernel ();
 	down (&s->mutex);
 	s->state = _stopped;
 	up (&s->mutex);
@@ -139,7 +138,6 @@
 		wake_up (&s->remove_ok);
 
 	s->opened = 0;
-	unlock_kernel ();
 
 	return 0;
 }

Re: BKL in tiglusb release function

[Dave Hansen]

Romain Liévin wrote:
 > I have followed examples from other codes such as dabusb.c &
 > printer.c (2.4.14). After a further look: - dabusb.c from 2.4.14
 > contains a kernel_lock/unlock couple only in _release but it has
 > been removed in 2.5.7 - printer.c contains a kernel_lock/unlock
 > couple in _open & _release in both 2.4.14 and 2.5.7

I was probably responsible for the removal from dabusb.c.  In almost all
cases that I found (over 60 of them), the BKL is not needed in the 
release function.

The use of s->mutex confuses me.  In tiglusb_disconnect(), there is an 
up().  Where is the matching down?  The comment for the mutex just says, 
"locks this struct".  However, it is clear that it doesn't protect the 
whole structure.  There are many places where structure members are 
accessed without the lock being held.

-- 
Dave Hansen
haveblue@us.ibm.com