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 EB99BA04A9; Wed, 9 Feb 2022 12:04:45 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BC81841144; Wed, 9 Feb 2022 12:04:45 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 476B541143 for ; Wed, 9 Feb 2022 12:04:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1644404683; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YPx+lFEs0rriKNOxdOLxWebXBxiaptGg4JRWdOvdktY=; b=eebHLZRhHLgGK7wQpKUf0wZ7rD02d8yi3SY0rItP21GzeNzvVwT50KsdQmsU7DMA/mLGwo E2e7x53YF4AmgPbBpnyKSJo5sKydcVRc83lijJfLTzV98N5aWUl3VDM2sUIxhBTjX/yjoQ IySwnmHsyHr4bbSJ45OUrB6qe4scB3U= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-220-R_1jYf0QMS2v3_NYxMQXWw-1; Wed, 09 Feb 2022 06:04:42 -0500 X-MC-Unique: R_1jYf0QMS2v3_NYxMQXWw-1 Received: by mail-lf1-f72.google.com with SMTP id 27-20020ac25f1b000000b0043edb7bf7e7so419572lfq.20 for ; Wed, 09 Feb 2022 03:04:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=YPx+lFEs0rriKNOxdOLxWebXBxiaptGg4JRWdOvdktY=; b=K80VO4kxxJWYhfYWJl8l9aoCbvghRHHIWVwB4TKLw2e4JLyyZD/AZka/vPMhgfBTuf PkLAcs0O0pTaevb9o32x/SP3XyAffb6g/eycdbsuoYnqppptxf91odw/WIdudPjqhjCg VzC4U3V9k2AKytmzxtKXo0Yf73Y/IXV6Sp+looeYUeWOGLEYQbXrGJWTi728uJChA4hI jI2EIM3v0qnxz/6cjWskixTtNJCTKcWMYNiRfulolavKtTjz+Y8Ujg+rp7Bl4UT2vGyl xuIiB5k+uQaxmwNRiu989pSe7N7G8tdfJ0uq5qm0XMKSzdviq8Fzvh6y3I5Z2EbP7yQr PnGQ== X-Gm-Message-State: AOAM531VH8iUstZ2eH8la2Jt+Ybysxx9/1OI1xXqIOQVMyowfhi6r7GX WLcBGmIbUBoy8JAncUl8oIHEbtLSC3ktsbrE1Y6TFinoVwxeb2rHLIetQZeNEdzFU9UyaPRTc5N dxRjB6xB9UZH3c+ByzpY= X-Received: by 2002:a2e:5848:: with SMTP id x8mr1138608ljd.297.1644404680867; Wed, 09 Feb 2022 03:04:40 -0800 (PST) X-Google-Smtp-Source: ABdhPJyYfnA/IO0/4OHtf6DPNa08qToUY+4iL6Qq+A/1eS2eht0ZOfoP6+fy2EnnMyU5z9JDkJh3WHbR1WPybTGj6P4= X-Received: by 2002:a2e:5848:: with SMTP id x8mr1138577ljd.297.1644404680173; Wed, 09 Feb 2022 03:04:40 -0800 (PST) MIME-Version: 1.0 References: <20201008153048.19369-1-rohit.raj@nxp.com> <20220110052627.22577-1-rohit.raj@nxp.com> In-Reply-To: <20220110052627.22577-1-rohit.raj@nxp.com> From: David Marchand Date: Wed, 9 Feb 2022 12:04:28 +0100 Message-ID: Subject: Re: [PATCH v5 1/2] eal: add API for bus close To: Rohit Raj Cc: Bruce Richardson , Ray Kinsella , Dmitry Kozlyuk , Narcisa Ana Maria Vasile , Dmitry Malloy , Pallavi Kadam , dev , Nipun Gupta , Sachin Saxena , Hemant Agrawal Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" 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 Mon, Jan 10, 2022 at 6:26 AM wrote: > > From: Rohit Raj > > As per the current code we have API for bus probe, but the > bus close API is missing. This breaks the multi process > scenarios as objects are not cleaned while terminating the > secondary processes. After an application crash, how does this bus resets the associated resources? > > This patch adds a new API rte_bus_close() for cleanup of > bus objects which were acquired during probe. The patch in its current form breaks the ABI on rte_bus object. This can be seen in GHA, or calling DPDK_ABI_REF_VERSION=v21.11 ./devtools/test-meson-builds.sh. [snip] > diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst > index 6d99d1eaa9..7417606a2a 100644 > --- a/doc/guides/rel_notes/release_22_03.rst > +++ b/doc/guides/rel_notes/release_22_03.rst > @@ -55,6 +55,11 @@ New Features > Also, make sure to start the actual text at the margin. > ======================================================= > > + * **Added support to close bus.** > + > + Added capability to allow a user to do cleanup of bus objects which > + were acquired during bus probe. > + Wrongly indented. > > Removed Items > ------------- > @@ -84,6 +89,9 @@ API Changes > Also, make sure to start the actual text at the margin. > ======================================================= > > + * eal: Added new API ``rte_bus_close`` to perform cleanup bus objects which > + were acquired during bus probe. > + > > ABI Changes > ----------- > diff --git a/lib/eal/common/eal_common_bus.c b/lib/eal/common/eal_common_bus.c > index baa5b532af..2c3c0a90d2 100644 > --- a/lib/eal/common/eal_common_bus.c > +++ b/lib/eal/common/eal_common_bus.c > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright 2016 NXP > + * Copyright 2016,2022 NXP > */ > > #include > @@ -85,6 +85,37 @@ rte_bus_probe(void) > return 0; > } > > +/* Close all devices of all buses */ > +int > +rte_bus_close(void) > +{ > + int ret; > + struct rte_bus *bus, *vbus = NULL; > + > + TAILQ_FOREACH(bus, &rte_bus_list, next) { > + if (!strcmp(bus->name, "vdev")) { > + vbus = bus; > + continue; > + } > + > + if (bus->close) { > + ret = bus->close(); > + if (ret) > + RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n", > + bus->name); > + } > + } > + > + if (vbus && vbus->close) { > + ret = vbus->close(); > + if (ret) > + RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n", > + vbus->name); > + } The vdev bus is special in that some drivers can reference objects from other buses (see f4ce209a8ce5 ("eal: postpone vdev initialization") and da76cc02342b ("eal: probe new virtual bus after other bus devices")). For this reason, I would expect that the vdev bus is closed before the other buses. > + > + return 0; > +} > + > /* Dump information of a single bus */ > static int > bus_dump_one(FILE *f, struct rte_bus *bus) > diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c > index a1cd2462db..87d70c6898 100644 > --- a/lib/eal/freebsd/eal.c > +++ b/lib/eal/freebsd/eal.c > @@ -984,6 +984,7 @@ rte_eal_cleanup(void) > { > struct internal_config *internal_conf = > eal_get_internal_configuration(); > + rte_bus_close(); > rte_service_finalize(); > rte_mp_channel_cleanup(); > /* after this point, any DPDK pointers will become dangling */ > diff --git a/lib/eal/include/rte_bus.h b/lib/eal/include/rte_bus.h > index bbbb6efd28..c6211bbd95 100644 > --- a/lib/eal/include/rte_bus.h > +++ b/lib/eal/include/rte_bus.h > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright 2016 NXP > + * Copyright 2016,2022 NXP > */ > > #ifndef _RTE_BUS_H_ > @@ -66,6 +66,23 @@ typedef int (*rte_bus_scan_t)(void); > */ > typedef int (*rte_bus_probe_t)(void); > > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice > + * > + * Implementation specific close function which is responsible for resetting all > + * detected devices on the bus to a default state, closing UIO nodes or VFIO > + * groups and also freeing any memory allocated during rte_bus_probe like > + * private resources for device list. > + * > + * This is called while iterating over each registered bus. > + * > + * @return > + * 0 for successful close > + * !0 for any error while closing > + */ > +typedef int (*rte_bus_close_t)(void); > + > /** > * Device iterator to find a device on a bus. > * > @@ -263,6 +280,7 @@ struct rte_bus { > const char *name; /**< Name of the bus */ > rte_bus_scan_t scan; /**< Scan for devices attached to bus */ > rte_bus_probe_t probe; /**< Probe devices on bus */ > + rte_bus_close_t close; /**< Close devices on bus */ > rte_bus_find_device_t find_device; /**< Find a device on the bus */ > rte_bus_plug_t plug; /**< Probe single device for drivers */ > rte_bus_unplug_t unplug; /**< Remove single device from driver */ > @@ -317,6 +335,16 @@ int rte_bus_scan(void); > */ > int rte_bus_probe(void); > > +/** > + * For each device on the buses, call the device specific close. > + * > + * @return > + * 0 for successful close > + * !0 otherwise > + */ > +__rte_experimental > +int rte_bus_close(void); > + > /** > * Dump information of all the buses registered with EAL. > * > diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c > index 60b4924838..5c60131e46 100644 > --- a/lib/eal/linux/eal.c > +++ b/lib/eal/linux/eal.c > @@ -1362,6 +1362,14 @@ rte_eal_cleanup(void) > > if (rte_eal_process_type() == RTE_PROC_PRIMARY) > rte_memseg_walk(mark_freeable, NULL); > + > + /* Close all the buses and devices/drivers on them */ > + if (rte_bus_close()) { > + rte_eal_init_alert("Cannot close devices"); You can't call rte_eal_*init*_alert in rte_eal_cleanup. There is not much to do if the bus close fails, I'd rather leave the cleanup continue. > + rte_errno = ENOTSUP; > + return -1; > + } > + > rte_service_finalize(); > rte_mp_channel_cleanup(); > /* after this point, any DPDK pointers will become dangling */ > diff --git a/lib/eal/version.map b/lib/eal/version.map > index ab28c22791..39882dbbd5 100644 > --- a/lib/eal/version.map > +++ b/lib/eal/version.map > @@ -420,6 +420,9 @@ EXPERIMENTAL { > rte_intr_instance_free; > rte_intr_type_get; > rte_intr_type_set; > + > + # added in 22.03 > + rte_bus_close; > }; > > INTERNAL { > diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c > index 67db7f099a..5915ab6291 100644 > --- a/lib/eal/windows/eal.c > +++ b/lib/eal/windows/eal.c > @@ -260,6 +260,7 @@ rte_eal_cleanup(void) > struct internal_config *internal_conf = > eal_get_internal_configuration(); > > + rte_bus_close(); > eal_intr_thread_cancel(); > eal_mem_virt2iova_cleanup(); > /* after this point, any DPDK pointers will become dangling */ > -- > 2.17.1 > -- David Marchand