From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 008225681 for ; Thu, 22 Sep 2016 17:08:50 +0200 (CEST) Received: by mail-wm0-f47.google.com with SMTP id l132so153411746wmf.1 for ; Thu, 22 Sep 2016 08:08:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=qlg2/5osgPdCY6Plfhz8qzvsSNdBGzJNLiMLUGqu8tM=; b=as+cXQIPE+IpMRUz7s4yjSFLst11jMdwtZR+ROMdUopfXMosMV1Z4aN/J+SCq+35AY scAHKH1Kc3Vfe/2sytdyqVZwMRvWAPfNylWC/IwAfR3A+i0IkRi3vCSfe0o4mL6JihC4 np0XHoPHm8yy9u85iyZX8fv91xACcaxGAcLiZN5u11+QrgaCQxHFG4bo/0FLcr6sV2/t oGGei3qgl+nirMT5UI/mlQgmAHO7esfW8OBEyt7KkLr1yocWH5RbVrZbUcFFdoYfNDFp RuimXsqKNtFtmIsSJBw4qiHRmk/Ug7K0pdiFhlsegYSFG1R7eVuqdPEmmpYnE4p6fc6E 3p7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=qlg2/5osgPdCY6Plfhz8qzvsSNdBGzJNLiMLUGqu8tM=; b=Yzp8KZXwbo3zIhmievyL0KBkjl+gniwCirrECUmFMqh66O1wylh9hOoS4t8tQZQXjZ T81ROFu6qfFeZbDcoSpdB30o8FMT+X7DCq86L+XIA/v0/WEK2uWe2E+GNfK+h5/1gXbW /L7YlEOMO4BWvdtnc9YUy3gKU9Lg43X972Oo+3qU9ELB0LmttXO0sautHl5qoMWfbCYm uZdLFgamv8KFpNEj5ErDek4nDrn/zOLSpqDibflQOgpaANYGwW/sIJ+vkr9CDo0eQA+Q Jtiec5+Dg3WPsMYR2eEwDJwlcSogbWV5wLKm25JZ/6M+p1d1+ecA6lcyCeKQItgWmR8i Y9pA== X-Gm-Message-State: AE9vXwN6/lzDmIM+gaVES1gk7a+TgRio/qx+DvLirDBDdBlLXov6Wg+VmtL/GGJzDmlqIMSH X-Received: by 10.194.116.134 with SMTP id jw6mr2470395wjb.134.1474556930699; Thu, 22 Sep 2016 08:08:50 -0700 (PDT) Received: from xps13.localnet (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id n5sm2508779wjv.35.2016.09.22.08.08.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 22 Sep 2016 08:08:46 -0700 (PDT) From: Thomas Monjalon To: jozmarti@cisco.com Cc: dev@dpdk.org Date: Thu, 22 Sep 2016 17:08:42 +0200 Message-ID: <4326235.prI1QteV26@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <3d62cdb3b19648d8a4ed7e0ff4847db5@XCH-RCD-019.cisco.com> References: <1469016644-6521-1-git-send-email-jozmarti@cisco.com> <2644416.cGCtye7R7u@xps13> <3d62cdb3b19648d8a4ed7e0ff4847db5@XCH-RCD-019.cisco.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] rte_delay_us can be replaced with user function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2016 15:08:51 -0000 2016-09-22 08:37, Jozef Martiniak -X: > Hello Thomas, > > > First a general comment: please check your patch with scripts/checkpatches.sh. > Done. checkpath.pl is taken from latest linux kernel... I can see these warnings: UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned' LEADING_SPACE: please, no spaces at the start of a line > > What will happen if we need to provide more builtin handlers? > > I still think that rte_delay_us_block can be exported and initialized > The idea was to make this function simple&&fast as possible. If user needs > more builtin handlers - he can implement this functionality within his delay > function. > > Btw there is also another patch with the same functionality (uses weak symbols) > which I would prefer: > https://git.fd.io/cgit/vpp/tree/dpdk/dpdk-16.04_patches/0005-Allow-applications-to-override-rte_delay_us.patch I don't like the VPP patch because it is hacky and weak symbols should be avoided for static linking. > Do you have any idea how should rte_delay_us_callback_register look like for > multiple handlers ? Yes, just as simple as an assignment: void rte_delay_us_callback_register(void (*func)(unsigned)) { rte_delay_us = func; } We just need to export rte_delay_us_block and call rte_delay_us_callback_register(rte_delay_us_block) at EAL initialization or in a constructor. > Other comments are accepted. > > Best regards, > Jozef > > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Wednesday, September 21, 2016 3:13 PM > To: Jozef Martiniak -X (jozmarti - PANTHEON TECHNOLOGIES at Cisco) > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] rte_delay_us can be replaced with user function > > Hi, > > I think this feature should enter in the release 16.11. > We just need to make sure it is implemented with the right API. > Do you have any comment about managing several builtin handlers? > > > 2016-09-13 22:04, Thomas Monjalon: > > Hi, > > > > Sorry for late review. > > This patch was in a summer hole :/ > > > > First a general comment: please check your patch with > > scripts/checkpatches.sh. > > In order to ease tracking of this patch, please increment the version > > when sending a new one in the same thread: > > git send-email -1 -v3 --annotate --to dev@dpdk.org \ > > --in-reply-to 1469016644-6521-1-git-send-email-jozmarti@cisco.com > > > > More comments below. > > > > 2016-07-20 14:10, jozmarti@cisco.com: > > > +void rte_delay_us_callback_register(void (*userfunc)(unsigned)) { > > > + if (userfunc == NULL) > > > + rte_delay_us = rte_delay_us_block; > > > > Here you are creating an exception for rte_delay_us_block which is > > mapped as a NULL handler. > > What will happen if we need to provide more builtin handlers? > > I still think that rte_delay_us_block can be exported and initialized > > as the default handler. Other opinions are obviously welcome. > > > > > + else > > > + rte_delay_us = userfunc; > > > +} > > > + > > > +static void __attribute__((constructor)) > > > +rte_timer_init(void) > > > +{ > > > + /* set rte_delay_us_block as a delay function */ > > > + rte_delay_us_callback_register(NULL); > > > +} > > > diff --git a/lib/librte_eal/common/include/generic/rte_cycles.h > > > b/lib/librte_eal/common/include/generic/rte_cycles.h > > > index 8cc21f2..7a45b58 100644 > > > --- a/lib/librte_eal/common/include/generic/rte_cycles.h > > > +++ b/lib/librte_eal/common/include/generic/rte_cycles.h > > > @@ -182,13 +182,16 @@ rte_get_timer_hz(void) } > > > > > > /** > > > + * > > > > useless newline > > > > > * Wait at least us microseconds. > > > + * This function can be replaced with user-defined function using > > > + * rte_delay_us_callback_register > > > > I think you can use @see to point to rte_delay_us_callback_register. > > > > > * > > > * @param us > > > * The number of microseconds to wait. > > > */ > > > void > > > -rte_delay_us(unsigned us); > > > +(*rte_delay_us)(unsigned us); > > > > > > /** > > > * Wait at least ms milliseconds. > > > @@ -202,4 +205,14 @@ rte_delay_ms(unsigned ms) > > > rte_delay_us(ms * 1000); > > > } > > > > > > +/** > > > + * Replace rte_delay_us with user defined function. > > > + * > > > + * @param userfunc > > > + * User function which replaces rte_delay_us. NULL restores > > > + * buildin block delay function. > > > > buildin -> builtin ? > > > > > + */ > > > +void rte_delay_us_callback_register(void(*userfunc)(unsigned)); > > > >