[V5,5/7] bus: add helper to handle sigbus
Checks
Commit Message
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
> -----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>
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_ */
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>
>
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_ */
>
@@ -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;
+}
@@ -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_ */