From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f178.google.com (mail-io0-f178.google.com [209.85.223.178]) by dpdk.org (Postfix) with ESMTP id BD304D14C for ; Fri, 20 Apr 2018 15:55:50 +0200 (CEST) Received: by mail-io0-f178.google.com with SMTP id y128-v6so10643463iod.4 for ; Fri, 20 Apr 2018 06:55:50 -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=WEs2b9vU3bEAKNbsm0VIaDMY8LP9JPzdI72zSbZhJPo=; b=GwyVfPJuCJc9lJ1hZEVKrv2Er8poT0jeTL+4J1/ZKABtjaW+6CVhasRlEyI3cI1FQL YAdoqenSFEO3kWFWSTQJx9Efgl1ujH1YoN0zHeMM5hko5JPVNh5oGY/E6981o1JGNwpR zqzzoGw35tQ9LNkrIZNw2RysqurcHK3YUBY6OLoZvhGX64pD66/tfaCdZSj0ETawEb/K laGC5c5FWALKol4BOsGnzDgmSyGgWsaEGu/y1HAkbYJKFWvXixgc14+TVq5nvXX7p8OR OR9spFUeshNhkZxt1krHr4e3GLq7S1C5H9CNqhZ+Sos/XWUeADa1KSvGFF28kJ5fbA1u S/bQ== 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=WEs2b9vU3bEAKNbsm0VIaDMY8LP9JPzdI72zSbZhJPo=; b=CaVviPDMTYlvfMeq03ayA0a4HUqd+V3NDOx1B0WgLWxyXKanozixZBsZ2eFl28W6Vh EdK0te5AKP8XO+1UvUVjyOvH0clh2rDkBkdk3W3sg+FclnQUMbbpGNmp5cW0g0I7pD5R Wj7N9WMIb1hlVmyE7888U5P5jj3pQ3cUC3eRuBwYjs+rHwX4lg7nLoOfPNYbwey/3kUl YXbuREIr8d4wb1kCQseA1KkOA3G8JyEtFyMFoYAD5tzkQDrSnhW32Jgc8H1VqmQk+b40 bJIedgDuXtLiMvmRjsRGwh/RnrDk6VtL5C9IEnrgib9KYrFJJlJUEe1Ref6BvYYhZ4Lc VUEA== X-Gm-Message-State: ALQs6tCtyhcOCXn1Bht7hIDZf6J+aPiE9DlsDIAV9z+QFK7stK9WSPxe r6eQX5G7qoroCszApB7veF/oyvWOGYRb3C/++g/l+g== X-Google-Smtp-Source: AIpwx48F8paaYEeuyEatfvN3igGSf8tYbgITweeLBjrnJpTiyXbTYRuqXdAMQBHfpe+o/UmW8bpFIfssrds4DNONHEo= X-Received: by 2002:a6b:a89b:: with SMTP id e27-v6mr10224558ioj.180.1524232549965; Fri, 20 Apr 2018 06:55:49 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.142.145 with HTTP; Fri, 20 Apr 2018 06:55:49 -0700 (PDT) In-Reply-To: References: <1524117669-25729-1-git-send-email-arnon@qwilt.com> <1524117669-25729-11-git-send-email-arnon@qwilt.com> From: Arnon Warshavsky Date: Fri, 20 Apr 2018 16:55:49 +0300 Message-ID: To: Aaron Conole Cc: Thomas Monjalon , "Burakov, Anatoly" , "Lu, Wenzhuo" , "Doherty, Declan" , jerin.jacob@caviumnetworks.com, Bruce Richardson , "Yigit, Ferruh" , dev@dpdk.org, Kevin Traynor Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence 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: Fri, 20 Apr 2018 13:55:51 -0000 Thanks Aaron Previously, it wasn't possible for mem_cfg_fd to be reused after a > failure. Now it is - please reset it to -1. in these close conditions. > > Will do. > > > Again, previously this would have aborted on a failure. So it needs to > be reset to a value that allows retry. > Same here > > > switch (rte_config.process_type){ > > case RTE_PROC_PRIMARY: > > - rte_eal_config_create(); > > + if (rte_eal_config_create()) > > + return -1; > > break; > > case RTE_PROC_SECONDARY: > > - rte_eal_config_attach(); > > + if (rte_eal_config_attach()) > > + return -1; > > rte_eal_mcfg_wait_complete(rte_config.mem_config); > > break; > > case RTE_PROC_AUTO: > > case RTE_PROC_INVALID: > > Not for this patch, but I just noticed that this should probably use a > 'default' case. > > Will add this while Im here > > Use rte_eal_init_alert to indicate why you are failing the init. > Will do > > > if (rte_mp_channel_init() < 0) { > > rte_eal_init_alert("failed to init mp channel\n"); > > @@ -652,7 +676,8 @@ static void rte_eal_init_alert(const char *msg) > > > > eal_check_mem_on_local_socket(); > > > > - eal_thread_init_master(rte_config.master_lcore); > > + if (eal_thread_init_master(rte_config.master_lcore) != 0) > > + return -1; > > Is it ever possible to recover from this? Definitely not recoverable, but not different than the other cases where panic propagate all the way up rather than aborting > Still needs > rte_eal_init_alert() call. > Will do > > > How are you cleaning up the threads that were spawned? Lets say this > loop will execute 5 times, and on the 3rd entry, these errors happen. > You now leave DPDK 'half-initialized' - you've spun up threads and > allocated memory. > ... > > I don't see how any of this is better for the user. In fact, I think > this is worse because it will make portions of the application stop > working without any way to move forward. rte_panic() will at least give > the process a chance to recover from a potentially ephemeral condition. > > As I wrote in a different reply on this patch I was probably too eager to get rid of this panic taking some wrong assumptions on the way the library will be called. Removing the panic from the thread is obviously more complex and also ABI breaking. >>From my own bw, I will not make it with a proper change to this version, so I will revert back to panicing on this patchset and aim for the thread in the next build. > > This seems to only exist as a way of triggering the run_once check in > the eal_init. It doesn't add anything except one more state variable to > check against. What is the purpose? > Actually this is not a run-once in purpose, rather an attempt to define a state for the device and on the way work around breaking abi on the the void function called before that. > + if (rte_get_panic_state()) > + return -1; > + Please just use run_once. That's a better way of preventing this. > > As stated above - no a run-once > > All of the comments from the bsd side apply here. >