Re: BKL removal

[Dave Hansen <haveblue@us.ibm.com>]

Oliver Neukum wrote:
 >>> "up" is a local variable.  There is no point in protecting its
 >>> allocation.  If the goal is to protect data inside "up", there
 >>> should probably be a subsystem-level lock for all "struct
 >>> uhci_hcd"s or a lock contained inside of the structure itself.
 >>> Is this the kind of example you're looking for?
 >>
 >> So the BKL isn't wrong here, but incorrectly used?
 >
 > The BKL, unless used unbalanced, can never cause a bug. It could be
 > insufficient or superfluous, but never be really buggy in itself.

Does incredibly high lock contention qualify as a bug?

 >> Is it really okay to "lock the whole kernel" because of one
 >> struct file? This brings us back to spinlocks...
 >>
 >> You're possibly right about this one. What did Greg K-H say?
 >
 > I don't speak for Greg, but in this example it could be dropped
 > IMO. The spinlock protects the critical section anyway. As a rule,
 > if you do a kmalloc without GFP_ATOMIC under BKL you are doing
 > either insufficient locking (you may need a semaphore) or useless
 > locking.

Don't forget that the BKL is released on sleep.  It is OK to hold it
over a schedule()able operation.  If I remember right, there is no 
real protection needed for the file->private_data either because there 
is 1 and only 1 struct file per open, and the data is not shared among 
struct files.

 > This should have been posted on linux-usb long ago.

I just pulled it out of thin air 10 minutes ago!

-- 
Dave Hansen
haveblue@us.ibm.com