From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id B4C971BACA for ; Tue, 26 Jun 2018 17:48:25 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 08:48:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,274,1526367600"; d="scan'208";a="66482572" Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by fmsmga004.fm.intel.com with ESMTP; 26 Jun 2018 08:48:20 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.139]) by IRSMSX107.ger.corp.intel.com ([169.254.10.238]) with mapi id 14.03.0319.002; Tue, 26 Jun 2018 16:48:19 +0100 From: "De Lara Guarch, Pablo" To: "Wang, Yipeng1" CC: "dev@dpdk.org" , "Mcnamara, John" , "Richardson, Bruce" , "honnappa.nagarahalli@arm.com" , "vguvva@caviumnetworks.com" , "brijesh.s.singh@gmail.com" Thread-Topic: [PATCH v1 2/3] test: add test case for read write concurrency Thread-Index: AQHT/1INSQmB/Agm5U27Ygfy7iE3TKRysIAQ Date: Tue, 26 Jun 2018 15:48:18 +0000 Message-ID: References: <1528455078-328182-1-git-send-email-yipeng1.wang@intel.com> <1528455078-328182-3-git-send-email-yipeng1.wang@intel.com> In-Reply-To: <1528455078-328182-3-git-send-email-yipeng1.wang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMWZjZTZiZjYtYWQ2Yy00OGMyLThjZGMtNWRiM2E1MzRmMDhiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQUxDK3YwNStqb0dIUXVvdnR1U3RUZzJJWkNaTnYyQWxvVEtNeWRXaHllbmQ4T2tHXC81eWwrZ01kZ25TWTNPMW0ifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1 2/3] test: add test case for read write concurrency 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: Tue, 26 Jun 2018 15:48:26 -0000 Hi Yipeng, > -----Original Message----- > From: Wang, Yipeng1 > Sent: Friday, June 8, 2018 11:51 AM > To: De Lara Guarch, Pablo > Cc: dev@dpdk.org; Wang, Yipeng1 ; Mcnamara, > John ; Richardson, Bruce > ; honnappa.nagarahalli@arm.com; > vguvva@caviumnetworks.com; brijesh.s.singh@gmail.com > Subject: [PATCH v1 2/3] test: add test case for read write concurrency >=20 > This commit adds a new test case for testing read/write concurrency. Could you split this patch into two? One adding lock "support" in the curre= nt performance test code and another one adding the new readwrite tests? >=20 > Signed-off-by: Yipeng Wang > --- > test/test/Makefile | 1 + > test/test/test_hash_perf.c | 36 ++- > test/test/test_hash_readwrite.c | 649 ... > +++ b/test/test/test_hash_readwrite.c ... > +struct { > + uint32_t *keys; > + uint32_t *found; > + uint32_t nb_tsx_insertion; > + uint32_t rounded_nb_total_tsx_insertion; > + struct rte_hash *h; > +} tbl_multiwriter_test_params; I think " rounded_nb_total_tsx_insertion" and " tbl_multiwriter_test_params= " are too long, and that's why you need to split a long line into two down be= low. Could you shorten these names a bit? You can change "multiwriter" to "mw", and "rounded_nb_total_tsx_insertion" to "total_nb_tsx_inserts". ... > + snprintf(name, 32, "tests"); > + hash_params.name =3D name; You can set hash_params.name =3D "tests" directly. > + > + handle =3D rte_hash_create(&hash_params); > + if (handle =3D=3D NULL) { > + printf("hash creation failed"); > + return -1; > + } ... > + > +err2: > + rte_free(keys); > +err1: > + rte_hash_free(handle); I think you can have just one label "err" with these two frees. If the variables haven't been set, they will be NULL and that's allowed. =20 > + > + return -1; > +} > + ... > + begin =3D rte_rdtsc_precise(); > + for (i =3D 0; i < read_cnt; i++) { > + void *data; > + rte_hash_lookup_data(tbl_multiwriter_test_params.h, > + tbl_multiwriter_test_params.keys + i, > + &data); > + if (i !=3D (uint64_t)data) { I see the following error and other errors when compiling with gcc 32 bits. test/test/test_hash_readwrite.c:281:12: error: cast from pointer to integer of different size [-Werror=3Dpointer-to-int-ca= st] if (i !=3D (uint64_t)data) { ^ > + printf("lookup find wrong value %d, %ld\n", i, > + (uint64_t)data); ... > + > + if (reader_faster) { > + unsigned long long int cycles_per_insertion =3D > + rte_atomic64_read(&gread_cycles)/ > + rte_atomic64_read(&greads); > + perf_results->read_only[n] =3D cycles_per_insertion; > + printf("Reader only: cycles per lookup: %llu\n", > + cycles_per_insertion); > + } > + > + else { } else { ... > + use_htm =3D 1; > + if (test_hash_readwrite_functional() < 0) > + return -1; > + > + reader_faster =3D 1; Maybe a comment about this reader_faster would be good. Also, can we declare this variable and use_html as local and pass them to t= he functions, instead of declaring them as global? > + if (test_hash_readwrite_perf(&htm_results) < 0) > + return -1;