BKL in tiglusb release function
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.
Signature · 2 lines
Dave Hansen [email protected]
drivers/usb/tiglusb.c +0 -2
@@ -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 ; }
Quoted 5 lines · Romain Liévin
I have followed examples from other codes such as
dabusb.c&printer.c(2.4.14). After a further look: -dabusb.cfrom 2.4.14 contains a kernel_lock/unlock couple only in_releasebut it has been removed in 2.5.7 -printer.ccontains a kernel_lock/unlock couple in_open&_releasein 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.
Signature · 2 lines
Dave Hansen [email protected]