From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id CED87A0543; Mon, 13 Jun 2022 17:59:03 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BE6B640222; Mon, 13 Jun 2022 17:59:03 +0200 (CEST) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id C6A8140150 for ; Mon, 13 Jun 2022 17:59:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1655135943; x=1686671943; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=prdZST9H09164+vdhh3Yp1igslpmPLwZcxDQ6a5shy4=; b=bV+yaw1zrZFRPwz7MzR2CSKDmc39+Q5fIk/YaW+lmGaw4qIJjChapLyx VcsAgQgD6VI63hDTSt3sMmU2aYcJlETewih2GZphnDpVn96nzOVL4tZDJ d+M9/F0fGkOhmyYc1tu75fSQDpgI9C1kVeWcNgGf/SNwPoZ1q1fnYGwod CSxdwAHCaZfYGKlwtLHtDhhJsNq5gRjEnQE5bwjTpD2h8e2uQ3M4IfZC2 A1u2SBpYpZswGF1dc9ydi3F1/gJfH6LBQLYHZuZ9+8w3dBoCmJ5TuPNR/ s4VB5CXIAzkAq7vOSqBArIRwz5hV9SWcZSxO3fPISYaiLi5RDWn1k5WoV A==; X-IronPort-AV: E=McAfee;i="6400,9594,10377"; a="339997791" X-IronPort-AV: E=Sophos;i="5.91,297,1647327600"; d="scan'208";a="339997791" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jun 2022 08:59:01 -0700 X-IronPort-AV: E=Sophos;i="5.91,297,1647327600"; d="scan'208";a="726333568" Received: from bricha3-mobl.ger.corp.intel.com ([10.55.133.106]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 13 Jun 2022 08:58:58 -0700 Date: Mon, 13 Jun 2022 16:58:54 +0100 From: Bruce Richardson To: David Marchand Cc: Thomas Monjalon , Kevin Laatz , dev , Morten =?iso-8859-1?Q?Br=F8rup?= , Li Zhang , Matan Azrad , Stephen Hemminger , lihuisong Subject: Re: [PATCH v7] eal: add bus cleanup to eal cleanup Message-ID: References: <20220419161438.1837860-1-kevin.laatz@intel.com> <20220603143601.230519-1-kevin.laatz@intel.com> <7465552.R56niFO833@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Jun 07, 2022 at 05:12:02PM +0200, David Marchand wrote: > On Tue, Jun 7, 2022 at 1:09 PM Thomas Monjalon wrote: > > > > 03/06/2022 16:36, Kevin Laatz: > > > During EAL init, all buses are probed and the devices found are > > > initialized. On eal_cleanup(), the inverse does not happen, meaning any > > > allocated memory and other configuration will not be cleaned up > > > appropriately on exit. > > [...] > > > --- a/devtools/libabigail.abignore > > > +++ b/devtools/libabigail.abignore > > > @@ -56,3 +56,12 @@ > > > ; Ignore libabigail false-positive in clang builds, after moving code. > > > [suppress_function] > > > name = rte_eal_remote_launch > > > + > > > +; Ignore field inserted to rte_bus, adding cleanup function > > > +[suppress_type] > > > + name = rte_bus > > > + has_data_member_inserted_at = end > > > + > > > +; Ignore changes to internally used structs containing rte_bus > > > +[suppress_type] > > > + name = rte_pci_bus, rte_vmbus_bus, rte_vdev_bus > > (This change is strange as there is no rte_vdev_bus type, but I won't > investigate the relevance of this rule for now). > > > > > I'm not sure we can safely consider these structs as internal. > > The right process is to send a deprecation notice, > > and then remove them from the public API. > > Same for me, I don't think we can safely ignore. > > A rte_bus struct is embedded in rte_pci_bus (resp. rte_vmbus_bus). > If we make it grow, any inlined access (like walk in device_list or > driver_list) after the rte_bus object is broken for code accessing it > out of DPDK. > Such code might exist out there, since we expose > FOREACH_DEVICE_ON_PCIBUS, for example. > > > > > > For info, Li has sent a patch for the bus cleanup > > which is not updating the bus code: > > https://patches.dpdk.org/project/dpdk/patch/20220606114650.209612-3-lizh@nvidia.com/ > > It may be a temporary solution before the deprecation. > > On the principle, that's probably the best, there is no question about > unclear frontier of the ABI. > (In practice though, the mentionned patch is triggering segfaults in > two CI, for pdump). > > Hiding rte_bus object should be straightforward in v22.11, I had some > patches, but never finished the work. > > It would be great too, to look into rte_driver and rte_device which > are exposed important types, but that's another story. > Agreed, we need to look into all this for 22.11 release, let's defer this patch until we get proper deprecation process. Temporary patch looks fine as a fix too. /Bruce