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=-4.1 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 64B4EC43467 for ; Fri, 9 Oct 2020 11:40:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 19BB7222B8 for ; Fri, 9 Oct 2020 11:40:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602243655; bh=djPO21ZI0rtD0x7eWZ96qPf9XDpj0bJs7WKf+/73JqY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=ZiNmdi4LZWP5aNv858U5yUnvCmUegxdZY312usiftEK45LsfUR0kWD1E8XNbQ7IMx VS11bBzbIxxbMihI3Cuwpt/3xd2RZz+ds3ef/2esHHLO1+EmKuT8VsYjj4JHI3VEgP ioTOOD4GeY4FBmhDgXZRf/fnm4oEUeSGOhj38xkw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388162AbgJILky (ORCPT ); Fri, 9 Oct 2020 07:40:54 -0400 Received: from mail.kernel.org ([198.145.29.99]:54004 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388152AbgJILkx (ORCPT ); Fri, 9 Oct 2020 07:40:53 -0400 Received: from localhost (unknown [213.57.247.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B0C5D22261; Fri, 9 Oct 2020 11:40:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602243651; bh=djPO21ZI0rtD0x7eWZ96qPf9XDpj0bJs7WKf+/73JqY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HDcHBEhXf9UNRMliUnYR5v/8aiRm+ZG2p1kwmx+R6ybNi/ZkFjm3mN6dMyfcIU4+S uKEcBdce3uL7cTIvO5tRMdnzsiEFSfeSNdI4A0YFG1/wIwR0gRYqZZILGkHBdIeBkQ 7dXXGUweVqCk/CE2Geya+U0+J54NbsDjvcRzMtW0= Date: Fri, 9 Oct 2020 14:40:47 +0300 From: Leon Romanovsky To: Pierre-Louis Bossart Cc: Parav Pandit , "Ertman, David M" , "alsa-devel@alsa-project.org" , "parav@mellanox.com" , "tiwai@suse.de" , "netdev@vger.kernel.org" , "ranjani.sridharan@linux.intel.com" , "fred.oh@linux.intel.com" , "linux-rdma@vger.kernel.org" , "dledford@redhat.com" , "broonie@kernel.org" , Jason Gunthorpe , "gregkh@linuxfoundation.org" , "kuba@kernel.org" , "Williams, Dan J" , "Saleem, Shiraz" , "davem@davemloft.net" , "Patil, Kiran" Subject: Re: [PATCH v2 1/6] Add ancillary bus support Message-ID: <20201009114047.GQ13580@unreal> References: <20201006170241.GM1874917@unreal> <20201007192610.GD3964015@unreal> <1e2a38ac-e259-f955-07ad-602431ad354b@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1e2a38ac-e259-f955-07ad-602431ad354b@linux.intel.com> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Oct 08, 2020 at 08:29:00AM -0500, Pierre-Louis Bossart wrote: > > > > > > > But ... since the init() function is performing both device_init and > > > > > > device_add - it should probably be called ancillary_device_register, > > > > > > and we are back to a single exported API for both register and > > > > > > unregister. > > > > > > > > > > Kind reminder that we introduced the two functions to allow the > > > > > caller to know if it needed to free memory when initialize() fails, > > > > > and it didn't need to free memory when add() failed since > > > > > put_device() takes care of it. If you have a single init() function > > > > > it's impossible to know which behavior to select on error. > > > > > > > > > > I also have a case with SoundWire where it's nice to first > > > > > initialize, then set some data and then add. > > > > > > > > > > > > > The flow as outlined by Parav above does an initialize as the first > > > > step, so every error path out of the function has to do a > > > > put_device(), so you would never need to manually free the memory in > > > the setup function. > > > > It would be freed in the release call. > > > > > > err = ancillary_device_initialize(); > > > if (err) > > > return ret; > > > > > > where is the put_device() here? if the release function does any sort of > > > kfree, then you'd need to do it manually in this case. > > Since device_initialize() failed, put_device() cannot be done here. > > So yes, pseudo code should have shown, > > if (err) { > > kfree(adev); > > return err; > > } > > This doesn't work if the adev is part of a larger structure allocated by the > parent, which is pretty much the intent to extent the basic bus and pass > additional information which can be accessed with container_of(). Please take a look how ib_alloc_device() is implemented. It does all that you wrote above in very similar manner to netdev_alloc. In a nutshell, ib_alloc_device receives needed size from the user and requires from the users to extend their structures below "general" one. Thanks