diff mbox series

[v3,1/3] eal: add API for bus close

Message ID 20200826055233.26075-1-rohit.raj@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v3,1/3] eal: add API for bus close |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Rohit Raj Aug. 26, 2020, 5:52 a.m. UTC 
  From: Rohit Raj <rohit.raj@nxp.com>

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.

This patch adds a new API rte_bus_close() for cleanup of
bus objects which were acquired during probe.

Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
---

v3:
* nit: combined nested if statements

v2:
* Moved rte_bus_close call to rte_eal_cleanup path.

 lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
 lib/librte_eal/include/rte_bus.h       | 25 +++++++++++++++++++-
 lib/librte_eal/linux/eal.c             |  1 +
 lib/librte_eal/rte_eal_version.map     |  3 +++
 4 files changed, 59 insertions(+), 2 deletions(-)

Comments

Hemant Agrawal Sept. 8, 2020, 4:46 a.m. UTC | #1 
-----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of rohit.raj@nxp.com
> Sent: Wednesday, August 26, 2020 11:23 AM
> To: Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> Cc: dev@dpdk.org; Rohit Raj <rohit.raj@nxp.com>
> Subject: [dpdk-dev] [PATCH v3 1/3] eal: add API for bus close
> 
> From: Rohit Raj <rohit.raj@nxp.com>
> 
> 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.
> 
> This patch adds a new API rte_bus_close() for cleanup of bus objects which
> were acquired during probe.
> 
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Ray Kinsella Sept. 17, 2020, 3:34 p.m. UTC | #2 
On 26/08/2020 06:52, rohit.raj@nxp.com wrote:
> From: Rohit Raj <rohit.raj@nxp.com>
> 
> 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.
> 
> This patch adds a new API rte_bus_close() for cleanup of
> bus objects which were acquired during probe.
> 
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
> ---
> 
> v3:
> * nit: combined nested if statements
> 
> v2:
> * Moved rte_bus_close call to rte_eal_cleanup path.
> 
>  lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
>  lib/librte_eal/include/rte_bus.h       | 25 +++++++++++++++++++-
>  lib/librte_eal/linux/eal.c             |  1 +
>  lib/librte_eal/rte_eal_version.map     |  3 +++
>  4 files changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..5fd7cf6c5 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>   */
>  
>  #include <stdio.h>
> @@ -56,6 +56,36 @@ rte_bus_scan(void)
>  	return 0;
>  }
>  
> +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);
> +	}
> +
> +	return 0;
> +}
> +
>  /* Probe all devices of all buses */
>  int
>  rte_bus_probe(void)
> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..af4787b18 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>   */
>  
>  #ifndef _RTE_BUS_H_
> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>   */
>  typedef int (*rte_bus_probe_t)(void);
>  
> +/**
> + * Implementation specific close function which is responsible for closing
> + * devices on that bus.
> + *
> + * 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.
>   *
> @@ -248,6 +260,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 */
> @@ -301,6 +314,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/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index 0960f01d0..a8a63d8ca 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1357,6 +1357,7 @@ rte_eal_cleanup(void)
>  
>  	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>  		rte_memseg_walk(mark_freeable, NULL);
> +	rte_bus_close();
>  	rte_service_finalize();
>  	rte_mp_channel_cleanup();
>  	rte_trace_save();
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 0b18e2ef8..dc5266f22 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,9 @@ EXPERIMENTAL {
>  	rte_mp_disable;
>  	rte_thread_register;
>  	rte_thread_unregister;
> +
> +	# added in 20.11
> +	rte_bus_close;
>  };
>  
>  INTERNAL {
> 

Putting EXPERITMENAL on such a trivial API feels like overkill, however ...

Acked-by: Ray Kinsella <mdr@ashroe.eu>
Thomas Monjalon Sept. 23, 2020, 11:54 p.m. UTC | #3 
> > Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Please could you look urgently to the close of a port?
All NXP drivers are late to migrate to the new behaviour:
http://inbox.dpdk.org/dev/20200913220711.3768597-17-thomas@monjalon.net/
http://inbox.dpdk.org/dev/80837131.n3tlCtGQ32@thomas/
Ferruh Yigit Sept. 24, 2020, 11:39 a.m. UTC | #4 
On 8/26/2020 6:52 AM, rohit.raj@nxp.com wrote:
> From: Rohit Raj <rohit.raj@nxp.com>
> 
> 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.
> 
> This patch adds a new API rte_bus_close() for cleanup of
> bus objects which were acquired during probe.
> 
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>

<...>

> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>    */
>   typedef int (*rte_bus_probe_t)(void);
>   
> +/**
> + * Implementation specific close function which is responsible for closing
> + * devices on that bus.
> + *
> + * 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);
> +

Can you please put more comments to clarify what does "closing devices 
on bus" mean, what is the responsibility of that function, what is the 
expected state after that function, etc ?

Since 'pci' & 'vdev' bus implementations are missing, this comment 
should help them to clarify what exactly needs to be implemented for 
those buses.
Sachin Saxena (OSS) Sept. 29, 2020, 4:33 a.m. UTC | #5 
On 26-Aug-20 11:22 AM, rohit.raj@nxp.com wrote:
> From: Rohit Raj <rohit.raj@nxp.com>
>
> 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.
>
> This patch adds a new API rte_bus_close() for cleanup of
> bus objects which were acquired during probe.
>
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
> ---
>
> v3:
> * nit: combined nested if statements
>
> v2:
> * Moved rte_bus_close call to rte_eal_cleanup path.
>
>   lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
>   lib/librte_eal/include/rte_bus.h       | 25 +++++++++++++++++++-
>   lib/librte_eal/linux/eal.c             |  1 +
>   lib/librte_eal/rte_eal_version.map     |  3 +++
>   4 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..5fd7cf6c5 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>    */
>   
>   #include <stdio.h>
> @@ -56,6 +56,36 @@ rte_bus_scan(void)
>   	return 0;
>   }
>   
> +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);
> +	}
> +
> +	return 0;
> +}
> +
>   /* Probe all devices of all buses */
>   int
>   rte_bus_probe(void)
> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..af4787b18 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>    */
>   
>   #ifndef _RTE_BUS_H_
> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>    */
>   typedef int (*rte_bus_probe_t)(void);
>   
> +/**
> + * Implementation specific close function which is responsible for closing
> + * devices on that bus.
> + *
> + * 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.
>    *
> @@ -248,6 +260,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 */
> @@ -301,6 +314,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/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index 0960f01d0..a8a63d8ca 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1357,6 +1357,7 @@ rte_eal_cleanup(void)
>   
>   	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>   		rte_memseg_walk(mark_freeable, NULL);
> +	rte_bus_close();
Since this is common framework change, we should introduce similar call 
for "rte_bus_close" in FreeBSD and Windows version also.

