Messages in this thread | | | Date | Sun, 7 Jul 2002 16:51:14 -0700 | From | Greg KH <> | Subject | Re: BKL removal |
| |
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. - 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.
thanks,
greg k-h - 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/
| |