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

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

On Tue, Oct 4, 2022 at 3:08 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
>
> 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.
>
> Currently, in order for device cleanup to take place, applications must
> call the driver-relevant functions to ensure proper cleanup is done before
> the application exits. Since initialization occurs for all devices on the
> bus, not just the devices used by an application, it requires a)
> application awareness of all bus devices that could have been probed on the
> system, and b) code duplication across applications to ensure cleanup is
> performed. An example of this is rte_eth_dev_close() which is commonly used
> across the example applications.
>
> This patch proposes adding bus cleanup to the eal_cleanup() to make EAL's
> init/exit more symmetrical, ensuring all bus devices are cleaned up
> appropriately without the application needing to be aware of all bus types
> that may have been probed during initialization.
>
> Contained in this patch are the changes required to perform cleanup for
> devices on the PCI bus and VDEV bus during eal_cleanup(). There would be an
> ask for bus maintainers to add the relevant cleanup for their buses since
> they have the domain expertise.
>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
>

Thanks for the rebase.
Most of it lgtm, just one question/comment.

[snip]

> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index a1bb5363b1..b9a7792c19 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -896,6 +896,7 @@ rte_eal_cleanup(void)
>         rte_mp_channel_cleanup();
>         rte_trace_save();
>         eal_trace_fini();
> +       eal_bus_cleanup();
>         /* after this point, any DPDK pointers will become dangling */
>         rte_eal_memory_detach();
>         rte_eal_alarm_cleanup();

Do you have a reason to put the bus cleanup after the traces are
stored and the trace subsystem is uninitialised?

With the current location for eal_bus_cleanup(), it means that this
function (and any code it calls) is not traceable.
To be fair, I don't think we have any trace points in this code at the
moment, but we might have in the future.


-- 
David Marchand