diff mbox series

[V5,5/7] bus: add helper to handle sigbus

Message ID 1530776333-30318-6-git-send-email-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series hot plug failure handle mechanism |

Checks

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

Commit Message

Guo, Jia July 5, 2018, 7:38 a.m. UTC 
  This patch aim to add a helper to iterate all buses to find the
corresponding bus to handle the sigbus error.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v5->v4:
refine the errno restore logic
---
 lib/librte_eal/common/eal_common_bus.c | 36 +++++++++++++++++++++++++++++++++-
 lib/librte_eal/common/eal_private.h    | 12 ++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

Comments

He, Shaopeng July 6, 2018, 3:22 p.m. UTC | #1 
> -----Original Message-----
> From: Guo, Jia
> Sent: Thursday, July 5, 2018 3:39 PM
> 
> This patch aim to add a helper to iterate all buses to find the
> corresponding bus to handle the sigbus error.
> 

[...]

> +	bus = rte_bus_find(NULL, bus_handle_sigbus, failure_addr);
> +	/* failed to handle the sigbus, pass the new errno. */
> +	if (bus && rte_errno == -1)
> +		return -1;
> +	else if (!bus)
> +		ret = 1;

Change the compare order, code will be a little bit shorter?
	if (!bus)
		ret = 1
	else if (rte_errno == -1)
		return -1;

[...]

Acked-by: Shaopeng He <shaopeng.he@intel.com>
Andrew Rybchenko July 8, 2018, 1:30 p.m. UTC | #2 
On 05.07.2018 10:38, Jeff Guo wrote:
> This patch aim to add a helper to iterate all buses to find the
> corresponding bus to handle the sigbus error.
>
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v5->v4:
> refine the errno restore logic
> ---
>   lib/librte_eal/common/eal_common_bus.c | 36 +++++++++++++++++++++++++++++++++-
>   lib/librte_eal/common/eal_private.h    | 12 ++++++++++++
>   2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 0943851..c9f3566 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -37,6 +37,7 @@
>   #include <rte_bus.h>
>   #include <rte_debug.h>
>   #include <rte_string_fns.h>
> +#include <rte_errno.h>
>   
>   #include "eal_private.h"
>   
> @@ -220,7 +221,6 @@ rte_bus_find_by_device_name(const char *str)
>   	return rte_bus_find(NULL, bus_can_parse, name);
>   }
>   
> -

Unrelated change.

>   /*
>    * Get iommu class of devices on the bus.
>    */
> @@ -242,3 +242,37 @@ rte_bus_get_iommu_class(void)
>   	}
>   	return mode;
>   }
> +
> +static int
> +bus_handle_sigbus(const struct rte_bus *bus,
> +			const void *failure_addr)
> +{
> +	int ret;
> +
> +	ret = bus->sigbus_handler(failure_addr);

Shouldn't bus->sigbus_handler be checked here against NULL?
It looks like not all buses implement it.

> +	rte_errno = ret;
> +
> +	return !(bus->sigbus_handler && ret <= 0);
> +}
> +
> +int
> +rte_bus_sigbus_handler(const void *failure_addr)
> +{
> +	struct rte_bus *bus;
> +
> +	int ret = 0;
> +	int old_errno = rte_errno;
> +	rte_errno = 0;
> +
> +	bus = rte_bus_find(NULL, bus_handle_sigbus, failure_addr);
> +	/* failed to handle the sigbus, pass the new errno. */
> +	if (bus && rte_errno == -1)
> +		return -1;
> +	else if (!bus)
> +		ret = 1;
> +
> +	/* otherwise restore the old errno. */
> +	rte_errno = old_errno;
> +
> +	return ret;
> +}
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index bdadc4d..a91c4b5 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -258,4 +258,16 @@ int rte_mp_channel_init(void);
>    */
>   void dev_callback_process(char *device_name, enum rte_dev_event_type event);
>   
> +
> +/**
> + * Iterate all buses to find the corresponding bus, to handle the sigbus error.
> + * @param failure_addr
> + *	Pointer of the fault address of the sigbus error.
> + *
> + * @return
> + *	 0 success to handle the sigbus.
> + *	-1 failed to handle the sigbus
> + *	 1 no bus can handler the sigbus
> + */
> +int rte_bus_sigbus_handler(const void *failure_addr);

Empty line is missing after the function.

>   #endif /* _EAL_PRIVATE_H_ */
Guo, Jia July 9, 2018, 5:31 a.m. UTC | #3 
hi, shaopeng

thanks for your review.


On 7/6/2018 11:22 PM, He, Shaopeng wrote:
>> -----Original Message-----
>> From: Guo, Jia
>> Sent: Thursday, July 5, 2018 3:39 PM
>>
>> This patch aim to add a helper to iterate all buses to find the
>> corresponding bus to handle the sigbus error.
>>
> [...]
>
>> +	bus = rte_bus_find(NULL, bus_handle_sigbus, failure_addr);
>> +	/* failed to handle the sigbus, pass the new errno. */
>> +	if (bus && rte_errno == -1)
>> +		return -1;
>> +	else if (!bus)
>> +		ret = 1;
> Change the compare order, code will be a little bit shorter?
> 	if (!bus)
> 		ret = 1
> 	else if (rte_errno == -1)
> 		return -1;
>
> [...]

make sense.

> Acked-by: Shaopeng He <shaopeng.he@intel.com>
>
Guo, Jia July 9, 2018, 5:33 a.m. UTC | #4 
hi, andrew

Thanks for your reviewing.

