[v11,6/7] eal: add failure handle mechanism for hot-unplug

Message ID 1538307003-11836-7-git-send-email-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v11,1/7] bus: add hot-unplug handler |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Guo, Jia Sept. 30, 2018, 11:30 a.m. UTC
  The mechanism can initially register the sigbus handler after the device
event monitor is enabled. When a sigbus event is captured, it will check
the failure address and accordingly handle the memory failure of the
corresponding device by invoke the hot-unplug handler. It could prevent
the application from crashing when a device is hot-unplugged.

By this patch, users could call below new added APIs to enable/disable
the device hotplug handle mechanism. Note that it just implement the
hot-unplug handler in these functions, the other handler of hotplug, such
as handler for hotplug binding, could be add in the future if need:
  - rte_dev_hotplug_handle_enable
  - rte_dev_hotplug_handle_disable

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Shaopeng He <shaopeng.he@intel.com>
---
v11->v10:
change some words and change the invoked func name.
add experimental tag
---
 doc/guides/rel_notes/release_18_08.rst  |   5 +
 lib/librte_eal/bsdapp/eal/eal_dev.c     |  14 +++
 lib/librte_eal/common/eal_private.h     |  26 +++++
 lib/librte_eal/common/include/rte_dev.h |  26 +++++
 lib/librte_eal/linuxapp/eal/eal_dev.c   | 162 +++++++++++++++++++++++++++++++-
 lib/librte_eal/rte_eal_version.map      |   2 +
 6 files changed, 234 insertions(+), 1 deletion(-)
  

Comments

Ananyev, Konstantin Sept. 30, 2018, 7:46 p.m. UTC | #1
Hi Jeff,

