From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6E5AE3B3BFC; Thu, 11 Jun 2026 15:54:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781193284; cv=none; b=FWdmUTDNQHhu0T3yAFTN/kzlD1JEMl8M9WsWA0is1GX9PIqpGlEtKIp0tOvYH+AarK4zsOaofMwolJ+u6+sYMVglfpkhuKgw88isNkNV6RYQ+Ak6XyI4T0fIceIZR6T7/e467OrF9mVwAiCDoBx/FtfqFS4IeQbgby47ZfTTofE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781193284; c=relaxed/simple; bh=0d6ab74yd5kEzFzfTr1wPyxvkvewk0FCwPFPD3Na4h4=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=kCetR5WyhtDA9HpjNjxYbQ1lEwL2Yiw/tuSpusowMpwl5V5jmA+A+F6GIZxmCN3Wl8xX8qlM2eCuVlx43yJucnUMmFH8AJtyC+SloxHLY8bZ2eDgeyEyAMS+BJnPjN+oMWhD/azEqY5J+PiFzmrBN9u4fx4RYn6YGWhsJb4jcD8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=SnwA76P4; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="SnwA76P4" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3F8B71F00893; Thu, 11 Jun 2026 15:54:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1781193282; bh=4TQhWa4URtxn6HrzLpdqS4kgU4581lhO4yl/qFTymkc=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=SnwA76P4YZBcMd0y+S6MCgLpL3brG1sTKA34JB15OSIS6LCb1xEyXCNV0rSksVBmw 0DHxvZ9dGH4p9JXJJFmATayu40QWvAPiaX+cq44LnT4dqyEtFDO5QMjIfUHiVMu2OK 76xiQFVia85GAv+hDVXk3h5Fi2r9oT/LDzCAIBeDXZOycd70P36uD9YnUNTLDoCzd1 vIOxHZJHiEsPbQ+2l3bc4A7fzBW7gUubR7jBiX23LHJSROmYZK3vPjS4hBMFXSYMyV 5zUp/6lGmdwGeJ3WhRHLkEoQKg0ptvhaJsXJYuvr28sg2KetnREhKbwKNfMMJFpfOn CaUaM5qwlEgmw== Date: Thu, 11 Jun 2026 08:54:40 -0700 From: Jakub Kicinski To: Mark Bloch Cc: Eric Dumazet , Paolo Abeni , Andrew Lunn , "David S. Miller" , Jonathan Corbet , Shuah Khan , Jiri Pirko , Simon Horman , Sunil Goutham , Linu Cherian , Geetha sowjanya , hariprasad , Subbaraya Sundeep , Bharat Bhushan , Saeed Mahameed , Leon Romanovsky , Tariq Toukan , Ethan Nelson-Moore , linux-doc@vger.kernel.org, netdev@vger.kernel.org, linux-rdma@vger.kernel.org Subject: Re: [PATCH net-next V3 2/7] netdevsim: Register devlink after device init Message-ID: <20260611085440.4fe36bf2@kernel.org> In-Reply-To: References: <20260605181030.3486619-1-mbloch@nvidia.com> <20260605181030.3486619-3-mbloch@nvidia.com> <20260610165053.7c91f331@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 11 Jun 2026 09:02:03 +0300 Mark Bloch wrote: > On 11/06/2026 2:50, Jakub Kicinski wrote: > > On Fri, 5 Jun 2026 21:10:25 +0300 Mark Bloch wrote: > >> devl_register() makes the devlink instance visible to userspace. A later > >> patch also makes registration the point where devlink core may call > >> eswitch_mode_set() to apply a boot-time default eswitch mode. > >> > >> Move netdevsim registration after all objects (resources, params, regions, > >> traps, debugfs etc) are initialized, and after the initial eswitch mode is > >> set to legacy. > >> > >> Move devl_unregister() to the beginning of nsim_drv_remove(), before those > >> devlink objects are torn down. This keeps devlink register/unregister as > >> the notification barrier and makes the later object teardown paths run > >> after devlink is no longer registered, so they do not emit their own > >> netlink DEL notifications. > > > > This is going backwards. At some point someone from nVidia thought that > > we can order our way out of locking, so mlx5 is likely ordered this way, > > but this must not be required, or in any way normalized. > > We (syzbot) quickly discovered that it doesn't cover all corner cases. > > devl_lock() is exposed specifically to allow the driver to finish > > whatever init it needs without letting user space invoke callbacks, yet. > > Almost (?) all driver callbacks hold devl_lock(), so maybe the devlink > > instance is "visible" to user space but that should not matter. > > Let me clarify. > > No locking is changed here, and I don't want to make register/unregister > ordering a substitute for devl_lock(). > > The only requirement I have for this series is that devl_register() is called > only once the driver is ready for devlink core to call eswitch_mode_set(). > That follows from the earlier direction to have the core apply the default > mode from devl_register() instead of adding an explicit driver call. This is exactly what I'm objecting to. AFAIU we are trading off explicit call to get the default value for an implicit behavior depending on order of calls. We want to optimize for how easy it is to get the API wrong, not for LoC. If we don't have a clean way to implement this without driver changes let's add the explicit API to get the default value. If driver doesn't call it schedule a work to go via the callback once devl_lock() is dropped. That way drivers which care can optimize themselves by reading the default value upfront. Drivers which don't care will work correctly, and there's no API call order trap. Not ideal, but isn't that best we can do here? I still have flashbacks of the fallout from the call ordering games, we have too many drivers to keep this straight... > So if the objection is to the commit message wording, I can fix that and drop > the "notification barrier" language. > > For unregister, I can probably leave the old ordering as-is. I moved it only > to mirror the register path, which felt cleaner, but it is not required for > the default-mode change and as the lock is held I see no issue with doing > that.