From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com [74.125.82.67]) by dpdk.org (Postfix) with ESMTP id 8B9C25F34 for ; Tue, 10 Jul 2018 10:41:29 +0200 (CEST) Received: by mail-wm0-f67.google.com with SMTP id v3-v6so19807486wmh.0 for ; Tue, 10 Jul 2018 01:41:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=GPbjDPrgr6Qf3XPrHUJSx+F4BzeYDioDsLyjPR3kv9Q=; b=PxRmhn/txJFkmu1pUCo7vODQ1UQ6OG/KpcDK7OTzYj8Leya4BeN9ougZZ1cvE8j0Kz /5uX45M2y9fgdN8hGW9wj4YbI3uAa8pctLEKufUj+au6+2znhYD7p/2HBcFGqVAhZDVF VMVtKLdvoDWM4P/N7rsglrQAiBMvLGFD+kS/aNKFv77UYr1wzXye9Ndlrm/zhUC5ssrA MARhobYA/vO9VvKwTmSvqDkf8J/E/PTV8vkQ7xMLuYJZz7ow5GpGdkDKr7up6qp7l0Ey MtT2ijom27iwxDFMWY9/QhKoLaWp7ktW+CVATquuekaxOATGZadBbfEzbvie3tjR67qQ YOIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=GPbjDPrgr6Qf3XPrHUJSx+F4BzeYDioDsLyjPR3kv9Q=; b=SPmFulhB5i1BBaUf9y5GORJUPXY4PsWueD9FnaArvOmAMpR2LoYyPNDFaEZ0MrTmT9 sZiBckfKsIevOp+EsYPU+Jf1T7CSk+uUL/bfyk+A8lH6RAqwqiBZTatKtzerUIkJhdGK +jkqAWPabfIKmYAU6BahgSuOROwR/rEJjV6tGUxdkd6kL3QoRn26XjvrFwh2hHOx5KiI l4+1oO0BR9LmsBjdOkRkEFtbdOVB7f+tZcvH+4XdCzoh/ErweqWi3I69AMDW1+1zcvXE 3oSb680dA058r3IJyrVZvllJAutBqMBTBh7Uuf7XTFQPM1hpU/p2ON5lGwrUPS86d+tX rOmA== X-Gm-Message-State: APt69E1nseiEirDwffbihStOJ1iK8jkHdzRFGZvW/61874VMj4LVolr8 yjh41GEGuCITSjO19AzUGCk2Cg== X-Google-Smtp-Source: AAOMgpdqO5GbP3fy/wRgW7tovBEUHF8pRA3c8vF6Bt0XqZ628q5RToikLxZTFxxoRfBsONRjf9ZmdA== X-Received: by 2002:a1c:3c4:: with SMTP id 187-v6mr13078571wmd.96.1531212089059; Tue, 10 Jul 2018 01:41:29 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id y3-v6sm9147533wmd.24.2018.07.10.01.41.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 10 Jul 2018 01:41:28 -0700 (PDT) Date: Tue, 10 Jul 2018 10:40:46 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Jeff Guo Cc: Andrew Rybchenko , stephen@networkplumber.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, jingjing.wu@intel.com, thomas@monjalon.net, motih@mellanox.com, matan@mellanox.com, harry.van.haaren@intel.com, qi.z.zhang@intel.com, shaopeng.he@intel.com, bernard.iremonger@intel.com, wenzhuo.lu@intel.com, jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com Message-ID: <20180710084046.mhejmfodrebapgzt@bidouze.vm.6wind.com> References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1531137666-10351-1-git-send-email-jia.guo@intel.com> <1531137666-10351-6-git-send-email-jia.guo@intel.com> <7d719385-51e5-a45b-341a-d07f7cb92445@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7d719385-51e5-a45b-341a-d07f7cb92445@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v7 5/7] bus: add helper to handle sigbus X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Jul 2018 08:41:29 -0000 On Tue, Jul 10, 2018 at 04:22:23PM +0800, Jeff Guo wrote: > > > On 7/9/2018 9:48 PM, Andrew Rybchenko wrote: > > On 09.07.2018 15:01, 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 > > > Acked-by: Shaopeng He > > > --- > > > v7->v6: > > > no change > > > --- > > > lib/librte_eal/common/eal_common_bus.c | 42 > > > ++++++++++++++++++++++++++++++++++ > > > lib/librte_eal/common/eal_private.h | 12 ++++++++++ > > > 2 files changed, 54 insertions(+) > > > > > > diff --git a/lib/librte_eal/common/eal_common_bus.c > > > b/lib/librte_eal/common/eal_common_bus.c > > > index 0943851..8856adc 100644 > > > --- a/lib/librte_eal/common/eal_common_bus.c > > > +++ b/lib/librte_eal/common/eal_common_bus.c > > > @@ -37,6 +37,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include "eal_private.h" > > > @@ -242,3 +243,44 @@ rte_bus_get_iommu_class(void) > > > } > > > return mode; > > > } > > > + > > > +static int > > > +bus_handle_sigbus(const struct rte_bus *bus, > > > + const void *failure_addr) > > > +{ > > > + int ret; > > > + > > > + if (!bus->sigbus_handler) { > > > + RTE_LOG(ERR, EAL, "Function sigbus_handler not supported by " > > > + "bus (%s)\n", bus->name); > > > > It is not an error. It is OK that some buses cannot handle SIGBUS. > > > > yes, it is. > > > > + return -1; > > > + } > > > + > > > + ret = bus->sigbus_handler(failure_addr); > > > + rte_errno = ret; > > > + > > > + return !(bus->sigbus_handler && ret <= 0); > > > > There is no point to check bus->sigbus_handler here. It is already > > checked above. > > So, it should be just: > > return ret > 0; > > I.e. we should continue search if the address is not handled by any > > device > > on the bus (we should stop if it is handled (ret==0) or failed to to > > handle > > (ret < 0)). > > > > i will modify it, thanks. > Why is rte_errno set here? rte_errno is meant by the bus dev to be set on error. You do not have to modify it. ret would already be <0 on error. At most, you could do something like: if (ret < 0 && rte_errno == 0) rte_errno = ENOTSUP; Or something akin, with a non-descriptive error hinting that the developper didn't seem to care about setting errno to something meaningful (so only partially respecting the API). > > > +} > > > + > > > +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) > > > + ret = 1; > > > + else if (rte_errno == -1) > > > > I'm still thinking it is bad to keep negative value in rte_errno here. > > > > i think the rte_errno just no used for the caller if return -1. Since if > find bus but process failed, will use rte_exit to process whatever the > rte_errno value. Only return 1 means use the origin sigbus handler that will > care about the errno. > With the changes above, the check should be something like: if (bus == NULL) return 1; else if (rte_errno != 0) return -rte_errno; rte_errno = old_errno; return 0; Which would avoid resetting rte_errno on top of whichever value a dev would have used, and having it set to a negative non-errno value. (Please do not just use this as-is, if you think this is not a good idea just tell us why or how you would prefer to do it. I'm only proposing a way that I think would work.) Regards, -- Gaëtan Rivet 6WIND