> 
> The mechanism can initially register the sigbus handler after the device
> event monitor is enabled. When a sigbus event is captured, it will check
> the failure address and accordingly handle the memory failure of the
> corresponding device by invoke the hot-unplug handler. It could prevent
> the application from crashing when a device is hot-unplugged.
> 
> By this patch, users could call below new added APIs to enable/disable
> the device hotplug handle mechanism. Note that it just implement the
> hot-unplug handler in these functions, the other handler of hotplug, such
> as handler for hotplug binding, could be add in the future if need:
>   - rte_dev_hotplug_handle_enable
>   - rte_dev_hotplug_handle_disable
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> Acked-by: Shaopeng He <shaopeng.he@intel.com>
> ---
> v11->v10:
> change some words and change the invoked func name.
> add experimental tag
> ---
>  doc/guides/rel_notes/release_18_08.rst  |   5 +
>  lib/librte_eal/bsdapp/eal/eal_dev.c     |  14 +++
>  lib/librte_eal/common/eal_private.h     |  26 +++++
>  lib/librte_eal/common/include/rte_dev.h |  26 +++++
>  lib/librte_eal/linuxapp/eal/eal_dev.c   | 162 +++++++++++++++++++++++++++++++-
>  lib/librte_eal/rte_eal_version.map      |   2 +
>  6 files changed, 234 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
> index 321fa84..fe0e60f 100644
> --- a/doc/guides/rel_notes/release_18_08.rst
> +++ b/doc/guides/rel_notes/release_18_08.rst
> @@ -117,6 +117,11 @@ New Features
> 
>    Added support for chained mbufs (input and output).
> 
> +* **Added hot-unplug handle mechanism.**
> +
> +  ``rte_dev_hotplug_handle_enable`` and ``rte_dev_hotplug_handle_disable`` are
> +  for enabling or disabling hotplug handle mechanism.
> +
> 
>  API Changes
>  -----------
> diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c b/lib/librte_eal/bsdapp/eal/eal_dev.c
> index 1c6c51b..255d611 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_dev.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
> @@ -19,3 +19,17 @@ rte_dev_event_monitor_stop(void)
>  	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>  	return -1;
>  }
> +
> +int __rte_experimental
> +rte_dev_hotplug_handle_enable(void)
> +{
> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
> +	return -1;
> +}
> +
> +int __rte_experimental
> +rte_dev_hotplug_handle_disable(void)
> +{
> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
> +	return -1;
> +}
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index a2d1528..637f20d 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -317,4 +317,30 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
>   */
>  int rte_bus_sigbus_handler(const void *failure_addr);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Register the sigbus handler.
> + *
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +int
> +rte_dev_sigbus_handler_register(void);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Unregister the sigbus handler.
> + *
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +int
> +rte_dev_sigbus_handler_unregister(void);
> +
>  #endif /* _EAL_PRIVATE_H_ */
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index b80a805..ff580a0 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -460,4 +460,30 @@ rte_dev_event_monitor_start(void);
>  int __rte_experimental
>  rte_dev_event_monitor_stop(void);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Enable hotplug handling for devices.
> + *
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_dev_hotplug_handle_enable(void);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Disable hotplug handling for devices.
> + *
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_dev_hotplug_handle_disable(void);
> +
>  #endif /* _RTE_DEV_H_ */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c
> index 1cf6aeb..14b18d8 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_dev.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
> @@ -4,6 +4,8 @@
> 
>  #include <string.h>
>  #include <unistd.h>
> +#include <fcntl.h>
> +#include <signal.h>
>  #include <sys/socket.h>
>  #include <linux/netlink.h>
> 
> @@ -14,15 +16,32 @@
>  #include <rte_malloc.h>
>  #include <rte_interrupts.h>
>  #include <rte_alarm.h>
> +#include <rte_bus.h>
> +#include <rte_eal.h>
> +#include <rte_spinlock.h>
> +#include <rte_errno.h>
> 
>  #include "eal_private.h"
> 
>  static struct rte_intr_handle intr_handle = {.fd = -1 };
>  static bool monitor_started;
> +static bool hotplug_handle;
> 
>  #define EAL_UEV_MSG_LEN 4096
>  #define EAL_UEV_MSG_ELEM_LEN 128
> 
> +/*
> + * spinlock for device hot-unplug failure handling. If it try to access bus or
> + * device, such as handle sigbus on bus or handle memory failure for device
> + * just need to use this lock. It could protect the bus and the device to avoid
> + * race condition.
> + */
> +static rte_spinlock_t failure_handle_lock = RTE_SPINLOCK_INITIALIZER;
> +
> +static struct sigaction sigbus_action_old;
> +
> +static int sigbus_need_recover;
> +
>  static void dev_uev_handler(__rte_unused void *param);
> 
>  /* identify the system layer which reports this event. */
> @@ -33,6 +52,49 @@ enum eal_dev_event_subsystem {
>  	EAL_DEV_EVENT_SUBSYSTEM_MAX
>  };
> 
> +static void
> +sigbus_action_recover(void)
> +{
> +	if (sigbus_need_recover) {
> +		sigaction(SIGBUS, &sigbus_action_old, NULL);
> +		sigbus_need_recover = 0;
> +	}
> +}
> +
> +static void sigbus_handler(int signum, siginfo_t *info,
> +				void *ctx __rte_unused)
> +{
> +	int ret;
> +
> +	RTE_LOG(INFO, EAL, "Thread[%d] catch SIGBUS, fault address:%p\n",
> +		(int)pthread_self(), info->si_addr);
> +
> +	rte_spinlock_lock(&failure_handle_lock);
> +	ret = rte_bus_sigbus_handler(info->si_addr);
> +	rte_spinlock_unlock(&failure_handle_lock);
> +	if (ret == -1) {
> +		rte_exit(EXIT_FAILURE,
> +			 "Failed to handle SIGBUS for hot-unplug, "
> +			 "(rte_errno: %s)!", strerror(rte_errno));
> +	} else if (ret == 1) {
> +		if (sigbus_action_old.sa_handler)
> +			(*(sigbus_action_old.sa_handler))(signum);
> +		else
> +			rte_exit(EXIT_FAILURE,
> +				 "Failed to handle generic SIGBUS!");
> +	}
> +
> +	RTE_LOG(INFO, EAL, "Success to handle SIGBUS for hot-unplug!\n");
> +}
> +
> +static int cmp_dev_name(const struct rte_device *dev,
> +	const void *_name)
> +{
> +	const char *name = _name;
> +
> +	return strcmp(dev->name, name);
> +}
> +
>  static int
>  dev_uev_socket_fd_create(void)
>  {
> @@ -147,6 +209,9 @@ dev_uev_handler(__rte_unused void *param)
>  	struct rte_dev_event uevent;
>  	int ret;
>  	char buf[EAL_UEV_MSG_LEN];
> +	struct rte_bus *bus;
> +	struct rte_device *dev;
> +	const char *busname = "";
> 
>  	memset(&uevent, 0, sizeof(struct rte_dev_event));
>  	memset(buf, 0, EAL_UEV_MSG_LEN);
> @@ -171,8 +236,43 @@ dev_uev_handler(__rte_unused void *param)
>  	RTE_LOG(DEBUG, EAL, "receive uevent(name:%s, type:%d, subsystem:%d)\n",
>  		uevent.devname, uevent.type, uevent.subsystem);
> 
> -	if (uevent.devname)
> +	switch (uevent.subsystem) {
> +	case EAL_DEV_EVENT_SUBSYSTEM_PCI:
> +	case EAL_DEV_EVENT_SUBSYSTEM_UIO:
> +		busname = "pci";
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (uevent.devname) {
> +		if (uevent.type == RTE_DEV_EVENT_REMOVE && hotplug_handle) {
> +			rte_spinlock_lock(&failure_handle_lock);
> +			bus = rte_bus_find_by_name(busname);
> +			if (bus == NULL) {
> +				RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n",
> +					busname);
> +				return;
> +			}
> +
> +			dev = bus->find_device(NULL, cmp_dev_name,
> +					       uevent.devname);
> +			if (dev == NULL) {
> +				RTE_LOG(ERR, EAL, "Cannot find device (%s) on "
> +					"bus (%s)\n", uevent.devname, busname);
> +				return;
> +			}
> +
> +			ret = bus->hot_unplug_handler(dev);
> +			rte_spinlock_unlock(&failure_handle_lock);
> +			if (ret) {
> +				RTE_LOG(ERR, EAL, "Can not handle hot-unplug "
> +					"for device (%s)\n", dev->name);
> +				return;
> +			}
> +		}
>  		dev_callback_process(uevent.devname, uevent.type);
> +	}
>  }
> 
>  int __rte_experimental
> @@ -220,5 +320,65 @@ rte_dev_event_monitor_stop(void)
>  	close(intr_handle.fd);
>  	intr_handle.fd = -1;
>  	monitor_started = false;
> +
>  	return 0;
>  }
> +
> +int __rte_experimental
> +rte_dev_sigbus_handler_register(void)
> +{
> +	sigset_t mask;
> +	struct sigaction action;
> +
> +	rte_errno = 0;
> +

Shouldn't you call sigaction only if sigbus_need_recover == 0?

> +	sigemptyset(&mask);
> +	sigaddset(&mask, SIGBUS);
> +	action.sa_flags = SA_SIGINFO;
> +	action.sa_mask = mask;
> +	action.sa_sigaction = sigbus_handler;
> +	sigbus_need_recover = !sigaction(SIGBUS, &action, &sigbus_action_old);
> +
> +	return rte_errno;
> +}
> +
> +int __rte_experimental
> +rte_dev_sigbus_handler_unregister(void)
> +{
> +	rte_errno = 0;
> +	sigbus_need_recover = 1;

Hmm, why to set sugbus_need_recover to 1 here?
If user called rte_dev_sigbus_handler_register() before, and it was successful, it already would be 1.
In other cases, you probably don't have to do anything.
Konstantin

> +
> +	sigbus_action_recover();
> +
> +	return rte_errno;
> +}
> +
> +int __rte_experimental
> +rte_dev_hotplug_handle_enable(void)
> +{
> +	int ret = 0;
> +
> +	ret = rte_dev_sigbus_handler_register();
> +	if (ret < 0)
> +		RTE_LOG(ERR, EAL, "fail to register sigbus handler for "
> +			"devices.\n");
> +
> +	hotplug_handle = true;
> +
> +	return ret;
> +}
> +
> +int __rte_experimental
> +rte_dev_hotplug_handle_disable(void)
> +{
> +	int ret = 0;
> +
> +	ret = rte_dev_sigbus_handler_unregister();
> +	if (ret < 0)
> +		RTE_LOG(ERR, EAL, "fail to unregister sigbus handler for "
> +			"devices.\n");
> +
> +	hotplug_handle = false;
> +
> +	return ret;
> +}
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 73282bb..a3255aa 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -281,6 +281,8 @@ EXPERIMENTAL {
>  	rte_dev_event_callback_unregister;
>  	rte_dev_event_monitor_start;
>  	rte_dev_event_monitor_stop;
> +	rte_dev_hotplug_handle_enable;
> +	rte_dev_hotplug_handle_disable;
>  	rte_dev_iterator_init;
>  	rte_dev_iterator_next;
>  	rte_devargs_add;
> --
> 2.7.4
  
Guo, Jia Oct. 2, 2018, 4:01 a.m. UTC | #2
hi, constantin

On 10/1/2018 3:46 AM, Ananyev, Konstantin wrote:
> Hi Jeff,
>
>> The mechanism can initially register the sigbus handler after the device
>> event monitor is enabled. When a sigbus event is captured, it will check
>> the failure address and accordingly handle the memory failure of the
>> corresponding device by invoke the hot-unplug handler. It could prevent
>> the application from crashing when a device is hot-unplugged.
>>
>> By this patch, users could call below new added APIs to enable/disable
>> the device hotplug handle mechanism. Note that it just implement the
>> hot-unplug handler in these functions, the other handler of hotplug, such
>> as handler for hotplug binding, could be add in the future if need:
>>    - rte_dev_hotplug_handle_enable
>>    - rte_dev_hotplug_handle_disable
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> Acked-by: Shaopeng He <shaopeng.he@intel.com>
>> ---
>> v11->v10:
>> change some words and change the invoked func name.
>> add experimental tag
>> ---
>>   doc/guides/rel_notes/release_18_08.rst  |   5 +
>>   lib/librte_eal/bsdapp/eal/eal_dev.c     |  14 +++
>>   lib/librte_eal/common/eal_private.h     |  26 +++++
>>   lib/librte_eal/common/include/rte_dev.h |  26 +++++
>>   lib/librte_eal/linuxapp/eal/eal_dev.c   | 162 +++++++++++++++++++++++++++++++-
>>   lib/librte_eal/rte_eal_version.map      |   2 +
>>   6 files changed, 234 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
>> index 321fa84..fe0e60f 100644
>> --- a/doc/guides/rel_notes/release_18_08.rst
>> +++ b/doc/guides/rel_notes/release_18_08.rst
>> @@ -117,6 +117,11 @@ New Features
>>
>>     Added support for chained mbufs (input and output).
>>
>> +* **Added hot-unplug handle mechanism.**
>> +
>> +  ``rte_dev_hotplug_handle_enable`` and ``rte_dev_hotplug_handle_disable`` are
>> +  for enabling or disabling hotplug handle mechanism.
>> +
>>
>>   API Changes
>>   -----------
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c b/lib/librte_eal/bsdapp/eal/eal_dev.c
>> index 1c6c51b..255d611 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_dev.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
>> @@ -19,3 +19,17 @@ rte_dev_event_monitor_stop(void)
>>   	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>>   	return -1;
>>   }
>> +
>> +int __rte_experimental
>> +rte_dev_hotplug_handle_enable(void)
>> +{
>> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>> +	return -1;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_hotplug_handle_disable(void)
>> +{
>> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>> +	return -1;
>> +}
>> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
>> index a2d1528..637f20d 100644
>> --- a/lib/librte_eal/common/eal_private.h
>> +++ b/lib/librte_eal/common/eal_private.h
>> @@ -317,4 +317,30 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
>>    */
>>   int rte_bus_sigbus_handler(const void *failure_addr);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Register the sigbus handler.
>> + *
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +int
>> +rte_dev_sigbus_handler_register(void);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Unregister the sigbus handler.
>> + *
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +int
>> +rte_dev_sigbus_handler_unregister(void);
>> +
>>   #endif /* _EAL_PRIVATE_H_ */
>> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
>> index b80a805..ff580a0 100644
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -460,4 +460,30 @@ rte_dev_event_monitor_start(void);
>>   int __rte_experimental
>>   rte_dev_event_monitor_stop(void);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Enable hotplug handling for devices.
>> + *
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +int __rte_experimental
>> +rte_dev_hotplug_handle_enable(void);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Disable hotplug handling for devices.
>> + *
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +int __rte_experimental
>> +rte_dev_hotplug_handle_disable(void);
>> +
>>   #endif /* _RTE_DEV_H_ */
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c
>> index 1cf6aeb..14b18d8 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_dev.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
>> @@ -4,6 +4,8 @@
>>
>>   #include <string.h>
>>   #include <unistd.h>
>> +#include <fcntl.h>
>> +#include <signal.h>
>>   #include <sys/socket.h>
>>   #include <linux/netlink.h>
>>
>> @@ -14,15 +16,32 @@
>>   #include <rte_malloc.h>
>>   #include <rte_interrupts.h>
>>   #include <rte_alarm.h>
>> +#include <rte_bus.h>
>> +#include <rte_eal.h>
>> +#include <rte_spinlock.h>
>> +#include <rte_errno.h>
>>
>>   #include "eal_private.h"
>>
>>   static struct rte_intr_handle intr_handle = {.fd = -1 };
>>   static bool monitor_started;
>> +static bool hotplug_handle;
>>
>>   #define EAL_UEV_MSG_LEN 4096
>>   #define EAL_UEV_MSG_ELEM_LEN 128
>>
>> +/*
>> + * spinlock for device hot-unplug failure handling. If it try to access bus or
>> + * device, such as handle sigbus on bus or handle memory failure for device
>> + * just need to use this lock. It could protect the bus and the device to avoid
>> + * race condition.
>> + */
>> +static rte_spinlock_t failure_handle_lock = RTE_SPINLOCK_INITIALIZER;
>> +
>> +static struct sigaction sigbus_action_old;
>> +
>> +static int sigbus_need_recover;
>> +
>>   static void dev_uev_handler(__rte_unused void *param);
>>
>>   /* identify the system layer which reports this event. */
>> @@ -33,6 +52,49 @@ enum eal_dev_event_subsystem {
>>   	EAL_DEV_EVENT_SUBSYSTEM_MAX
>>   };
>>
>> +static void
>> +sigbus_action_recover(void)
>> +{
>> +	if (sigbus_need_recover) {
>> +		sigaction(SIGBUS, &sigbus_action_old, NULL);
>> +		sigbus_need_recover = 0;
>> +	}
>> +}
>> +
>> +static void sigbus_handler(int signum, siginfo_t *info,
>> +				void *ctx __rte_unused)
>> +{
>> +	int ret;
>> +
>> +	RTE_LOG(INFO, EAL, "Thread[%d] catch SIGBUS, fault address:%p\n",
>> +		(int)pthread_self(), info->si_addr);
>> +
>> +	rte_spinlock_lock(&failure_handle_lock);
>> +	ret = rte_bus_sigbus_handler(info->si_addr);
>> +	rte_spinlock_unlock(&failure_handle_lock);
>> +	if (ret == -1) {
>> +		rte_exit(EXIT_FAILURE,
>> +			 "Failed to handle SIGBUS for hot-unplug, "
>> +			 "(rte_errno: %s)!", strerror(rte_errno));
>> +	} else if (ret == 1) {
>> +		if (sigbus_action_old.sa_handler)
>> +			(*(sigbus_action_old.sa_handler))(signum);
>> +		else
>> +			rte_exit(EXIT_FAILURE,
>> +				 "Failed to handle generic SIGBUS!");
>> +	}
>> +
>> +	RTE_LOG(INFO, EAL, "Success to handle SIGBUS for hot-unplug!\n");
>> +}
>> +
>> +static int cmp_dev_name(const struct rte_device *dev,
>> +	const void *_name)
>> +{
>> +	const char *name = _name;
>> +
>> +	return strcmp(dev->name, name);
>> +}
>> +
>>   static int
>>   dev_uev_socket_fd_create(void)
>>   {
>> @@ -147,6 +209,9 @@ dev_uev_handler(__rte_unused void *param)
>>   	struct rte_dev_event uevent;
>>   	int ret;
>>   	char buf[EAL_UEV_MSG_LEN];
>> +	struct rte_bus *bus;
>> +	struct rte_device *dev;
>> +	const char *busname = "";
>>
>>   	memset(&uevent, 0, sizeof(struct rte_dev_event));
>>   	memset(buf, 0, EAL_UEV_MSG_LEN);
>> @@ -171,8 +236,43 @@ dev_uev_handler(__rte_unused void *param)
>>   	RTE_LOG(DEBUG, EAL, "receive uevent(name:%s, type:%d, subsystem:%d)\n",
>>   		uevent.devname, uevent.type, uevent.subsystem);
>>
>> -	if (uevent.devname)
>> +	switch (uevent.subsystem) {
>> +	case EAL_DEV_EVENT_SUBSYSTEM_PCI:
>> +	case EAL_DEV_EVENT_SUBSYSTEM_UIO:
>> +		busname = "pci";
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	if (uevent.devname) {
>> +		if (uevent.type == RTE_DEV_EVENT_REMOVE && hotplug_handle) {
>> +			rte_spinlock_lock(&failure_handle_lock);
>> +			bus = rte_bus_find_by_name(busname);
>> +			if (bus == NULL) {
>> +				RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n",
>> +					busname);
>> +				return;
>> +			}
>> +
>> +			dev = bus->find_device(NULL, cmp_dev_name,
>> +					       uevent.devname);
>> +			if (dev == NULL) {
>> +				RTE_LOG(ERR, EAL, "Cannot find device (%s) on "
>> +					"bus (%s)\n", uevent.devname, busname);
>> +				return;
>> +			}
>> +
>> +			ret = bus->hot_unplug_handler(dev);
>> +			rte_spinlock_unlock(&failure_handle_lock);
>> +			if (ret) {
>> +				RTE_LOG(ERR, EAL, "Can not handle hot-unplug "
>> +					"for device (%s)\n", dev->name);
>> +				return;
>> +			}
>> +		}
>>   		dev_callback_process(uevent.devname, uevent.type);
>> +	}
>>   }
>>
>>   int __rte_experimental
>> @@ -220,5 +320,65 @@ rte_dev_event_monitor_stop(void)
>>   	close(intr_handle.fd);
>>   	intr_handle.fd = -1;
>>   	monitor_started = false;
>> +
>>   	return 0;
>>   }
>> +
>> +int __rte_experimental
>> +rte_dev_sigbus_handler_register(void)
>> +{
>> +	sigset_t mask;
>> +	struct sigaction action;
>> +
>> +	rte_errno = 0;
>> +
> Shouldn't you call sigaction only if sigbus_need_recover == 0?


i guess what you mean is that the sigbus_need_recover is need check 
before call sigaction, because the register could be call many times, if 
so, i think it is make sense to check it every time.


>> +	sigemptyset(&mask);
>> +	sigaddset(&mask, SIGBUS);
>> +	action.sa_flags = SA_SIGINFO;
>> +	action.sa_mask = mask;
>> +	action.sa_sigaction = sigbus_handler;
>> +	sigbus_need_recover = !sigaction(SIGBUS, &action, &sigbus_action_old);
>> +
>> +	return rte_errno;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_sigbus_handler_unregister(void)
>> +{
>> +	rte_errno = 0;
>> +	sigbus_need_recover = 1;
> Hmm, why to set sugbus_need_recover to 1 here?
> If user called rte_dev_sigbus_handler_register() before, and it was successful, it already would be 1.
> In other cases, you probably don't have to do anything.
> Konstantin


you are right, it should let each sigaction calling to manage this macro 
but no other more place, thanks.


>> +
>> +	sigbus_action_recover();
>> +
>> +	return rte_errno;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_hotplug_handle_enable(void)
>> +{
>> +	int ret = 0;
>> +
>> +	ret = rte_dev_sigbus_handler_register();
>> +	if (ret < 0)
>> +		RTE_LOG(ERR, EAL, "fail to register sigbus handler for "
>> +			"devices.\n");
>> +
>> +	hotplug_handle = true;
>> +
>> +	return ret;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_hotplug_handle_disable(void)
>> +{
>> +	int ret = 0;
>> +
>> +	ret = rte_dev_sigbus_handler_unregister();
>> +	if (ret < 0)
>> +		RTE_LOG(ERR, EAL, "fail to unregister sigbus handler for "
>> +			"devices.\n");
>> +
>> +	hotplug_handle = false;
>> +
>> +	return ret;
>> +}
>> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
>> index 73282bb..a3255aa 100644
>> --- a/lib/librte_eal/rte_eal_version.map
>> +++ b/lib/librte_eal/rte_eal_version.map
>> @@ -281,6 +281,8 @@ EXPERIMENTAL {
>>   	rte_dev_event_callback_unregister;
>>   	rte_dev_event_monitor_start;
>>   	rte_dev_event_monitor_stop;
>> +	rte_dev_hotplug_handle_enable;
>> +	rte_dev_hotplug_handle_disable;
>>   	rte_dev_iterator_init;
>>   	rte_dev_iterator_next;
>>   	rte_devargs_add;
>> --
>> 2.7.4
  

Patch

diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
index 321fa84..fe0e60f 100644
--- a/doc/guides/rel_notes/release_18_08.rst
+++ b/doc/guides/rel_notes/release_18_08.rst
@@ -117,6 +117,11 @@  New Features
 
   Added support for chained mbufs (input and output).
 
+* **Added hot-unplug handle mechanism.**
+
+  ``rte_dev_hotplug_handle_enable`` and ``rte_dev_hotplug_handle_disable`` are
+  for enabling or disabling hotplug handle mechanism.
+
 
 API Changes
 -----------
diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c b/lib/librte_eal/bsdapp/eal/eal_dev.c
index 1c6c51b..255d611 100644
--- a/lib/librte_eal/bsdapp/eal/eal_dev.c
+++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
@@ -19,3 +19,17 @@  rte_dev_event_monitor_stop(void)
 	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
 	return -1;
 }
+
+int __rte_experimental
+rte_dev_hotplug_handle_enable(void)
+{
+	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
+	return -1;
+}
+
+int __rte_experimental
+rte_dev_hotplug_handle_disable(void)
+{
+	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
+	return -1;
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index a2d1528..637f20d 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -317,4 +317,30 @@  rte_devargs_layers_parse(struct rte_devargs *devargs,
  */
 int rte_bus_sigbus_handler(const void *failure_addr);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Register the sigbus handler.
+ *
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int
+rte_dev_sigbus_handler_register(void);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Unregister the sigbus handler.
+ *
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int
+rte_dev_sigbus_handler_unregister(void);
+
 #endif /* _EAL_PRIVATE_H_ */
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a805..ff580a0 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -460,4 +460,30 @@  rte_dev_event_monitor_start(void);
 int __rte_experimental
 rte_dev_event_monitor_stop(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Enable hotplug handling for devices.
+ *
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int __rte_experimental
+rte_dev_hotplug_handle_enable(void);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Disable hotplug handling for devices.
+ *
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int __rte_experimental
+rte_dev_hotplug_handle_disable(void);
+
 #endif /* _RTE_DEV_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c
index 1cf6aeb..14b18d8 100644
--- a/lib/librte_eal/linuxapp/eal/eal_dev.c
+++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
@@ -4,6 +4,8 @@ 
 
 #include <string.h>
 #include <unistd.h>
+#include <fcntl.h>
+#include <signal.h>
 #include <sys/socket.h>
 #include <linux/netlink.h>
 
@@ -14,15 +16,32 @@ 
 #include <rte_malloc.h>
 #include <rte_interrupts.h>
 #include <rte_alarm.h>
+#include <rte_bus.h>
+#include <rte_eal.h>
+#include <rte_spinlock.h>
+#include <rte_errno.h>
 
 #include "eal_private.h"
 
 static struct rte_intr_handle intr_handle = {.fd = -1 };
 static bool monitor_started;
+static bool hotplug_handle;
 
 #define EAL_UEV_MSG_LEN 4096
 #define EAL_UEV_MSG_ELEM_LEN 128
 
+/*
+ * spinlock for device hot-unplug failure handling. If it try to access bus or
+ * device, such as handle sigbus on bus or handle memory failure for device
+ * just need to use this lock. It could protect the bus and the device to avoid
+ * race condition.
+ */
+static rte_spinlock_t failure_handle_lock = RTE_SPINLOCK_INITIALIZER;
+
+static struct sigaction sigbus_action_old;
+
+static int sigbus_need_recover;
+
 static void dev_uev_handler(__rte_unused void *param);
 
 /* identify the system layer which reports this event. */
@@ -33,6 +52,49 @@  enum eal_dev_event_subsystem {
 	EAL_DEV_EVENT_SUBSYSTEM_MAX
 };
 
+static void
+sigbus_action_recover(void)
+{
+	if (sigbus_need_recover) {
+		sigaction(SIGBUS, &sigbus_action_old, NULL);
+		sigbus_need_recover = 0;
+	}
+}
+
+static void sigbus_handler(int signum, siginfo_t *info,
+				void *ctx __rte_unused)
+{
+	int ret;
+
+	RTE_LOG(INFO, EAL, "Thread[%d] catch SIGBUS, fault address:%p\n",
+		(int)pthread_self(), info->si_addr);
+
+	rte_spinlock_lock(&failure_handle_lock);
+	ret = rte_bus_sigbus_handler(info->si_addr);
+	rte_spinlock_unlock(&failure_handle_lock);
+	if (ret == -1) {
+		rte_exit(EXIT_FAILURE,
+			 "Failed to handle SIGBUS for hot-unplug, "
+			 "(rte_errno: %s)!", strerror(rte_errno));
+	} else if (ret == 1) {
+		if (sigbus_action_old.sa_handler)
+			(*(sigbus_action_old.sa_handler))(signum);
+		else
+			rte_exit(EXIT_FAILURE,
+				 "Failed to handle generic SIGBUS!");
+	}
+
+	RTE_LOG(INFO, EAL, "Success to handle SIGBUS for hot-unplug!\n");
+}
+
+static int cmp_dev_name(const struct rte_device *dev,
+	const void *_name)
+{
+	const char *name = _name;
+
+	return strcmp(dev->name, name);
+}
+
 static int
 dev_uev_socket_fd_create(void)
 {
@@ -147,6 +209,9 @@  dev_uev_handler(__rte_unused void *param)
 	struct rte_dev_event uevent;
 	int ret;
 	char buf[EAL_UEV_MSG_LEN];
+	struct rte_bus *bus;
+	struct rte_device *dev;
+	const char *busname = "";
 
 	memset(&uevent, 0, sizeof(struct rte_dev_event));
 	memset(buf, 0, EAL_UEV_MSG_LEN);
@@ -171,8 +236,43 @@  dev_uev_handler(__rte_unused void *param)
 	RTE_LOG(DEBUG, EAL, "receive uevent(name:%s, type:%d, subsystem:%d)\n",
 		uevent.devname, uevent.type, uevent.subsystem);
 
-	if (uevent.devname)
+	switch (uevent.subsystem) {
+	case EAL_DEV_EVENT_SUBSYSTEM_PCI:
+	case EAL_DEV_EVENT_SUBSYSTEM_UIO:
+		busname = "pci";
+		break;
+	default:
+		break;
+	}
+
+	if (uevent.devname) {
+		if (uevent.type == RTE_DEV_EVENT_REMOVE && hotplug_handle) {
+			rte_spinlock_lock(&failure_handle_lock);
+			bus = rte_bus_find_by_name(busname);
+			if (bus == NULL) {
+				RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n",
+					busname);
+				return;
+			}
+
+			dev = bus->find_device(NULL, cmp_dev_name,
+					       uevent.devname);
+			if (dev == NULL) {
+				RTE_LOG(ERR, EAL, "Cannot find device (%s) on "
+					"bus (%s)\n", uevent.devname, busname);
+				return;
+			}
+
+			ret = bus->hot_unplug_handler(dev);
+			rte_spinlock_unlock(&failure_handle_lock);
+			if (ret) {
+				RTE_LOG(ERR, EAL, "Can not handle hot-unplug "
+					"for device (%s)\n", dev->name);
+				return;
+			}
+		}
 		dev_callback_process(uevent.devname, uevent.type);
+	}
 }
 
 int __rte_experimental
@@ -220,5 +320,65 @@  rte_dev_event_monitor_stop(void)
 	close(intr_handle.fd);
 	intr_handle.fd = -1;
 	monitor_started = false;
+
 	return 0;
 }
+
+int __rte_experimental
+rte_dev_sigbus_handler_register(void)
+{
+	sigset_t mask;
+	struct sigaction action;
+
+	rte_errno = 0;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGBUS);
+	action.sa_flags = SA_SIGINFO;
+	action.sa_mask = mask;
+	action.sa_sigaction = sigbus_handler;
+	sigbus_need_recover = !sigaction(SIGBUS, &action, &sigbus_action_old);
+
+	return rte_errno;
+}
+
+int __rte_experimental
+rte_dev_sigbus_handler_unregister(void)
+{
+	rte_errno = 0;
+	sigbus_need_recover = 1;
+
+	sigbus_action_recover();
+
+	return rte_errno;
+}
+
+int __rte_experimental
+rte_dev_hotplug_handle_enable(void)
+{
+	int ret = 0;
+
+	ret = rte_dev_sigbus_handler_register();
+	if (ret < 0)
+		RTE_LOG(ERR, EAL, "fail to register sigbus handler for "
+			"devices.\n");
+
+	hotplug_handle = true;
+
+	return ret;
+}
+
+int __rte_experimental
+rte_dev_hotplug_handle_disable(void)
+{
+	int ret = 0;
+
+	ret = rte_dev_sigbus_handler_unregister();
+	if (ret < 0)
+		RTE_LOG(ERR, EAL, "fail to unregister sigbus handler for "
+			"devices.\n");
+
+	hotplug_handle = false;
+
+	return ret;
+}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 73282bb..a3255aa 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -281,6 +281,8 @@  EXPERIMENTAL {
 	rte_dev_event_callback_unregister;
 	rte_dev_event_monitor_start;
 	rte_dev_event_monitor_stop;
+	rte_dev_hotplug_handle_enable;
+	rte_dev_hotplug_handle_disable;
 	rte_dev_iterator_init;
 	rte_dev_iterator_next;
 	rte_devargs_add;