From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f175.google.com (mail-io0-f175.google.com [209.85.223.175]) by dpdk.org (Postfix) with ESMTP id 78D685F36 for ; Wed, 21 Mar 2018 10:17:33 +0100 (CET) Received: by mail-io0-f175.google.com with SMTP id v13so5773406iob.6 for ; Wed, 21 Mar 2018 02:17:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qwilt-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=fCQg5ca1GsGla2XXlEq/MY54mSa/pyfojkZJOEnudhs=; b=GK0+lUGQbqNfYrrdQzC31B2+yISDL1Fb1nzAuLgaWc2m17LOfSJnECfPNzE/MkOGnf SBydMMmnnUjiJMOhTk4dJ9wTxU4NIFhYuc/CI0j71PuBg8NEKlHFaq+vDSd07HoT0lQ5 u6VUeFQh7v626eohpsDB4RN9hJ9X3yNxuPgn/U9JtjX+EdcJJVR9ujX7r6Uxv9PruN2p MRyGQzC1dc9Zv9Pq/oifK5RT9vcbyVij6ftDf39dVLMmkEP9sReWV5nVPwjIgC3vfS3o IkLpzoWEBY4PXBo157jLITO+LrKNVMVs4GRUAw86WE5IMotdmhFJQe67DzXdWNjhNFGf Y1Ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fCQg5ca1GsGla2XXlEq/MY54mSa/pyfojkZJOEnudhs=; b=QFjTRzwZk+xl7lKW1AFThAf8h+Q0hJf9utxeuFfimDmvhDGZRltoxqbLVhU9vgD53S 5QOzC+eQ+r/FmZKL3gR4rTN3HlRdJ/BGSbH1waRbjMEH7vkoAqOW7IZCK//uOtRyn9to B5oKacYSyylyprdfhXa3FEf2TU0I88g188SRiBIQN01QFlS+K0UeIXGmfWQbUDlZAEhh U6xWoWmNSdw//SxVKchihvRWgf0sUwQo/fHZW1AuIhfM5xOAE+2gIvwM+F69k5MVyxzW uIRUbKVrykrMjG5rnZoSsbrbIELDazSvZ/hZ1Pd4yxQumY/z/e7nxc9m5CSN7DzdNE34 kcWQ== X-Gm-Message-State: AElRT7FL1u6P+6yy71SLYsYLOxT9Fq502l43DSCHeh0CrO8aETW3YNaz HCMZlDnZrLssoTEcDVrw7zxZ0trSW4hf4xiMLxzV4A== X-Google-Smtp-Source: AG47ELtMR8nAzEFu+0HenrPWj/u3V7tPDAU41GApqLS5J48ybTd4TDrSdrnb+/cmZk+R4nktTPz9xnH933bc55wo6yY= X-Received: by 10.107.58.11 with SMTP id h11mr20395267ioa.283.1521623852722; Wed, 21 Mar 2018 02:17:32 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.142.202 with HTTP; Wed, 21 Mar 2018 02:17:32 -0700 (PDT) In-Reply-To: <20180321091153.GA17868@bricha3-MOBL.ger.corp.intel.com> References: <1521582788-32364-1-git-send-email-arnon@qwilt.com> <20180321091153.GA17868@bricha3-MOBL.ger.corp.intel.com> From: Arnon Warshavsky Date: Wed, 21 Mar 2018 11:17:32 +0200 Message-ID: To: Bruce Richardson Cc: Thomas Monjalon , "Burakov, Anatoly" , wenzhuo.lu@intel.com, declan.doherty@intel.com, jerin.jacob@caviumnetworks.com, ferruh.yigit@intel.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3] eal: replace rte_panic instances to return an error value 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: Wed, 21 Mar 2018 09:17:33 -0000 I'm a fan of such 1:1 baby steps, but I guess I will stick with the flavor of majority here. Will shift those to explicit log and return. On Wed, Mar 21, 2018 at 11:11 AM, Bruce Richardson < bruce.richardson@intel.com> wrote: > On Tue, Mar 20, 2018 at 11:53:08PM +0200, Arnon Warshavsky wrote: > > The purpose of this patch is to cleanup the library code > > from paths that end up aborting the process, > > and move to checking error values, in order to allow the running process > > perform an orderly tear-down or other mitigation of the event. > > > > This patch modifies the majority of rte_panic calls under lib and > drivers, > > and replaces them with a new variation of rte_panic macro > > that does not abort and returns an error value > > that can be propagated up the call stack. > > > > - Focus was given to the dpdk initialization path > > - Some of the panic calls within drivers were left in place where > > the call is from within an interrupt or calls that are on the data > path, > > where there is no simple applicative route to propagate > > the error to termination. > > These should be handled by the driver maintainers. > > I would like to define a device health state that can be monitored from > > the side,and this will be an independent patch. > > - No change took place in example and test files > > - No change took place for debug assertions calling panic > > - Some previously void functions calling panic where changed > > to return a value,with callers modified accordingly. > > > > An additional independent patch to devtools/checkpatches.sh > > will be submitted in order to prevent new additions of calls to rte_panic > > under lib and drivers. > > > > Keep calm and don't panic. > > > > --- > > V2: Fix typos in commit message > > --- > > V3: More commit message formatting > > > > Signed-off-by: Arnon Warshavsky > > --- > > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 2 +- > > drivers/crypto/dpaa_sec/dpaa_sec.c | 2 +- > > drivers/net/bonding/rte_eth_bond_8023ad.c | 23 +++++--- > > drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- > > drivers/net/bonding/rte_eth_bond_api.c | 20 +++++-- > > drivers/net/bonding/rte_eth_bond_pmd.c | 5 +- > > drivers/net/bonding/rte_eth_bond_private.h | 2 +- > > drivers/net/e1000/e1000_ethdev.h | 2 +- > > drivers/net/e1000/igb_ethdev.c | 3 +- > > drivers/net/e1000/igb_pf.c | 10 ++-- > > drivers/net/ixgbe/ixgbe_ethdev.c | 3 +- > > drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- > > drivers/net/ixgbe/ixgbe_pf.c | 6 +- > > lib/librte_eal/bsdapp/eal/eal.c | 54 > ++++++++++------- > > lib/librte_eal/bsdapp/eal/eal_debug.c | 14 +++++ > > lib/librte_eal/bsdapp/eal/eal_thread.c | 43 +++++++++----- > > lib/librte_eal/common/eal_common_memzone.c | 2 +- > > lib/librte_eal/common/eal_thread.h | 4 +- > > lib/librte_eal/common/include/rte_debug.h | 20 +++++++ > > lib/librte_eal/common/rte_malloc.c | 6 +- > > lib/librte_eal/linuxapp/eal/eal.c | 72 > ++++++++++++++--------- > > lib/librte_eal/linuxapp/eal/eal_debug.c | 14 +++++ > > lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 19 ++++-- > > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 16 +++-- > > lib/librte_eal/linuxapp/eal/eal_thread.c | 43 +++++++++----- > > lib/librte_eal/rte_eal_version.map | 1 + > > lib/librte_ether/rte_ethdev.c | 10 +++- > > lib/librte_eventdev/rte_eventdev_pmd_pci.h | 2 +- > > lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 6 +- > > lib/librte_kni/rte_kni.c | 18 ++++-- > > lib/librte_kni/rte_kni_fifo.h | 6 +- > > 31 files changed, 291 insertions(+), 141 deletions(-) > > > > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > > index 9a790dd..9fda47c 100644 > > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c > > @@ -2865,7 +2865,7 @@ struct rte_security_ops dpaa2_sec_security_ops = { > > rte_socket_id()); > > > > if (cryptodev->data->dev_private == NULL) > > - rte_panic("Cannot allocate memzone for private " > > + rte_panic_return("Cannot allocate memzone for > private " > > "device data"); > > } > > To echo Thomas' previous comment, I'm not sure I like the addition of this > new "panic_return" macro. It seems an artificial hack just to have a 1:1 > replacement for rte_panic. I think I'd rather an explicit log and then > return call. > > /Bruce > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon@qwilt.com *