>   	rte_service_finalize();
>   	rte_mp_channel_cleanup();
>   	rte_trace_save();
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 0b18e2ef8..dc5266f22 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,9 @@ EXPERIMENTAL {
>   	rte_mp_disable;
>   	rte_thread_register;
>   	rte_thread_unregister;
> +
> +	# added in 20.11
> +	rte_bus_close;
>   };
>   
>   INTERNAL {
Ferruh Yigit Sept. 30, 2020, 11:50 a.m. UTC | #6 
On 8/26/2020 6:52 AM, rohit.raj@nxp.com wrote:
> From: Rohit Raj <rohit.raj@nxp.com>
> 
> 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.
> 
> This patch adds a new API rte_bus_close() for cleanup of
> bus objects which were acquired during probe.
> 
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
> ---
> 
> v3:
> * nit: combined nested if statements
> 
> v2:
> * Moved rte_bus_close call to rte_eal_cleanup path.
> 
>   lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
>   lib/librte_eal/include/rte_bus.h       | 25 +++++++++++++++++++-
>   lib/librte_eal/linux/eal.c             |  1 +
>   lib/librte_eal/rte_eal_version.map     |  3 +++
>   4 files changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..5fd7cf6c5 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>    */
>   
>   #include <stdio.h>
> @@ -56,6 +56,36 @@ rte_bus_scan(void)
>   	return 0;
>   }
>   
> +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;
> +		}

This special treatment for 'vdev' bus is done in probe to be sure physically 
device port ids start from '0',  I guess we don't need to do this for 'close'.

