Re: BKL removal

From: Dave Hansen (haveblue@us.ibm.com)
Date: Sun Jul 07 2002 - 18:45:21 EST


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

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



This archive was generated by hypermail 2b29 : Sun Jul 07 2002 - 22:00:18 EST