From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 7B0525946 for ; Thu, 28 Aug 2014 10:36:59 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 28 Aug 2014 01:41:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,416,1406617200"; d="scan'208";a="594442268" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga002.jf.intel.com with ESMTP; 28 Aug 2014 01:41:04 -0700 Received: from irsmsx109.ger.corp.intel.com (163.33.3.23) by IRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 28 Aug 2014 09:41:03 +0100 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.112]) by IRSMSX109.ger.corp.intel.com ([169.254.13.200]) with mapi id 14.03.0195.001; Thu, 28 Aug 2014 09:41:03 +0100 From: "Richardson, Bruce" To: Cyril Chemparathy , Thomas Monjalon , "dev@dpdk.org" Thread-Topic: [dpdk-dev] next releases Thread-Index: AQHPwIhBDWUWJgaSGUiCZC+tdifuR5vkoCKAgAEPAzA= Date: Thu, 28 Aug 2014 08:41:02 +0000 Message-ID: <59AF69C657FD0841A61C55336867B5B0343ECAF0@IRSMSX103.ger.corp.intel.com> References: <1598074.SMl35i2x6y@xps13> <53FE0FA5.8020900@tilera.com> In-Reply-To: <53FE0FA5.8020900@tilera.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] next releases 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, 28 Aug 2014 08:37:00 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Cyril Chemparathy > Sent: Wednesday, August 27, 2014 6:05 PM > To: Thomas Monjalon; dev@dpdk.org > Subject: Re: [dpdk-dev] next releases >=20 > Hi Thomas, >=20 > On 8/25/2014 10:15 AM, Thomas Monjalon wrote: > > Hello all, > > > > I am back from holidays; thanks for all the > > patches/reviews/comments done during last weeks. > > > > I'd like to have a version 1.7.1, ideally at the end of this week. > > > > For the coming days, > > - first priority is to integrate bug fixes > > - some changes which do not imply API could be part of 1.7.1 > > - please, do not send more features until 1.8.0-rc1 > > - features that have been been *properly* reviewed or acked before > > end of august will be integrated in 1.8.0-rc1 > > - all pending features which do not have any review will be postpon= ed > > after 1.8.0-rc1 > > - then rc2 will integrate new features if *properly* reviewed at th= at time > > > > I'd like to have some cleanups in version 1.8. Examples: > > - get rid of doxygen warnings > > - check if compile time options can be moved to run time > > - rename some options (CONFIG_RTE_LIBRTE_*_PMD -> > CONFIG_RTE_LIBRTE_PMD_*) > > - merge common code between linux and bsd implementions > > - check secondary process rights > > - remove drivers lists from code for easy integration of new driver= s > > - use rte_eth_dev_atomic_read_link_status in drivers > > - use librte_cfgfile instead of examples/qos_sched > > - add sysfs functions as eal services > > - replace printf calls by rte functions > > - use new assert macros for unit tests > > - remove kni traces from bsd > > - remove bare metal traces > > - compress test_lpm*_routes.h >=20 > Any thoughts on consolidating/cleaning up the timer interfaces? >=20 > Usage across rte_rdtsc(), rte_get_tsc_cycles(), and > rte_get_timer_cycles() could use some rationalization, I think. >=20 > It looks like most code should use rte_get_timer_cycles() and generally > honor user specified timer source selection. The relatively few places > that have a good cause to pin down on TSC should probably use > rte_get_tsc_cycles() instead of rde_rdtsc(). On the other hand, if > rte_rdtsc() is meant for direct use, why do we need the > rte_get_tsc_cycles() wrapper? >=20 > Also, I'm not quite clear on the intended usage of rte_rdtsc_precise(). > I can't find uses of this function on master, and it is not quite clear > to me if the intent is to replace rte_rdtsc() in some (or all?) places > in the code. Any insights on this? >=20 > Thanks > -- Cyril. No comments on any planned cleanup, but I can provide some background on th= e existing situation. (This is all to the best of my memory, since much of = it dates back a few years and I'm not digging through old repos for the off= icial logs :-) )=20 Originally there was an explicit rte_rdtsc() call to do timestamping and ro= ugh cycle counts, and an EAL provided timer api to do wall-time measurement= s using the HPET. Using the HPET proved more awkward than it needed to be (= due to the fact that most distro kernels did not enable the MMAP_HPET optio= n), and had less precision than using rdtsc. So we changed over to having t= he EAL timer use either HPET or TSC depending on what was available at boot= . So we had two timer APIs both of which generally used the TSC, and no way= to allow an application to explicitly request the HPET if that was the tim= esource we wanted. What was worse was that for compatibility across version= s the EAL timers were accessed via APIs with HPET in the name - even when t= hey returned values based off TSC. So we cleaned things up a bit, so that we had a truly generic set of APIs t= hat had a function to return the frequency and cycles for the system-defaul= t time source, as well as specific functions to return that info for both H= PET (if available) and TSC. The names were made as consistent as possible -= so we have rte_get_tsc_cycles() to match the rte_get_timer_cycles() and rt= e_get_hpet_cycles() functions. The original rte_rdtsc() function was also k= ept around for backward compatibility, as it was widely used, so both it an= d rte_get_tsc_cycles() are indeed identical. However, I believe these are t= he only two duplicate functions in the API, with the wrapper function being= just one line long, so it's not a big overhead. I wouldn't look to remove = the wrapper function as it would break the consistency of the API for the s= ake of removing a single-line function, and I definitely wouldn't remove rt= e_rdtsc() because it's so widely used.=20 As for usage, my recommendation that anything wanting to work with wall-tim= e uses rte_get_timer_cycles() (as you suggest), and that anything doing uni= t tests which wants to get approximate cycle counts uses rte_rdtsc() as exi= sting unit test code does. As for rte_rdtsc_precise, I'm not sure about its= origins, but I'm surprised to see that it does not correspond to the rdtsc= p instruction. Can anyone else comment on this one? I would assume it's des= igned to be used to get more accurate measurements of smaller blocks of cod= e that we want to benchmark, since rdtsc works best when timing larger bloc= ks (in terms of cycle counts, that is, not source lines :-) ). Regards, /Bruce