Messages in this thread | | | Date | Tue, 26 Oct 2021 12:56:02 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next] netdevsim: Register and unregister devlink traps on probe/remove device |
| |
On Tue, 26 Oct 2021 22:30:23 +0300 Leon Romanovsky wrote: > On Tue, Oct 26, 2021 at 12:02:34PM -0700, Jakub Kicinski wrote: > > On Tue, 26 Oct 2021 19:14:58 +0300 Leon Romanovsky wrote: > > > I understand your temptation to send revert, at the end it is the > > > easiest solution. However, I prefer to finish this discussion with > > > decision on how the end result in mlxsw will look like. > > > > > > Let's hear Jiri and Jakub before we are rushing to revert something that > > > is correct in my opinion. We have whole week till merge window, and > > > revert takes less than 5 minutes, so no need to rush and do it before > > > direction is clear. > > > > Having drivers in a broken state will not be conducive to calm discussions. > > Let's do a quick revert and unbreak the selftests. > > No problem, I'll send a revert now, but what is your take on the direction?
I haven't put in the time to understand the detail so I was hoping not to pass judgment on the direction. My likely unfounded feeling is that reshuffling ordering is not going to fix what is fundamentally a locking issue. Driver has internal locks it needs to hold both inside devlink callbacks and when registering devlink objects. We would solve a lot of the problems if those were one single lock instead of two. At least that's my recollection from the times I was actually writing driver code...
> IMHO, the mlxsw layering should be fixed. All this recursive devlink re-entry > looks horrible and adds unneeded complexity.
If you're asking about mlxsw or bnxt in particular I wouldn't say what they do is wrong until we can point out bugs.
| |