On 7/8/2018 9:30 PM, Andrew Rybchenko wrote:
> On 05.07.2018 10:38, Jeff Guo wrote:
>> This patch aim to add a helper to iterate all buses to find the
>> corresponding bus to handle the sigbus error.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v5->v4:
>> refine the errno restore logic
>> ---
>>   lib/librte_eal/common/eal_common_bus.c | 36 
>> +++++++++++++++++++++++++++++++++-
>>   lib/librte_eal/common/eal_private.h    | 12 ++++++++++++
>>   2 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_bus.c 
>> b/lib/librte_eal/common/eal_common_bus.c
>> index 0943851..c9f3566 100644
>> --- a/lib/librte_eal/common/eal_common_bus.c
>> +++ b/lib/librte_eal/common/eal_common_bus.c
>> @@ -37,6 +37,7 @@
>>   #include <rte_bus.h>
>>   #include <rte_debug.h>
>>   #include <rte_string_fns.h>
>> +#include <rte_errno.h>
>>     #include "eal_private.h"
>>   @@ -220,7 +221,6 @@ rte_bus_find_by_device_name(const char *str)
>>       return rte_bus_find(NULL, bus_can_parse, name);
>>   }
>>   -
>
> Unrelated change.
>

ok. I am fine to let it left to other specific patch.

>>   /*
>>    * Get iommu class of devices on the bus.
>>    */
>> @@ -242,3 +242,37 @@ rte_bus_get_iommu_class(void)
>>       }
>>       return mode;
>>   }
>> +
>> +static int
>> +bus_handle_sigbus(const struct rte_bus *bus,
>> +            const void *failure_addr)
>> +{
>> +    int ret;
>> +
>> +    ret = bus->sigbus_handler(failure_addr);
>
> Shouldn't bus->sigbus_handler be checked here against NULL?
> It looks like not all buses implement it.
>

should be like what you said.

>> +    rte_errno = ret;
>> +
>> +    return !(bus->sigbus_handler && ret <= 0);
>> +}
>> +
>> +int
>> +rte_bus_sigbus_handler(const void *failure_addr)
>> +{
>> +    struct rte_bus *bus;
>> +
>> +    int ret = 0;
>> +    int old_errno = rte_errno;
>> +    rte_errno = 0;
>> +
>> +    bus = rte_bus_find(NULL, bus_handle_sigbus, failure_addr);
>> +    /* failed to handle the sigbus, pass the new errno. */
>> +    if (bus && rte_errno == -1)
>> +        return -1;
>> +    else if (!bus)
>> +        ret = 1;
>> +
>> +    /* otherwise restore the old errno. */
>> +    rte_errno = old_errno;
>> +
>> +    return ret;
>> +}
>> diff --git a/lib/librte_eal/common/eal_private.h 
>> b/lib/librte_eal/common/eal_private.h
>> index bdadc4d..a91c4b5 100644
>> --- a/lib/librte_eal/common/eal_private.h
>> +++ b/lib/librte_eal/common/eal_private.h
>> @@ -258,4 +258,16 @@ int rte_mp_channel_init(void);
>>    */
>>   void dev_callback_process(char *device_name, enum 
>> rte_dev_event_type event);
>>   +
>> +/**
>> + * Iterate all buses to find the corresponding bus, to handle the 
>> sigbus error.
>> + * @param failure_addr
>> + *    Pointer of the fault address of the sigbus error.
>> + *
>> + * @return
>> + *     0 success to handle the sigbus.
>> + *    -1 failed to handle the sigbus
>> + *     1 no bus can handler the sigbus
>> + */
>> +int rte_bus_sigbus_handler(const void *failure_addr);
>
> Empty line is missing after the function.
>

ok.

>>   #endif /* _EAL_PRIVATE_H_ */
>
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 0943851..c9f3566 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -37,6 +37,7 @@ 
 #include <rte_bus.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
+#include <rte_errno.h>
 
 #include "eal_private.h"
 
@@ -220,7 +221,6 @@  rte_bus_find_by_device_name(const char *str)
 	return rte_bus_find(NULL, bus_can_parse, name);
 }
 
-
 /*
  * Get iommu class of devices on the bus.
  */
@@ -242,3 +242,37 @@  rte_bus_get_iommu_class(void)
 	}
 	return mode;
 }
+
+static int
+bus_handle_sigbus(const struct rte_bus *bus,
+			const void *failure_addr)
+{
+	int ret;
+
+	ret = bus->sigbus_handler(failure_addr);
+	rte_errno = ret;
+
+	return !(bus->sigbus_handler && ret <= 0);
+}
+
+int
+rte_bus_sigbus_handler(const void *failure_addr)
+{
+	struct rte_bus *bus;
+
+	int ret = 0;
+	int old_errno = rte_errno;
+	rte_errno = 0;
+
+	bus = rte_bus_find(NULL, bus_handle_sigbus, failure_addr);
+	/* failed to handle the sigbus, pass the new errno. */
+	if (bus && rte_errno == -1)
+		return -1;
+	else if (!bus)
+		ret = 1;
+
+	/* otherwise restore the old errno. */
+	rte_errno = old_errno;
+
+	return ret;
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index bdadc4d..a91c4b5 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -258,4 +258,16 @@  int rte_mp_channel_init(void);
  */
 void dev_callback_process(char *device_name, enum rte_dev_event_type event);
 
+
+/**
+ * Iterate all buses to find the corresponding bus, to handle the sigbus error.
+ * @param failure_addr
+ *	Pointer of the fault address of the sigbus error.
+ *
+ * @return
+ *	 0 success to handle the sigbus.
+ *	-1 failed to handle the sigbus
+ *	 1 no bus can handler the sigbus
+ */
+int rte_bus_sigbus_handler(const void *failure_addr);
 #endif /* _EAL_PRIVATE_H_ */