Re: [PATCH v7] eal: add bus cleanup to eal cleanup

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Jun 07, 2022 at 05:12:02PM +0200, David Marchand wrote:
> On Tue, Jun 7, 2022 at 1:09 PM Thomas Monjalon <thomas@monjalon.net> 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