Greg KH wrote:
>>It is not just a matter of scalability, but maintainability. I find
>>it hard to read and understand code which uses the BKL. As Greg said
>>in his OLS coding style talk, I want you to be able to read my code to
>>find _my_ mistakes. Your ability to do that will be impaired by every
>>bad indentation, typedef, and BKL use.
>
> Excuse me, but please do NOT compare coding style with using the BKL! I
> can use the BKL in my code just fine, and it does not impare your
> ability to use, modify, or understand my code. As long as I comment why
> I am using the BKL.
If it was easy to modify, I wouldn't be making such a mess of things,
right? (this assumes I'm _not_ a blundering idiot, which hasn't been
proven)
"As long as I comment [and understand] why I am using the BKL."
would be a bit more accurate. How many places in the kernel have you
seen comments about what the BKL is actually doing? Could you point
me to some of your comments where _you_ are using the BKL? Once you
fully understand why it is there, the extra step of removal is usually
very small.
>>A lock with a single purpose, guarding relatively small amounts of
>>data is much easier to understand than one such as the BKL. Would you
>>want a simple VM operation to take 1 second as the TTY layer and ext3
>>take their sweet time with the BKL?
>
> If ext3 is spinning on the BKL, then try to fix that, as it seems like a
> worthwhile task (like the ext2 changes proved.) If you want to remove
> the BKL from the tty layer, be my guest, that will involve rewriting
> that whole subsystem :)
All that I'm saying is that there can be unintended consequences. By
having it in your code, you open the possibility of these strange
interactions and a drop in _your_ code's reliability.
I'm staying well away from the TTY layer, it is just a well known
example.
>>I would mind the BKL a lot less if it was as well understood
>>everywhere as it is in VFS. The funny part is that a lock like the
>>BKL would not last very long if it were well understood or documented
>>everywhere it was used.
>
> I would mind it a whole lot less if when you try to remove the BKL, you
> do it correctly. So far it seems like you enjoy doing "hit and run"
> patches, without even fully understanding or testing your patches out
> (the driverfs and input layer patches are proof of that.) This does
> nothing but aggravate the developers who have to go clean up after you.
Like it or not, the only way to get maintainers to pay any attention
at all is to give them code. Is there more likely to be progress if I
just say, "Hey Greg, why don't you take the BKL out of USB", or if I
send you a crappy patch which has the beginning of a valid approach?
Code gets people thinking.
> Also, stay away from instances of it's use WHERE IT DOES NOT MATTER for
> performance. If I grab the BKL on insertion or removal of a USB device,
> who cares? I know you are trying to remove it entirely out of the
> kernel, but please focus on places where it actually helps, and leave
> the other instances alone.
You're sorely mistaken because I'm not trying to remove it entirely
from the kernel. I wouldn't cry if that happened tomorrow, but I
don't have any delusions about it happening any time soon.
Call me "Ike", but I'm a domino theorist. One instance of the BKL
spawns many more over time, because they tend to proliferate for no
other reason than "just to be safe". The hardest ones to remove are
those that shouldn't have been there in the first place.
--
Dave Hansen
[email protected]
Hi,
On Sun, 7 Jul 2002, Dave Hansen wrote:
> "As long as I comment [and understand] why I am using the BKL."
> would be a bit more accurate. How many places in the kernel have you
> seen comments about what the BKL is actually doing? Could you point
> me to some of your comments where _you_ are using the BKL? Once you
> fully understand why it is there, the extra step of removal is usually
> very small.
Old Blue, could you please bring me an example on where in USB the bkl
shouldn't be used, but is? And can you explain why it's wrong to use bkl
there?
Regards,
Thunder
--
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o? K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y-
------END GEEK CODE BLOCK------
On Sun, Jul 07, 2002 at 02:35:31PM -0700, Dave Hansen wrote:
>
> "As long as I comment [and understand] why I am using the BKL."
> would be a bit more accurate. How many places in the kernel have you
> seen comments about what the BKL is actually doing? Could you point
> me to some of your comments where _you_ are using the BKL? Once you
> fully understand why it is there, the extra step of removal is usually
> very small.
I have never written any kernel code that added usages of the BKL. The
fact that I am now maintaining code that uses it should not be confused
with this. Yes, some of my filesystems (pcihpfs and usbfs) now use it,
but that was due to other people pushing it down, out of the VFS.
> >>A lock with a single purpose, guarding relatively small amounts of
> >>data is much easier to understand than one such as the BKL. Would you
> >>want a simple VM operation to take 1 second as the TTY layer and ext3
> >>take their sweet time with the BKL?
> >
> >If ext3 is spinning on the BKL, then try to fix that, as it seems like a
> >worthwhile task (like the ext2 changes proved.) If you want to remove
> >the BKL from the tty layer, be my guest, that will involve rewriting
> >that whole subsystem :)
>
> All that I'm saying is that there can be unintended consequences. By
> having it in your code, you open the possibility of these strange
> interactions and a drop in _your_ code's reliability.
Yes, and there can be unintended conequences for just blindly removing
the BKL as your input layer patch proved. The input layer was using the
BKL in a reliable manner, it just wasn't documented. And by using the
BKL in my code, it does not decrease reliability in any way (just might
slow it down under some system loads, but it still works properly.)
> I'm staying well away from the TTY layer, it is just a well known
> example.
Ah, so it's easier to try to pick on subsystems that don't matter like
USB and driverfs? :)
> >>I would mind the BKL a lot less if it was as well understood
> >>everywhere as it is in VFS. The funny part is that a lock like the
> >>BKL would not last very long if it were well understood or documented
> >>everywhere it was used.
> >
> >I would mind it a whole lot less if when you try to remove the BKL, you
> >do it correctly. So far it seems like you enjoy doing "hit and run"
> >patches, without even fully understanding or testing your patches out
> >(the driverfs and input layer patches are proof of that.) This does
> >nothing but aggravate the developers who have to go clean up after you.
>
> Like it or not, the only way to get maintainers to pay any attention
> at all is to give them code. Is there more likely to be progress if I
> just say, "Hey Greg, why don't you take the BKL out of USB", or if I
> send you a crappy patch which has the beginning of a valid approach?
> Code gets people thinking.
Either way, you get my same response, "Why?" Again, as someone stated,
where in the USB code is the BKL used that affects performance in any
real life situations?
And yes, sending crappy patches does get people jumping, but don't rely
on that being a good method of doing development. People only fall for
that one so many times.
greg k-h
In article <[email protected]> you wrote:
> Either way, you get my same response, "Why?" Again, as someone stated,
> where in the USB code is the BKL used that affects performance in any
> real life situations?
AFAIK the BKL in a not often used path can still be hold too long and affect
latecy. I think the most recent low latency patches find a few instances. I
am not completly shure if that is only about interrupts, or if it applies to
the BKL, too.
Greetings
Bernd