> +
> +		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);
> +	}
> +
> +	return 0;
> +}
> +
>   /* Probe all devices of all buses */
>   int
>   rte_bus_probe(void)
> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..af4787b18 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>    */
>   
>   #ifndef _RTE_BUS_H_
> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>    */
>   typedef int (*rte_bus_probe_t)(void);
>   
> +/**
> + * Implementation specific close function which is responsible for closing
> + * devices on that bus.
> + *
> + * 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);
> +

As I checked the 'rte_fslmc_bus->close()' ops, it iterates on all devices in the 
bus instead of doing a bus level close, in that case
instead of adding a new 'close' bus operations, will it work if existing 
'bus->unplug(dev)' used?
Whatever done in the 'rte_fslmc_bus->close()' per device, can it be done under 
the 'fslmc_bus_unplug()'?

And in that case a 'rte_bus_remove()' API can be added which can call 
'bus->unplug(dev)' for all buses and it will be beneficial for all buses, and it 
can fit well into the 'rte_eal_cleanup()'.

What do you think?
Ferruh Yigit Oct. 8, 2020, 4:24 p.m. UTC | #7 
On 9/30/2020 12:50 PM, Ferruh Yigit wrote:
> On 8/26/2020 6:52 AM, rohit.raj@nxp.com wrote:
>> From: Rohit Raj <rohit.raj@nxp.com>
>>
>> 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.
>>
>> This patch adds a new API rte_bus_close() for cleanup of
>> bus objects which were acquired during probe.
>>
>> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
>> ---
>>
>> v3:
>> * nit: combined nested if statements
>>
>> v2:
>> * Moved rte_bus_close call to rte_eal_cleanup path.
>>
>>   lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
>>   lib/librte_eal/include/rte_bus.h       | 25 +++++++++++++++++++-
>>   lib/librte_eal/linux/eal.c             |  1 +
>>   lib/librte_eal/rte_eal_version.map     |  3 +++
>>   4 files changed, 59 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_bus.c 
>> b/lib/librte_eal/common/eal_common_bus.c
>> index baa5b532a..5fd7cf6c5 100644
>> --- a/lib/librte_eal/common/eal_common_bus.c
>> +++ b/lib/librte_eal/common/eal_common_bus.c
>> @@ -1,5 +1,5 @@
>>   /* SPDX-License-Identifier: BSD-3-Clause
>> - * Copyright 2016 NXP
>> + * Copyright 2016,2020 NXP
>>    */
>>   #include <stdio.h>
>> @@ -56,6 +56,36 @@ rte_bus_scan(void)
>>       return 0;
>>   }
>> +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;
>> +        }
> 
> This special treatment for 'vdev' bus is done in probe to be sure physically 
> device port ids start from '0',  I guess we don't need to do this for 'close'.
> 
>> +
>> +        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);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   /* Probe all devices of all buses */
>>   int
>>   rte_bus_probe(void)
>> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
>> index d3034d0ed..af4787b18 100644
>> --- a/lib/librte_eal/include/rte_bus.h
>> +++ b/lib/librte_eal/include/rte_bus.h
>> @@ -1,5 +1,5 @@
>>   /* SPDX-License-Identifier: BSD-3-Clause
>> - * Copyright 2016 NXP
>> + * Copyright 2016,2020 NXP
>>    */
>>   #ifndef _RTE_BUS_H_
>> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>>    */
>>   typedef int (*rte_bus_probe_t)(void);
>> +/**
>> + * Implementation specific close function which is responsible for closing
>> + * devices on that bus.
>> + *
>> + * 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);
>> +
> 
> As I checked the 'rte_fslmc_bus->close()' ops, it iterates on all devices in the 
> bus instead of doing a bus level close, in that case
> instead of adding a new 'close' bus operations, will it work if existing 
> 'bus->unplug(dev)' used?
> Whatever done in the 'rte_fslmc_bus->close()' per device, can it be done under 
> the 'fslmc_bus_unplug()'?
> 
> And in that case a 'rte_bus_remove()' API can be added which can call 
> 'bus->unplug(dev)' for all buses and it will be beneficial for all buses, and it 
> can fit well into the 'rte_eal_cleanup()'.
> 
> What do you think?

Hi Rohit,

I have seen new version has been sent today, I want to remind above question, 
can you please check it?
Rohit Raj Oct. 9, 2020, 4:53 a.m. UTC | #8 
Hi Ferruh,

Sorry, I missed you comment last time. I will send an updated patch series after taking care of your suggested changes.

Regards,
Rohit Raj
Software Engineer | NXP India Private Limited

Sent from Outlook Mobile
David Marchand Oct. 18, 2020, 9:25 a.m. UTC | #9 
Hello Rohit,

On Fri, Oct 9, 2020 at 6:53 AM Rohit Raj <rohit.raj@nxp.com> wrote:
> Sorry, I missed you comment last time. I will send an updated patch series after taking care of your suggested changes.

Added few more comments.
Waiting for a respin.

Thanks.
diff mbox series

Patch

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index baa5b532a..5fd7cf6c5 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2016 NXP
+ * Copyright 2016,2020 NXP
  */
 
 #include <stdio.h>
@@ -56,6 +56,36 @@  rte_bus_scan(void)
 	return 0;
 }
 
+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);
+	}
+
+	return 0;
+}
+
 /* Probe all devices of all buses */
 int
 rte_bus_probe(void)
diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
index d3034d0ed..af4787b18 100644
--- a/lib/librte_eal/include/rte_bus.h
+++ b/lib/librte_eal/include/rte_bus.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2016 NXP
+ * Copyright 2016,2020 NXP
  */
 
 #ifndef _RTE_BUS_H_
@@ -67,6 +67,18 @@  typedef int (*rte_bus_scan_t)(void);
  */
 typedef int (*rte_bus_probe_t)(void);
 
+/**
+ * Implementation specific close function which is responsible for closing
+ * devices on that bus.
+ *
+ * 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.
  *
@@ -248,6 +260,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 */
@@ -301,6 +314,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/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 0960f01d0..a8a63d8ca 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1357,6 +1357,7 @@  rte_eal_cleanup(void)
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+	rte_bus_close();
 	rte_service_finalize();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 0b18e2ef8..dc5266f22 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -397,6 +397,9 @@  EXPERIMENTAL {
 	rte_mp_disable;
 	rte_thread_register;
 	rte_thread_unregister;
+
+	# added in 20.11
+	rte_bus_close;
 };
 
 INTERNAL {