hi, gaetan and konstantin
answer both of your questions here as below.
On 6/29/2018 8:52 PM, Gaëtan Rivet wrote:
On Fri, Jun 29, 2018 at 12:21:39PM +0000, Ananyev, Konstantin wrote:
-----Original Message-----
From: Guo, Jia
Sent: Friday, June 29, 2018 12:23 PM
To: Ananyev, Konstantin <konstantin.anan...@intel.com>;
step...@networkplumber.org; Richardson, Bruce
<bruce.richard...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com>;
gaetan.ri...@6wind.com; Wu, Jingjing
<jingjing...@intel.com>; tho...@monjalon.net; mo...@mellanox.com;
ma...@mellanox.com; Van Haaren, Harry
<harry.van.haa...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; He, Shaopeng
<shaopeng...@intel.com>; Iremonger, Bernard
<bernard.iremon...@intel.com>
Cc: jblu...@infradead.org; shreyansh.j...@nxp.com; dev@dpdk.org; Zhang, Helin
<helin.zh...@intel.com>
Subject: Re: [PATCH V4 5/9] bus: add helper to handle sigbus
hi, konstantin
On 6/29/2018 6:51 PM, Ananyev, Konstantin wrote:
+int
+rte_bus_sigbus_handler(const void *failure_addr)
+{
+ struct rte_bus *bus;
+ int old_errno = rte_errno;
+ int ret = 0;
+
+ rte_errno = 0;
+
+ bus = rte_bus_find(NULL, bus_handle_sigbus, failure_addr);
+ if (bus == NULL) {
+ RTE_LOG(ERR, EAL, "No bus can handle the sigbus error!");
+ ret = -1;
+ } else if (rte_errno != 0) {
+ RTE_LOG(ERR, EAL, "Failed to handle the sigbus error!");
+ ret = -1;
+ }
+
+ /* if sigbus not be handled, return back old errno. */
+ if (ret)
+ rte_errno = old_errno;
Hmm, not sure why we need to set/restore rte_errno here?
restore old_errno just use to let caller know that the generic sigbus
still not handler by bus hotplug handler, that involve find a bus
handle but failed and can not find a hander, and can corresponding use
the previous sigbus handler to process it.
that is also unwser your question in other patch. do you think that make
sense?
Sorry, still don't understand the intention.
Suppose rte_bus_find() will return NULL, in that case you'll setup rte_errno
to what it was before calling that function.
If the returned bus is not NULL, but bus_find() set's an rte_errno,
you still would restore rte_ernno?
What is the prupose?
Why do you need to touch rte_errno at all in that function?
Konstantin
The way it is written here does not work, but the intention is
to make sure that a previous error is still catched. Something like
that:
int old_errno = rte_errno;
rte_errno = 0;
rte_eal_call();
if (rte_errno)
return -1;
else {
rte_errno = old_errno;
return 0;
}
If someone calls the function while rte_errno is already set, then an
earlier error would be hidden by setting rte_errno to 0 within the
function.
I'm not sure this is useful, but sometimes when using errno within a
library call I'm bothered that I am masking previous issues.
Should it be avoided?
i agree with konstantin about distinguish to process the handle failed
or no handle,
and agree with gaetan about restore the errno if it is not belong to the
sigbus handler.
Could you check if it is fulfill that as bellow,
-1 means find bus but handle failed, use rte_exit.
1 means can no find bus, use older handler to handle.
0 means find bus and success handle. the handler is the new handler.
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;
}
static void sigbus_handler(int signum, siginfo_t *info,
void *ctx __rte_unused)
{
int ret;
rte_spinlock_lock(&dev_failure_lock);
ret = rte_bus_sigbus_handler(info->si_addr);
rte_spinlock_unlock(&dev_failure_lock);
if (ret == -1) {
rte_exit(EXIT_FAILURE,
"Failed to handle SIGBUS for hotplug, "
"(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!");
}
}
