From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f169.google.com (mail-io0-f169.google.com [209.85.223.169]) by dpdk.org (Postfix) with ESMTP id 106C88D97 for ; Thu, 19 Apr 2018 16:48:03 +0200 (CEST) Received: by mail-io0-f169.google.com with SMTP id f3-v6so6878379iob.13 for ; Thu, 19 Apr 2018 07:48:02 -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=RM3SCngko26Op3MpWpI88kGp0TYtQ9ugG9+cl6WckRo=; b=MZvJ5VB/o5IQsszh2ueigRcyewxAJBB1+N9OV6zYKb2FkTeueF40qreg+3kktXHU2d SFc6DcJ6MDKV6JoCCg4WrJTstmsFXim6MziC0EtrEB7bGjq0BqG6Z5fx7friOHXA94yl Wn2hRsMuI90gF0FTqSxqIxSJvGTK6lfRe6qlEvsXk9nrl4JMqcDqDk6jO3vloOYvUEXC BbYgLH1SecFUSrmOT08KGlnICvPoiU/hvmtkOzsiOljB2236dfbKqgCGTzXyr4kmyqf2 gEJpiAyftNkJxa+5uMnFLpHN7+Tbkht2TjhXQamqpRl+nJjfNx5YD7ofTO8QvHeKpYz2 kbKg== 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=RM3SCngko26Op3MpWpI88kGp0TYtQ9ugG9+cl6WckRo=; b=A3/7WrtoQotvKr2r1SWZuomCXiW1cGXvQniGBhFlc50q/N9xGCTApS/eGFzc/wUB2D N5+qbgEr6B+PYwyMWXzrJGuHqsP9c8uczuwt3uInUhxAtg4l7mgdt/moyV3D34LXiBCn FoqBI4XBlbHDt8BQaT2nrh6XdrCbgEfDer1djztmuq4bZsfItWhVNrLyzKXyQU4/+TwF GdTmTbGlGWk342ZWwo5dC0Ub/xGnWFfriFiKzyc55DHMUs+13d00zRyouG7x8yBBrQqX GkKVKspU9sVJrt5wUlIxf77PauhE+VrWADaIQzWENG5HFGCr5kAqnpxPa0FQOiOsjxwv DlsA== X-Gm-Message-State: ALQs6tBAisKzMX5xVqxB4HbEfJZNxuHEMFF3kFzo8LSiWK26iWj5gCTJ kbvjriBUiIIIPN1xfSJINTKxwokws4rxXAqzB9OySxOglH0= X-Google-Smtp-Source: AB8JxZrdcV2DkaE5UMltip+Xb1jarf71fTQe/HbckNUiFNFxEi+cgLMJY+naPnlhSZ28y4RSMEuB1qMu/mppv1JyN1o= X-Received: by 2002:a6b:2293:: with SMTP id i141-v6mr816372ioi.283.1524149282423; Thu, 19 Apr 2018 07:48:02 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.142.145 with HTTP; Thu, 19 Apr 2018 07:48:01 -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: Thu, 19 Apr 2018 17:48:01 +0300 Message-ID: To: "Burakov, Anatoly" Cc: Thomas Monjalon , "Lu, Wenzhuo" , "Doherty, Declan" , jerin.jacob@caviumnetworks.com, Bruce Richardson , "Yigit, Ferruh" , dev@dpdk.org 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: Thu, 19 Apr 2018 14:48:03 -0000 Copy on the commit message and volatile. Regarding the new function defunct_and_remain_in_endless_loop () I don't think I can put that in a separate patch without breaking the current patch independence. On Thu, Apr 19, 2018 at 5:39 PM, Burakov, Anatoly wrote: > On 19-Apr-18 7:01 AM, Arnon Warshavsky wrote: > >> Local functions to this file, >> changing from void to int are non-abi-breaking. >> For handling the single function that cannot >> change from void to int due to abi, >> where this is the only place it is called in, >> I added a state variable that is being checked >> right after the call to this function. >> > > A rewrite of commit message is in order, i think :) Something like this: > > Change some functions' return type from void to int. This will not break > ABI because they are internal only. > > (see below for comments on lcore changes) > > >> -- >> >> v4 - fix split literal strings in log messages >> >> Signed-off-by: Arnon Warshavsky >> > > Again, please do not add patch/version notes to the commit message, put > them after "---". Version history is not for commit messages, it's for > people reviewing it before merge. > > --- >> lib/librte_eal/bsdapp/eal/eal.c | 86 ++++++++++++++------- >> lib/librte_eal/bsdapp/eal/eal_thread.c | 65 +++++++++++----- >> lib/librte_eal/common/eal_common_launch.c | 21 ++++++ >> lib/librte_eal/common/include/rte_debug.h | 12 +++ >> lib/librte_eal/linuxapp/eal/eal.c | 120 >> ++++++++++++++++++++---------- >> lib/librte_eal/linuxapp/eal/eal_thread.c | 65 +++++++++++----- >> 6 files changed, 270 insertions(+), 99 deletions(-) >> >> diff --git a/lib/librte_eal/bsdapp/eal/eal.c >> b/lib/librte_eal/bsdapp/eal/eal.c >> index d996190..9c2f6f1 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal.c >> +++ b/lib/librte_eal/bsdapp/eal/eal.c >> @@ -151,7 +151,7 @@ enum rte_iova_mode >> * We also don't lock the whole file, so that in future we can use >> read-locks >> * on other parts, e.g. memzones, to detect if there are running >> secondary >> * processes. */ >> -static void >> +static int >> > > <...> > > + >> +/* move to panic state and do not return */ >> +static __attribute__((noreturn)) void >> +defunct_and_remain_in_endless_loop(void) >> +{ >> + rte_move_to_panic_state(); >> + while (1) >> + sleep(1); >> } >> > > It seems like you're mixing two different patchsets here. Maybe it would > be beneficial to put lcore changes in a separate patch? Technically, > rte_panic's in lcore are not part of init sequence. > > (also, should panic state be volatile?) > > > /* main loop of threads */ >> @@ -106,8 +123,11 @@ void eal_thread_init_master(unsigned lcore_id) >> if (thread_id == lcore_config[lcore_id].thread_id) >> break; >> } >> - if (lcore_id == RTE_MAX_LCORE) >> - rte_panic("cannot retrieve lcore id\n"); >> + if (lcore_id == RTE_MAX_LCORE) { >> + RTE_LOG(CRIT, EAL, "%s(): Cannot retrieve lcore id\n", >> > > > -- > Thanks, > Anatoly > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon@qwilt.com *