Greg KH wrote:
> On Sun, Jul 07, 2002 at 03:42:56PM -0700, Dave Hansen wrote:
>
>>BKL use isn't right or wrong -- it isn't a case of creating a deadlock
>>or a race. I'm picking a relatively random function from "grep -r
>>lock_kernel * | grep /usb/". I'll show what I think isn't optimal
>>about it.
>>
>>"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?
>
>
> Nice example, it proves my previous points:
> - you didn't send this to the author of the code, who is very
> responsive when you do.
> - you didn't send this to the linux-usb-devel list, which is a
> very responsive list.
> - this is in the file drivers/usb/host/uhci-debug.c, which by
> its very nature leads you to believe that this is not critical
> code at all. This is true if you look at the code.
> - it looks like you could just remove the BKL entirely from this
> call, and not just move it around the kmalloc() call. But
> since I don't understand all of the different locking rules
> inside the uhci-hcd.c driver, I'm not going to do this. I'll
> let the author of the driver do that, incase it really matters
> (and yes, the locking in the uhci-hcd driver is tricky, but
> nicely documented.)
> - this file is about to be radically rewritten, to use driverfs
> instead of procfs, as the recent messages on linux-usb-devel
> state. So any patch you might make will probably be moot in
> about 3 days :) Again, contacting the author/maintainer is
> the proper thing to do.
You are taking this example way too seriously. Thunder wanted an
example and I grabbed the first one that I saw (I created it in the
last hour). I showed you how I arrived at it, just a quick grepping.
It wan't a real patch, only a quick little example snippet.
> - even if you remove the BKL from this code, what have you
> achieved? A faster kernel? A very tiny bit smaller kernel,
> yes, but not any faster overall. This is not on _any_
> critical path.
How many times do I have to say it? We're going around in circles
here. I _know_ that it isn't on a critical path, or saving a large
quantity of program text. I just think that it is better than it was
before.
-- 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