Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 26 Oct 2021 12:56:02 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     Leon Romanovsky <leon@kernel.org>
Cc:     Ido Schimmel <idosch@idosch.org>,
        "David S . Miller" <davem@davemloft.net>,
        Ido Schimmel <idosch@mellanox.com>,
        Jiri Pirko <jiri@mellanox.com>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org,
        syzbot+93d5accfaefceedf43c1@syzkaller.appspotmail.com,
        Edwin Peer <edwin.peer@broadcom.com>
Subject: Re: [PATCH net-next] netdevsim: Register and unregister devlink
 traps on probe/remove device
Message-ID: <20211026125602.7a8f8b7e@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <YXhXT/u9bFADwEIo@unreal>
References: <YXUtbOpjmmWr71dU@unreal>
        <YXU5+XLhQ9zkBGNY@shredder>
        <YXZB/3+IR6I0b2xE@unreal>
        <YXZl4Gmq6DYSdDM3@shredder>
        <YXaNUQv8RwDc0lif@unreal>
        <YXelYVqeqyVJ5HLc@shredder>
        <YXertDP8ouVbdnUt@unreal>
        <YXgMK2NKiiVYJhLl@shredder>
        <YXgpgr/BFpbdMLJp@unreal>
        <20211026120234.3408fbcc@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <YXhXT/u9bFADwEIo@unreal>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.