From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 88C84A0679 for ; Tue, 30 Apr 2019 13:26:06 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 662C85B2E; Tue, 30 Apr 2019 13:26:06 +0200 (CEST) Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by dpdk.org (Postfix) with ESMTP id 4C6535B2A for ; Tue, 30 Apr 2019 13:26:04 +0200 (CEST) Received: from DGGEMS403-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id ABF529B0FC726684FB1E for ; Tue, 30 Apr 2019 19:26:01 +0800 (CST) Received: from [127.0.0.1] (10.177.131.206) by DGGEMS403-HUB.china.huawei.com (10.3.19.203) with Microsoft SMTP Server id 14.3.439.0; Tue, 30 Apr 2019 19:26:00 +0800 To: "Burakov, Anatoly" , CC: References: <1556595548-53745-1-git-send-email-mousuanming@huawei.com> <1556624124-54930-1-git-send-email-mousuanming@huawei.com> <1556624124-54930-2-git-send-email-mousuanming@huawei.com> <655f3c30-b277-fad7-6a68-64dd70fd3601@intel.com> From: Suanming.Mou Message-ID: <49788495-9412-972b-5500-9ff48b36eafe@huawei.com> Date: Tue, 30 Apr 2019 19:25:45 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <655f3c30-b277-fad7-6a68-64dd70fd3601@intel.com> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.177.131.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v5] app/pdump: add pudmp exits with primary support. 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190430112545.MhlvOnw7Ass0-WTkOj7x8ToQd8PxUsOPnIge1vIlhMQ@z> On 2019/4/30 17:42, Burakov, Anatoly wrote: > On 30-Apr-19 12:35 PM, Suanming.Mou wrote: >> When primary app exits, the residual running pdump will stop the >> primary app to restart. Add pdump exits with primary support. >> >> Suggested-by: Varghese, Vipin >> Suggested-by: Burakov, Anatoly >> Signed-off-by: Suanming.Mou >> --- > > > >>   static void >> +disable_primary_monitor(void) >> +{ >> +    int ret; >> + >> +    /* Don't worry about it is primary exit case. The alarm cancel >> +     * function will take care about that. */ >> +    ret = rte_eal_alarm_cancel(monitor_primary, NULL); >> +    if (ret < 0) >> +        printf("Fail to disable monitor fail:%d\n", ret); > > Double fail :) Ah, yes, sorry for that the code gets worse.  :( > >> +} >> + >> +static void >>   signal_handler(int sig_num) >>   { >>       if (sig_num == SIGINT) { >> @@ -910,6 +936,19 @@ struct parse_val { >>           ; >>   } >>   +static void >> +enable_primary_monitor(void) >> +{ >> +    int ret; >> + >> +    /* Once primary exits, so will pdump. */ >> +    ret = rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary, NULL); >> +    if (ret < 0) { >> +        cleanup_pdump_resources(); >> +        rte_exit(EXIT_FAILURE, "Fail to monitor primary:%d\n", ret); >> +    } > > Why is this function void, when you could've called rte_exit() in the > caller on failure? And why is it such a fatal error to set up the > timer? IMO just a warning would've been enough. Here comes with two issues: Q1. The return value of the function: A1: I'm so sorry that it does not seem to make sense to check the function's return value. Does it mean if we change the timer set up from error to warning, then we can use the return value to judge if need to disable the primary_monitor? Q2. The choice when rte_eal_alarm_set fail: A2: OK, agree with that. > >> +} >> + >>   int >>   main(int argc, char **argv) >>   { >> @@ -950,11 +989,13 @@ struct parse_val { >>               rte_exit(EXIT_FAILURE, "Invalid argument\n"); >>       } >>   -    /* create mempool, ring and vdevs info */ >> +    /* create mempool, ring, vdevs info and primary monitor */ >>       create_mp_ring_vdev(); >>       enable_pdump(); >> +    enable_primary_monitor(); >>       dump_packets(); >>   +    disable_primary_monitor(); >>       cleanup_pdump_resources(); >>       /* dump debug stats */ >>       print_pdump_stats(); >> > >