From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1B7FEA04FA; Wed, 5 Feb 2020 17:41:55 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 593311C2B4; Wed, 5 Feb 2020 17:41:54 +0100 (CET) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id F14231C2A2 for ; Wed, 5 Feb 2020 17:41:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580920912; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1Gf3RNBK+nm185zjildISJ5NuwPm60bD1llzY2EFmcw=; b=FyF7B83RLr3+4rL99FW4Jg66saYJOOpz+GEbih8BOu+aPkhpkTA74wibRU28d+ezafDZFQ PveZgpCRxwfk0WEynMuu4I1BEtpYso636b4GX/hfj6F3F73C4noPxGpEiHo4LZOJWkRhYS X2V97AfuQpSfUT0PwyHZCUQjsA3LA88= Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-357-re70lotaOSCdykUQVEqKLw-1; Wed, 05 Feb 2020 11:41:50 -0500 Received: by mail-ua1-f71.google.com with SMTP id 71so745659uae.22 for ; Wed, 05 Feb 2020 08:41:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5HkQDN60N6gUqMEUw+DSoqNJe9j8e/2DqiSZkDu82eI=; b=J0zC1pdgyfxrTJGEEtDuhrfigDmVwn3vWR49bxJZ+bkFeNCQLQSG1NI5NvpMf6MaeA 7K6qLijqEzEctpHtC4WsoCWv57AiKUT3BMpbYmQCIB1mKFRbGNx+LX/6p9zkI+jGufUx 2sp1yo2EpiuyEKBxUAxBYQg4ziGZnmrcSQnxDgKYFaxw0VDekiD2pZpC9osBJoMeB7zC d8SpaX9vfTXsiDUOYCnShhMCto4Y28z0EwEVgg1euZtp5JeYKBH2IZl5uwGg3RJ+ox64 V9e8cSEDASMKOr0ERqfAuhJUwxOWUSb+bzmoQOkGWNLILMK1d/h5yf10buVqRxNxa2wB CJSA== X-Gm-Message-State: APjAAAUV00GkAWjgFBQKsYaZgGTaeXjapfOsXI1diz9KETSTMymF0cZi xSTUtZX3ZpWQpzpUGQJdP4wdtdo9KifPn3Y4hPVnjm/2587AcKEi/tZuoYzBRbePSa1xOAIM1nF w9gggdfNzYvFyyS9Ec2w= X-Received: by 2002:a1f:5385:: with SMTP id h127mr2193113vkb.56.1580920910208; Wed, 05 Feb 2020 08:41:50 -0800 (PST) X-Google-Smtp-Source: APXvYqxlbOGWo2aXRP9lzSzp1a5TEukN5RfMFkVhOOOMqmyzUQVk+SXRTI0UshzfBLNuPAvHFBT4TBe/ZOU8+JQ9qKg= X-Received: by 2002:a1f:5385:: with SMTP id h127mr2193081vkb.56.1580920909750; Wed, 05 Feb 2020 08:41:49 -0800 (PST) MIME-Version: 1.0 References: <1567748973-24192-1-git-send-email-agupta3@marvell.com> <20200203194912.4669-1-honnappa.nagarahalli@arm.com> <20200203194912.4669-4-honnappa.nagarahalli@arm.com> In-Reply-To: From: David Marchand Date: Wed, 5 Feb 2020 17:41:38 +0100 Message-ID: To: Honnappa Nagarahalli Cc: Amit Gupta , "Wang, Yipeng1" , "Gobriel, Sameh" , "thomas@monjalon.net" , dev , nd X-MC-Unique: re70lotaOSCdykUQVEqKLw-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests 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" On Wed, Feb 5, 2020 at 5:22 PM Honnappa Nagarahalli wrote: > > > > > On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli > > wrote: > > > > > > Add lock-free reader writer concurrency functional tests. > > > These tests will provide the same coverage that non lock-free APIs > > > have. > > > > > > Signed-off-by: Honnappa Nagarahalli > > > --- > > > app/test/test_hash_readwrite.c | 58 > > > +++++++++++++++++++++------------- > > > 1 file changed, 36 insertions(+), 22 deletions(-) > > > > > > diff --git a/app/test/test_hash_readwrite.c > > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644 > > > --- a/app/test/test_hash_readwrite.c > > > +++ b/app/test/test_hash_readwrite.c > > > @@ -121,7 +121,7 @@ > > test_hash_readwrite_worker(__attribute__((unused)) > > > void *arg) } > > > > > > static int > > > -init_params(int use_ext, int use_htm, int use_jhash) > > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash) > > > { > > > unsigned int i; > > > > > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int > > use_jhash) > > > else > > > hash_params.hash_func =3D rte_hash_crc; > > > > > > + hash_params.extra_flag =3D > > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > > > if (use_htm) > > > - hash_params.extra_flag =3D > > > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | > > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > > > + hash_params.extra_flag |=3D > > > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT; > > > + if (rw_lf) > > > + hash_params.extra_flag |=3D > > > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF; > > > else > > > - hash_params.extra_flag =3D > > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > > > + hash_params.extra_flag |=3D > > > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY; > > > > > > if (use_ext) > > > hash_params.extra_flag |=3D @@ -195,7 +196,7 @@ > > > init_params(int use_ext, int use_htm, int use_jhash) } > > > > > > static int > > > -test_hash_readwrite_functional(int use_ext, int use_htm) > > > +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int > > > +use_ext) > > > > This is a bit hard to read, please keep the same order than init_params= . > It looks like it is better to change the init_params. Otherwise, the code= in test_hash_rw_func_main becomes hard to read. See the comment below. > > > > > > > > { > > > unsigned int i; > > > const void *next_key; > > > @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int > > use_htm) > > > rte_atomic64_init(&ginsertions); > > > rte_atomic64_clear(&ginsertions); > > > > > > - if (init_params(use_ext, use_htm, use_jhash) !=3D 0) > > > + if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) !=3D = 0) > > > goto err; > > > > > > if (use_ext) > > > @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int > > use_htm) > > > tbl_rw_test_param.num_insert > > > * slave_cnt; > > > > > > + printf("\nHTM =3D %d, RW-LF =3D %d, EXT-Table =3D %d\n", > > > + use_htm, use_rw_lf, use_ext); > > > printf("++++++++Start function tests:+++++++++\n"); > > > > > > /* Fire all threads. */ > > > @@ -379,7 +382,7 @@ test_hash_readwrite_perf(struct perf *perf_result= s, > > int use_htm, > > > rte_atomic64_init(&gwrite_cycles); > > > rte_atomic64_clear(&gwrite_cycles); > > > > > > - if (init_params(0, use_htm, use_jhash) !=3D 0) > > > + if (init_params(0, use_htm, 0, use_jhash) !=3D 0) > > > goto err; > > > > > > /* > > > @@ -700,7 +703,6 @@ test_hash_rw_func_main(void) > > > * than writer threads. This is to timing either reader threa= ds or > > > * writer threads for performance numbers. > > > */ > > > - int use_htm, use_ext; > > > > The comments block just before is out of sync. > > > > > > > unsigned int i =3D 0, core_id =3D 0; > > > > > > if (rte_lcore_count() < 3) { > > > @@ -721,29 +723,41 @@ test_hash_rw_func_main(void) > > > > > > printf("Test read-write with Hardware transactional > > > memory\n"); > > > > > > - use_htm =3D 1; > > > - use_ext =3D 0; > > > + /* htm =3D 1, rw_lf =3D 0, ext =3D 0 */ > > > > I didn't like those local variables. > > But comments tend to get out of sync fairly easily, please remove too. > > > > > > > + if (test_hash_readwrite_functional(1, 0, 0) < 0) > > > + return -1; > > > > > > - if (test_hash_readwrite_functional(use_ext, use_htm) = < 0) > > > + /* htm =3D 1, rw_lf =3D 1, ext =3D 0 */ > > > + if (test_hash_readwrite_functional(1, 1, 0) < 0) > > > return -1; > > > > > > - use_ext =3D 1; > > > - if (test_hash_readwrite_functional(use_ext, use_htm) = < 0) > > > + /* htm =3D 1, rw_lf =3D 0, ext =3D 1 */ > > > + if (test_hash_readwrite_functional(1, 0, 1) < 0) > > > return -1; > > > > > > + /* htm =3D 1, rw_lf =3D 1, ext =3D 1 */ > > > + if (test_hash_readwrite_functional(1, 1, 1) < 0) > > > + return -1; > > > } else { > > > printf("Hardware transactional memory (lock elision) = " > > > "is NOT supported\n"); > > > } > > > > > > printf("Test read-write without Hardware transactional memory= \n"); > > > - use_htm =3D 0; > > > - use_ext =3D 0; > > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > > > + /* htm =3D 0, rw_lf =3D 0, ext =3D 0 */ > > > + if (test_hash_readwrite_functional(0, 0, 0) < 0) > > > + return -1; > > > + > > > + /* htm =3D 0, rw_lf =3D 1, ext =3D 0 */ > > > + if (test_hash_readwrite_functional(0, 1, 0) < 0) > > > + return -1; > > > + > > > + /* htm =3D 0, rw_lf =3D 0, ext =3D 1 */ > > > + if (test_hash_readwrite_functional(0, 0, 1) < 0) > > > return -1; > > > > > > - use_ext =3D 1; > > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > > > + /* htm =3D 0, rw_lf =3D 1, ext =3D 1 */ > > > + if (test_hash_readwrite_functional(0, 1, 1) < 0) > > > return -1; > The ordering of bits (0-0-0, 0-1-0, 0-0-1, 0-1-1) looks better here. Ok, forget my comment. I just want to get rid of this series and we stop getting random timeout in the CI. I will take it as is and cleanup if I find some time later. --=20 David Marchand