From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 851BEC433EF for ; Tue, 21 Sep 2021 12:40:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6659A61100 for ; Tue, 21 Sep 2021 12:40:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232656AbhIUMl2 (ORCPT ); Tue, 21 Sep 2021 08:41:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:50200 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230052AbhIUMl1 (ORCPT ); Tue, 21 Sep 2021 08:41:27 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id D5F6E60EE4; Tue, 21 Sep 2021 12:39:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1632227999; bh=dX+cwcBtXlRU2Cy6ZCzeR+J9bsOcN6ZcV8RsSQI80Tc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=rJfygDD0FiSpM1F9bZSz6k8lJNpdXjVV2EP4sEnWe2jlXIdydFW86nrMr7EMBdLn0 VWUFpgURddaCHdZSmTuyThkPIKs4v5M3/nSjv2NtgJ0Q2EisX/IzpbfOdYoNQhXlGP BvboNEPiV/Cl74u4hOvdBQUUujkkjhscfoMU9uBCmmovD2/Aa4jQoZzDodvGtqvFNN Sbb7MOM33wcHSi77wrnJBq7vXX/ETS/jqTbRsLrSh1zb46dQNXL//zQ7L2USoT4Iew dXNVdc83dt+NKtv3XktQnVk5aa3olkrpsrqy7sojj9EGjavU0zRo2xS9BcfsclKPMR 9zlABopkzywKA== Date: Tue, 21 Sep 2021 05:39:56 -0700 From: Jakub Kicinski To: Leon Romanovsky Cc: "David S . Miller" , Alexandre Belloni , Andrew Lunn , Ariel Elior , Bin Luo , Claudiu Manoil , Coiby Xu , Derek Chickles , drivers@pensando.io, Felix Manlunas , Florian Fainelli , Geetha sowjanya , Greg Kroah-Hartman , GR-everest-linux-l2@marvell.com, GR-Linux-NIC-Dev@marvell.com, hariprasad , Ido Schimmel , intel-wired-lan@lists.osuosl.org, Ioana Ciornei , Jerin Jacob , Jesse Brandeburg , Jiri Pirko , Jonathan Lemon , Linu Cherian , linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-rdma@vger.kernel.org, linux-staging@lists.linux.dev, Manish Chopra , Michael Chan , netdev@vger.kernel.org, oss-drivers@corigine.com, Richard Cochran , Saeed Mahameed , Salil Mehta , Satanand Burla , Shannon Nelson , Simon Horman , Subbaraya Sundeep , Sunil Goutham , Taras Chornyi , Tariq Toukan , Tony Nguyen , UNGLinuxDriver@microchip.com, Vadym Kochan , Vivien Didelot , Vladimir Oltean , Yisen Zhuang Subject: Re: [PATCH net-next] devlink: Make devlink_register to be void Message-ID: <20210921053956.11ac7156@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: References: <2e089a45e03db31bf451d768fc588c02a2f781e8.1632148852.git.leonro@nvidia.com> <20210920133915.59ddfeef@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> <20210920140407.0732b3d0@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Tue, 21 Sep 2021 05:19:06 +0300 Leon Romanovsky wrote: > On Mon, Sep 20, 2021 at 02:04:07PM -0700, Jakub Kicinski wrote: > > On Mon, 20 Sep 2021 13:39:15 -0700 Jakub Kicinski wrote: > > > On Mon, 20 Sep 2021 17:41:44 +0300 Leon Romanovsky wrote: > [...] > > > > > > Unlike unused functions bringing back error handling may be > > > non-trivial. I'd rather you deferred such cleanups until you're > > > ready to post your full rework and therefore give us some confidence > > > the revert will not be needed. > > > > If you disagree you gotta repost, new devlink_register call got added > > in the meantime. > > This is exactly what I afraid, new devlink API users are added faster > than I can cleanup them. > > For example, let's take a look on newly added ipc_devlink_init(), it is > called conditionally "if (stage == IPC_MEM_EXEC_STAGE_BOOT) {". How can > it be different stage if we are in driver .probe() routine? > > They also introduced devlink_sio.devlink_read_pend and > devlink_sio.read_sem to protect from something that right position of > devlink_register() will fix. I also have serious doubts that their > current protection is correct, once they called to devlink_params_publish() > the user can crash the system, because he can access the parameters before > they initialized their protection. > > So yes, I disagree. We will need to make sure that devlink_register() > can't fail and it will make life easier for everyone (no need to unwind) > while we put that command being last in probe sequence. Remains to be seen if return type makes people follow correct ordering. > If I repost, will you take it? I don't want to waste anyone time if it > is not. Yeah, go for it.