From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH v12 4/7] bus/pci: implement sigbus handler ops Date: Tue, 2 Oct 2018 15:39:54 +0100 Message-ID: <45ac1ad2-d31e-fa7f-9342-75a3c69fb0ca@intel.com> References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1538483726-96411-1-git-send-email-jia.guo@intel.com> <1538483726-96411-5-git-send-email-jia.guo@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com To: Jeff Guo , stephen@networkplumber.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, gaetan.rivet@6wind.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, arybchenko@solarflare.com, wenzhuo.lu@intel.com, jerin.jacob@caviumnetworks.com Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id B58225F1D for ; Tue, 2 Oct 2018 16:40:38 +0200 (CEST) In-Reply-To: <1538483726-96411-5-git-send-email-jia.guo@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 02-Oct-18 1:35 PM, Jeff Guo wrote: > This patch implements the ops for the PCI bus sigbus handler. It finds the > PCI device that is being hot-unplugged and calls the relevant ops of the > hot-unplug handler to handle the hot-unplug failure of the device. > > Signed-off-by: Jeff Guo > Acked-by: Shaopeng He > --- > v12->v11: > no change. > --- > drivers/bus/pci/pci_common.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c > index d286234..f313fe9 100644 > --- a/drivers/bus/pci/pci_common.c > +++ b/drivers/bus/pci/pci_common.c > @@ -405,6 +405,36 @@ pci_find_device(const struct rte_device *start, rte_dev_cmp_t cmp, > return NULL; > } > > +/** > + * find the device which encounter the failure, by iterate over all device on > + * PCI bus to check if the memory failure address is located in the range > + * of the BARs of the device. > + */ > +static struct rte_pci_device * > +pci_find_device_by_addr(const void *failure_addr) > +{ > + struct rte_pci_device *pdev = NULL; > + int i; > + > + FOREACH_DEVICE_ON_PCIBUS(pdev) { > + for (i = 0; i != RTE_DIM(pdev->mem_resource); i++) { > + if ((uint64_t)(uintptr_t)failure_addr >= > + (uint64_t)(uintptr_t)pdev->mem_resource[i].addr && > + (uint64_t)(uintptr_t)failure_addr < > + (uint64_t)(uintptr_t)pdev->mem_resource[i].addr + > + pdev->mem_resource[i].len) { You must *really* dislike local variables :) Suggested rewriting: const void *start, *end; size_t len; start = pdev->mem_resource[i].addr; len = pdev->mem_resource[i].len; end = RTE_PTR_ADD(start, len); if (failure_addr >= start && failure_addr < end) { ... } I think this is way clearer. > + RTE_LOG(INFO, EAL, "Failure address " > + "%16.16"PRIx64" belongs to " > + "device %s!\n", > + (uint64_t)(uintptr_t)failure_addr, > + pdev->device.name); I feel like this should have DEBUG level, rather than INFO. Alternatively, if you really feel like this should be at level INFO, the message should be reworded because the word "failure" might give the wrong impression :) (but really, i think this is info useful for debugging purposes but not interesting generally, so it should be under DEBUG IMO) -- Thanks, Anatoly