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

[David Marchand <david.marchand@redhat.com>]

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.


-- 
David Marchand