From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by dpdk.org (Postfix) with ESMTP id AE9EE1B727 for ; Wed, 19 Dec 2018 08:33:25 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 589BA40037 for ; Wed, 19 Dec 2018 08:33:25 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 360A240010; Wed, 19 Dec 2018 08:33:25 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on bernadotte.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,AWL autolearn=disabled version=3.4.1 X-Spam-Score: -1.0 Received: from [192.168.1.59] (host-90-232-140-56.mobileonline.telia.com [90.232.140.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 8030A40004; Wed, 19 Dec 2018 08:33:20 +0100 (CET) To: Thomas Monjalon , dev@dpdk.org Cc: Erik Gabriel Carrillo , rsanford@akamai.com, stephen@networkplumber.org, jerin.jacob@caviumnetworks.com, pbhagavatula@caviumnetworks.com References: <1544205180-31546-1-git-send-email-erik.g.carrillo@intel.com> <1544739996-26011-1-git-send-email-erik.g.carrillo@intel.com> <1740997.qR0t5JnZGs@xps> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= Message-ID: Date: Wed, 19 Dec 2018 08:33:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1740997.qR0t5JnZGs@xps> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP Subject: Re: [dpdk-dev] [PATCH v3 0/2] Timer library changes 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, 19 Dec 2018 07:33:25 -0000 On 2018-12-19 04:35, Thomas Monjalon wrote: > 13/12/2018 23:26, Erik Gabriel Carrillo: >> This patch series modifies the timer library in such a way that >> structures that used to be statically allocated in a process's data >> segment are now allocated in shared memory. As these structures contain >> lists of timers, new APIs are introduced that allow a caller to specify >> the particular structure instance into which a timer should be inserted >> or from which a timer should be removed. This enables primary and >> secondary processes to modify the same timer list, which enables some >> multi-process use cases that were not previously possible; e.g. a >> secondary process can start a timer whose expiration is detected in a >> primary process running a new flavor of timer_manage(). >> >> The original library API is mostly unchanged, though implementations are >> updated to call into newly added functions with a default structure >> instance ID that provides the original behavior. New functions are >> introduced to enable applications to allocate structure instances to >> house timer lists, and to reference them with an identifier when >> starting and stopping timers, and finally, to manage the timer lists >> referenced with an identifier. >> >> My initial performance testing with the "timer_perf_autotest" test shows >> no performance regression or improvement, and inspection of the >> generated optimized code shows that the extra function call gets inlined >> in the functions that now have an extra function call. >> >> Depends on: https://patches.dpdk.org/patch/48417/ >> >> Changes in v3: >> - remove C++ style comment in first patch in series (Stephen) >> >> Changes in v2: >> - split these changes out into their own series >> - version the symbols where the existing ABI was updated, and >> provide alternate implementation with behavior equivalent to original >> behavior. Validated ABI compatibility with validate-abi.sh >> - refactor changes to simplify patches >> >> Erik Gabriel Carrillo (2): >> timer: allow timer management in shared memory >> timer: add function to stop all timers in a list >> >> lib/librte_timer/Makefile | 1 + >> lib/librte_timer/rte_timer.c | 558 ++++++++++++++++++++++++++++++--- >> lib/librte_timer/rte_timer.h | 258 ++++++++++++++- >> lib/librte_timer/rte_timer_version.map | 23 ++ >> 4 files changed, 795 insertions(+), 45 deletions(-) > > It is a lot of changes! > Anyone to review please? > > I can give reviewing the overall aim with the patch set a try: Do we really want DPDK to support more secondary process-based use cases? I would rather see it supporting fewer, with the long term goal of dropping support for secondary processes altogether. DPDK secondary processes are a horrible mess, in my opinion, to put it bluntly.