On Fri, Sep 03, 2021 at 11:48:22PM +0300, Vladimir Oltean wrote: > On Fri, Sep 03, 2021 at 01:04:19AM +0100, Russell King (Oracle) wrote: > > Removing a lock and then running the kernel is a down right stupid > > way to test to see if a lock is necessary. > > > > That approach is like having built a iron bridge, covered it in paint, > > then you remove most the bolts, and then test to see whether it's safe > > for vehicles to travel over it by riding your bicycle across it and > > declaring it safe. > > > > Sorry, but if you think "remove lock, run kernel, if it works fine > > the lock is unnecessary" is a valid approach, then you've just > > disqualified yourself from discussing this topic any further. > > Locking is done by knowing the code and code analysis, not by > > playing "does the code fail if I remove it" games. I am utterly > > shocked that you think that this is a valid approach. > > ... and this is exactly why you will no longer get any attention from me > on this topic. Good luck. Good, because your approach to this to me reads as "I don't think you know what the hell you're doing so I'm going to remove a lock to test whether it is needed." Effectively, that action is an insult towards me as the author of that code. And as I said, if you think that's a valid approach, then quite frankly I don't want you touching my code, because you clearly don't know what you're doing as you aren't willing to put the necessary effort in to understanding the code. Removing a lock and running the kernel is _never_ a valid way to see whether the lock is required or not. The only way is via code analysis. I wonder whether you'd take the same approach with filesystems or memory management code. Why don't you try removing some locks from those subsystems and see how long your filesystems last? You could have asked why the lock was necessary, and I would have described it. That would have been the civil approach. Maybe even put forward a hypothesis why you think the lock isn't necessary, but no, you decide that the best way to go about this is to remove the lock and see whether the kernel breaks. It may shock you to know that those of us who have been working on the kernel for almost 30 years and have seen the evolution of the kernel from uniprocessor to SMP, have had to debug race conditions caused by a lack of locking know very well that you can have what seems to be a functioning kernel despite missing locks - and such a kernel can last quite a long time and only show up the race quite rarely. This is exactly why "lets remove the lock and see if it breaks" is a completely invalid approach. I'm sorry that you don't seem to realise just how stupid a suggestion that was. I can tell you now: removing the locks you proposed will not show an immediate problem, but by removing those locks you will definitely open up race conditions between driver binding events on the SFP side and network usage on the netdev side which will only occur rarely. And just because they only happen rarely is not a justification to remove locks, no matter how inconvenient those locks may be. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!