From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70073.outbound.protection.outlook.com [40.107.7.73]) by dpdk.org (Postfix) with ESMTP id BFF5B2BD3 for ; Mon, 25 Mar 2019 11:25:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=9SHtUg617uKGV7EnrR0N1Zl/aunS7HbJDp097yP3QvU=; b=i2qLSGroIjHShpA3kNYKGwqpmfNLtA42DKrSbPEmlJbgKDtvnRzYW2D5cm8gYb4tqB2360734ehL1+frPkjYFVcrSMWOjTBx9pipxvm6AL6BVi+yOGNyfxal6ca4X5ihk38jRmJ3HW41PtaU4k++dSXIq/QO+VK5oDlAw0Tr3V4= Received: from AM0PR08MB3587.eurprd08.prod.outlook.com (20.177.110.157) by AM0PR08MB3394.eurprd08.prod.outlook.com (20.177.110.93) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1730.18; Mon, 25 Mar 2019 10:25:05 +0000 Received: from AM0PR08MB3587.eurprd08.prod.outlook.com ([fe80::455f:1485:fda2:e120]) by AM0PR08MB3587.eurprd08.prod.outlook.com ([fe80::455f:1485:fda2:e120%2]) with mapi id 15.20.1730.019; Mon, 25 Mar 2019 10:25:05 +0000 From: "Joyce Kong (Arm Technology China)" To: "Ananyev, Konstantin" , "dev@dpdk.org" CC: nd , "stephen@networkplumber.org" , "jerin.jacob@caviumnetworks.com" , "thomas@monjalon.net" , Honnappa Nagarahalli , "Gavin Hu (Arm Technology China)" Thread-Topic: [PATCH v7 3/3] test/ticketlock: add ticket lock test case Thread-Index: AQHU4KPLtRuf2AP+AEWv8ZBZ6Al7FKYcKFIA Date: Mon, 25 Mar 2019 10:25:04 +0000 Message-ID: References: <1547802943-18711-1-git-send-email-joyce.kong@arm.com> <1553159608-205213-4-git-send-email-joyce.kong@arm.com> <2601191342CEEE43887BDE71AB977258013655EB2C@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258013655EB2C@irsmsx105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Joyce.Kong@arm.com; x-originating-ip: [113.29.88.7] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c9805111-5946-42d0-f64d-08d6b10c25c4 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600127)(711020)(4605104)(4618075)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:AM0PR08MB3394; x-ms-traffictypediagnostic: AM0PR08MB3394: x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr nodisclaimer: True x-microsoft-antispam-prvs: x-forefront-prvs: 0987ACA2E2 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(376002)(346002)(396003)(39860400002)(366004)(13464003)(51444003)(189003)(199004)(76176011)(8676002)(110136005)(54906003)(305945005)(316002)(7736002)(6246003)(33656002)(486006)(55016002)(14444005)(256004)(9686003)(14454004)(30864003)(446003)(6506007)(86362001)(55236004)(6436002)(105586002)(229853002)(81156014)(476003)(53936002)(5660300002)(11346002)(478600001)(52536014)(53546011)(71200400001)(2906002)(7696005)(97736004)(4326008)(26005)(68736007)(99286004)(25786009)(66066001)(72206003)(106356001)(102836004)(186003)(74316002)(3846002)(2501003)(6116002)(8936002)(81166006)(71190400001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR08MB3394; H:AM0PR08MB3587.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: 49EsHXV4E02EqEdylcUoMMxiPLW50+/I2nkwwI1+S+8EgwOtVmQxUTPIYMnP549M57BwdclyW7zWRS0nywSS8omc7JVeEe1K9baYuTsmRorhsZNg9Fkx7ff4EBZ7IHjkGRHLeMjOqqGc3KFc1pxmgPVZRAig7FEWaYM3QQN5CyuPosK52PYjPAbLCPH0wM6U8yf7KI1Cv/ssncResC1+hciBqnCwHCf3XkixmXiCRmlC181v81jaYnmCHel3ZkQECkWfRhihe0uDaG0pr/JH1wExbZA9kTJJISdFOZO9plKrnxDE7/ouB9Rktu/+wjUUgSwB7+1ZEbMWHmIuOHkcOX4Zu6PRBfhHEkfI/ASGfTPOdMhNtYoP/O1c4sdj0sxGgcA0OleqDLrXHYkzEwJ5sA6u6pWEVfL1lWW+Un6LWDQ= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: c9805111-5946-42d0-f64d-08d6b10c25c4 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Mar 2019 10:25:04.9682 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB3394 Subject: Re: [dpdk-dev] [PATCH v7 3/3] test/ticketlock: add ticket lock test case 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: Mon, 25 Mar 2019 10:25:07 -0000 Hi Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Friday, March 22, 2019 7:39 PM > To: Joyce Kong (Arm Technology China) ; > dev@dpdk.org > Cc: nd ; stephen@networkplumber.org; > jerin.jacob@caviumnetworks.com; thomas@monjalon.net; Honnappa > Nagarahalli ; Gavin Hu (Arm Technology > China) > Subject: RE: [PATCH v7 3/3] test/ticketlock: add ticket lock test case >=20 >=20 >=20 > > > > Add test cases for ticket lock, recursive ticket lock, and ticket lock > > performance. > > > > Signed-off-by: Joyce Kong > > Reviewed-by: Gavin Hu > > Reviewed-by: Ruifeng Wang > > --- > > MAINTAINERS | 1 + > > app/test/Makefile | 1 + > > app/test/autotest_data.py | 6 + > > app/test/meson.build | 1 + > > app/test/test_ticketlock.c | 311 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 5 files changed, 320 insertions(+) > > create mode 100644 app/test/test_ticketlock.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS index 3521271..b1ed4cc 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -213,6 +213,7 @@ F: app/test/test_bitmap.c Ticketlock > > M: Joyce Kong > > F: lib/librte_eal/common/include/generic/rte_ticketlock.h > > +F: app/test/test_ticketlock.c > > > > ARM v7 > > M: Jan Viktorin diff --git > > a/app/test/Makefile b/app/test/Makefile index 89949c2..d6aa28b 100644 > > --- a/app/test/Makefile > > +++ b/app/test/Makefile > > @@ -65,6 +65,7 @@ SRCS-y +=3D test_barrier.c SRCS-y +=3D test_malloc.c > > SRCS-y +=3D test_cycles.c SRCS-y +=3D test_spinlock.c > > +SRCS-y +=3D test_ticketlock.c > > SRCS-y +=3D test_memory.c > > SRCS-y +=3D test_memzone.c > > SRCS-y +=3D test_bitmap.c > > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py > > index 5f87bb9..db25274 100644 > > --- a/app/test/autotest_data.py > > +++ b/app/test/autotest_data.py > > @@ -171,6 +171,12 @@ > > "Report": None, > > }, > > { > > + "Name": "Ticketlock autotest", > > + "Command": "ticketlock_autotest", > > + "Func": ticketlock_autotest, > > + "Report": None, > > + } > > + { > > "Name": "Byte order autotest", > > "Command": "byteorder_autotest", > > "Func": default_autotest, > > diff --git a/app/test/meson.build b/app/test/meson.build index > > 05e5dde..ddb4d09 100644 > > --- a/app/test/meson.build > > +++ b/app/test/meson.build > > @@ -107,6 +107,7 @@ test_sources =3D files('commands.c', > > 'test_timer.c', > > 'test_timer_perf.c', > > 'test_timer_racecond.c', > > + 'test_ticketlock.c', > > 'test_version.c', > > 'virtual_pmd.c' > > ) > > diff --git a/app/test/test_ticketlock.c b/app/test/test_ticketlock.c > > new file mode 100644 index 0000000..67281ce > > --- /dev/null > > +++ b/app/test/test_ticketlock.c > > @@ -0,0 +1,311 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2018-2019 Arm Limited */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "test.h" > > + > > +/* > > + * Ticketlock test > > + * =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > + * > > + * - There is a global ticketlock and a table of ticketlocks (one per = lcore). > > + * > > + * - The test function takes all of these locks and launches the > > + * ``test_ticketlock_per_core()`` function on each core (except the = master). > > + * > > + * - The function takes the global lock, display something, then rel= eases > > + * the global lock. > > + * - The function takes the per-lcore lock, display something, then > releases > > + * the per-core lock. > > + * > > + * - The main function unlocks the per-lcore locks sequentially and > > + * waits between each lock. This triggers the display of a message > > + * for each core, in the correct order. The autotest script checks t= hat > > + * this order is correct. > > + * > > + * - A load test is carried out, with all cores attempting to lock a s= ingle lock > > + * multiple times > > + */ > > + > > +static rte_ticketlock_t tl, tl_try; > > +static rte_ticketlock_t tl_tab[RTE_MAX_LCORE]; static > > +rte_ticketlock_recursive_t tlr; static unsigned int count; > > + > > +static rte_atomic32_t synchro; > > + > > +static int > > +test_ticketlock_per_core(__attribute__((unused)) void *arg) { > > + rte_ticketlock_lock(&tl); > > + printf("Global lock taken on core %u\n", rte_lcore_id()); > > + rte_ticketlock_unlock(&tl); > > + > > + rte_ticketlock_lock(&tl_tab[rte_lcore_id()]); > > + printf("Hello from core %u !\n", rte_lcore_id()); > > + rte_ticketlock_unlock(&tl_tab[rte_lcore_id()]); > > + > > + return 0; > > +} >=20 > I think that's probably no enough for functional testing. > Need something extra to ensure that it provides correct locking in MT env= . > Probably extend the perf test below to do both? > Something like that: >=20 > static uint64_t lcount __rte_cache_aligned; static uint64_t > lcore_count[RTE_MAX_LCORE] __rte_cache_aligned; >=20 > ... >=20 > load_loop_fn(...) > { > ... > rte_ticketlock_lock(&lk); > lcount++; > rte_ticketlock_unlock(&lk); > lcore_count[current_lcore]++; > } >=20 > Then in test_ticketlock_perf() make sure that sum of al lcore_count[] val= ues > equals to lcount value: > tcount =3D 0; > for (i =3D 0; i !=3D RTE_DIM(lcore_count); i++) > tcount +=3D lcore_count[i]; >=20 > if (tcount !=3D lcount) > >=20 > Same thought for trylock. > Konstantin >=20 Got your opinion and will do this in next version. > > + > > +static int > > +test_ticketlock_recursive_per_core(__attribute__((unused)) void *arg) > > +{ > > + unsigned int id =3D rte_lcore_id(); > > + > > + rte_ticketlock_recursive_lock(&tlr); > > + printf("Global recursive lock taken on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_lock(&tlr); > > + printf("Global recursive lock taken on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_lock(&tlr); > > + printf("Global recursive lock taken on core %u - count =3D %d\n", > > + id, tlr.count); > > + > > + printf("Hello from within recursive locks from core %u !\n", id); > > + > > + rte_ticketlock_recursive_unlock(&tlr); > > + printf("Global recursive lock released on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_unlock(&tlr); > > + printf("Global recursive lock released on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_unlock(&tlr); > > + printf("Global recursive lock released on core %u - count =3D %d\n", > > + id, tlr.count); > > + > > + return 0; > > +} > > + > > +static rte_ticketlock_t lk =3D RTE_TICKETLOCK_INITIALIZER; static > > +uint64_t lock_count[RTE_MAX_LCORE] =3D {0}; > > + > > +#define TIME_MS 100 > > + > > +static int > > +load_loop_fn(void *func_param) > > +{ > > + uint64_t time_diff =3D 0, begin; > > + uint64_t hz =3D rte_get_timer_hz(); > > + uint64_t lcount =3D 0; > > + const int use_lock =3D *(int *)func_param; > > + const unsigned int lcore =3D rte_lcore_id(); > > + > > + /* wait synchro for slaves */ > > + if (lcore !=3D rte_get_master_lcore()) > > + while (rte_atomic32_read(&synchro) =3D=3D 0) > > + ; > > + > > + begin =3D rte_get_timer_cycles(); > > + while (time_diff < hz * TIME_MS / 1000) { > > + if (use_lock) > > + rte_ticketlock_lock(&lk); > > + lcount++; > > + if (use_lock) > > + rte_ticketlock_unlock(&lk); > > + /* delay to make lock duty cycle slighlty realistic */ >=20 > Probably better to do here the same as in test spinlock patches: > - remove delay_us() > - move > time_diff =3D rte_get_timer_cycles() - begin; out of the loop and repo= rt > aggregate cycles. >=20 Will do the same as test spinlock patches in next version. > > + rte_delay_us(1); > > + time_diff =3D rte_get_timer_cycles() - begin; > > + } > > + lock_count[lcore] =3D lcount; > > + return 0; > > +} > > + > > +static int > > +test_ticketlock_perf(void) > > +{ > > + unsigned int i; > > + uint64_t total =3D 0; > > + int lock =3D 0; > > + const unsigned int lcore =3D rte_lcore_id(); > > + > > + printf("\nTest with no lock on single core...\n"); > > + load_loop_fn(&lock); > > + printf("Core [%u] count =3D %"PRIu64"\n", lcore, lock_count[lcore]); > > + memset(lock_count, 0, sizeof(lock_count)); > > + > > + printf("\nTest with lock on single core...\n"); > > + lock =3D 1; > > + load_loop_fn(&lock); > > + printf("Core [%u] count =3D %"PRIu64"\n", lcore, lock_count[lcore]); > > + memset(lock_count, 0, sizeof(lock_count)); > > + > > + printf("\nTest with lock on %u cores...\n", rte_lcore_count()); > > + > > + /* Clear synchro and start slaves */ > > + rte_atomic32_set(&synchro, 0); > > + rte_eal_mp_remote_launch(load_loop_fn, &lock, SKIP_MASTER); > > + > > + /* start synchro and launch test on master */ > > + rte_atomic32_set(&synchro, 1); > > + load_loop_fn(&lock); > > + > > + rte_eal_mp_wait_lcore(); > > + > > + RTE_LCORE_FOREACH(i) { > > + printf("Core [%u] count =3D %"PRIu64"\n", i, lock_count[i]); > > + total +=3D lock_count[i]; > > + } > > + > > + printf("Total count =3D %"PRIu64"\n", total); > > + > > + return 0; > > +} > > + > > +/* > > + * Use rte_ticketlock_trylock() to trylock a ticketlock object, > > + * If it could not lock the object successfully, it would > > + * return immediately and the variable of "count" would be > > + * increased by one per times. the value of "count" could be > > + * checked as the result later. > > + */ > > +static int > > +test_ticketlock_try(__attribute__((unused)) void *arg) { > > + if (rte_ticketlock_trylock(&tl_try) =3D=3D 0) { > > + rte_ticketlock_lock(&tl); > > + count++; > > + rte_ticketlock_unlock(&tl); > > + } > > + > > + return 0; > > +} > > + > > + > > +/* > > + * Test rte_eal_get_lcore_state() in addition to ticketlocks > > + * as we have "waiting" then "running" lcores. > > + */ > > +static int > > +test_ticketlock(void) > > +{ > > + int ret =3D 0; > > + int i; > > + > > + /* slave cores should be waiting: print it */ > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + printf("lcore %d state: %d\n", i, > > + (int) rte_eal_get_lcore_state(i)); > > + } > > + > > + rte_ticketlock_init(&tl); > > + rte_ticketlock_init(&tl_try); > > + rte_ticketlock_recursive_init(&tlr); > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_ticketlock_init(&tl_tab[i]); > > + } > > + > > + rte_ticketlock_lock(&tl); > > + > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_ticketlock_lock(&tl_tab[i]); > > + rte_eal_remote_launch(test_ticketlock_per_core, NULL, i); > > + } > > + > > + /* slave cores should be busy: print it */ > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + printf("lcore %d state: %d\n", i, > > + (int) rte_eal_get_lcore_state(i)); > > + } > > + rte_ticketlock_unlock(&tl); > > + > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_ticketlock_unlock(&tl_tab[i]); > > + rte_delay_ms(10); > > + } > > + > > + rte_eal_mp_wait_lcore(); > > + > > + rte_ticketlock_recursive_lock(&tlr); > > + > > + /* > > + * Try to acquire a lock that we already own > > + */ > > + if (!rte_ticketlock_recursive_trylock(&tlr)) { > > + printf("rte_ticketlock_recursive_trylock failed on a lock that " > > + "we already own\n"); > > + ret =3D -1; > > + } else > > + rte_ticketlock_recursive_unlock(&tlr); > > + > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_eal_remote_launch(test_ticketlock_recursive_per_core, > > + NULL, i); > > + } > > + rte_ticketlock_recursive_unlock(&tlr); > > + rte_eal_mp_wait_lcore(); > > + > > + /* > > + * Test if it could return immediately from try-locking a locked obje= ct. > > + * Here it will lock the ticketlock object first, then launch all the > > + * slave lcores to trylock the same ticketlock object. > > + * All the slave lcores should give up try-locking a locked object an= d > > + * return immediately, and then increase the "count" initialized with > > + * zero by one per times. > > + * We can check if the "count" is finally equal to the number of all > > + * slave lcores to see if the behavior of try-locking a locked > > + * ticketlock object is correct. > > + */ > > + if (rte_ticketlock_trylock(&tl_try) =3D=3D 0) > > + return -1; > > + > > + count =3D 0; > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_eal_remote_launch(test_ticketlock_try, NULL, i); > > + } > > + rte_eal_mp_wait_lcore(); > > + rte_ticketlock_unlock(&tl_try); > > + if (rte_ticketlock_is_locked(&tl)) { > > + printf("ticketlock is locked but it should not be\n"); > > + return -1; > > + } > > + rte_ticketlock_lock(&tl); > > + if (count !=3D (rte_lcore_count() - 1)) > > + ret =3D -1; > > + > > + rte_ticketlock_unlock(&tl); > > + > > + /* > > + * Test if it can trylock recursively. > > + * Use rte_ticketlock_recursive_trylock() to check if it can lock > > + * a ticketlock object recursively. Here it will try to lock a > > + * ticketlock object twice. > > + */ > > + if (rte_ticketlock_recursive_trylock(&tlr) =3D=3D 0) { > > + printf("It failed to do the first ticketlock_recursive_trylock " > > + "but it should able to do\n"); > > + return -1; > > + } > > + if (rte_ticketlock_recursive_trylock(&tlr) =3D=3D 0) { > > + printf("It failed to do the second ticketlock_recursive_trylock > " > > + "but it should able to do\n"); > > + return -1; > > + } > > + rte_ticketlock_recursive_unlock(&tlr); > > + rte_ticketlock_recursive_unlock(&tlr); > > + > > + if (test_ticketlock_perf() < 0) > > + return -1; > > + > > + return ret; > > +} > > + > > +REGISTER_TEST_COMMAND(ticketlock_autotest, test_ticketlock); > > -- > > 2.7.4 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 27E5BA05D3 for ; Mon, 25 Mar 2019 11:25:08 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E76F634F3; Mon, 25 Mar 2019 11:25:07 +0100 (CET) Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70073.outbound.protection.outlook.com [40.107.7.73]) by dpdk.org (Postfix) with ESMTP id BFF5B2BD3 for ; Mon, 25 Mar 2019 11:25:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=9SHtUg617uKGV7EnrR0N1Zl/aunS7HbJDp097yP3QvU=; b=i2qLSGroIjHShpA3kNYKGwqpmfNLtA42DKrSbPEmlJbgKDtvnRzYW2D5cm8gYb4tqB2360734ehL1+frPkjYFVcrSMWOjTBx9pipxvm6AL6BVi+yOGNyfxal6ca4X5ihk38jRmJ3HW41PtaU4k++dSXIq/QO+VK5oDlAw0Tr3V4= Received: from AM0PR08MB3587.eurprd08.prod.outlook.com (20.177.110.157) by AM0PR08MB3394.eurprd08.prod.outlook.com (20.177.110.93) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1730.18; Mon, 25 Mar 2019 10:25:05 +0000 Received: from AM0PR08MB3587.eurprd08.prod.outlook.com ([fe80::455f:1485:fda2:e120]) by AM0PR08MB3587.eurprd08.prod.outlook.com ([fe80::455f:1485:fda2:e120%2]) with mapi id 15.20.1730.019; Mon, 25 Mar 2019 10:25:05 +0000 From: "Joyce Kong (Arm Technology China)" To: "Ananyev, Konstantin" , "dev@dpdk.org" CC: nd , "stephen@networkplumber.org" , "jerin.jacob@caviumnetworks.com" , "thomas@monjalon.net" , Honnappa Nagarahalli , "Gavin Hu (Arm Technology China)" Thread-Topic: [PATCH v7 3/3] test/ticketlock: add ticket lock test case Thread-Index: AQHU4KPLtRuf2AP+AEWv8ZBZ6Al7FKYcKFIA Date: Mon, 25 Mar 2019 10:25:04 +0000 Message-ID: References: <1547802943-18711-1-git-send-email-joyce.kong@arm.com> <1553159608-205213-4-git-send-email-joyce.kong@arm.com> <2601191342CEEE43887BDE71AB977258013655EB2C@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258013655EB2C@irsmsx105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Joyce.Kong@arm.com; x-originating-ip: [113.29.88.7] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c9805111-5946-42d0-f64d-08d6b10c25c4 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600127)(711020)(4605104)(4618075)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:AM0PR08MB3394; x-ms-traffictypediagnostic: AM0PR08MB3394: x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr nodisclaimer: True x-microsoft-antispam-prvs: x-forefront-prvs: 0987ACA2E2 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(376002)(346002)(396003)(39860400002)(366004)(13464003)(51444003)(189003)(199004)(76176011)(8676002)(110136005)(54906003)(305945005)(316002)(7736002)(6246003)(33656002)(486006)(55016002)(14444005)(256004)(9686003)(14454004)(30864003)(446003)(6506007)(86362001)(55236004)(6436002)(105586002)(229853002)(81156014)(476003)(53936002)(5660300002)(11346002)(478600001)(52536014)(53546011)(71200400001)(2906002)(7696005)(97736004)(4326008)(26005)(68736007)(99286004)(25786009)(66066001)(72206003)(106356001)(102836004)(186003)(74316002)(3846002)(2501003)(6116002)(8936002)(81166006)(71190400001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR08MB3394; H:AM0PR08MB3587.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: 49EsHXV4E02EqEdylcUoMMxiPLW50+/I2nkwwI1+S+8EgwOtVmQxUTPIYMnP549M57BwdclyW7zWRS0nywSS8omc7JVeEe1K9baYuTsmRorhsZNg9Fkx7ff4EBZ7IHjkGRHLeMjOqqGc3KFc1pxmgPVZRAig7FEWaYM3QQN5CyuPosK52PYjPAbLCPH0wM6U8yf7KI1Cv/ssncResC1+hciBqnCwHCf3XkixmXiCRmlC181v81jaYnmCHel3ZkQECkWfRhihe0uDaG0pr/JH1wExbZA9kTJJISdFOZO9plKrnxDE7/ouB9Rktu/+wjUUgSwB7+1ZEbMWHmIuOHkcOX4Zu6PRBfhHEkfI/ASGfTPOdMhNtYoP/O1c4sdj0sxGgcA0OleqDLrXHYkzEwJ5sA6u6pWEVfL1lWW+Un6LWDQ= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: c9805111-5946-42d0-f64d-08d6b10c25c4 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Mar 2019 10:25:04.9682 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB3394 Subject: Re: [dpdk-dev] [PATCH v7 3/3] test/ticketlock: add ticket lock test case 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" Message-ID: <20190325102504.4FuvhhGQdtvY3fKxGCQmuJBxtahnixEHM9syr6oxjlI@z> Hi Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Friday, March 22, 2019 7:39 PM > To: Joyce Kong (Arm Technology China) ; > dev@dpdk.org > Cc: nd ; stephen@networkplumber.org; > jerin.jacob@caviumnetworks.com; thomas@monjalon.net; Honnappa > Nagarahalli ; Gavin Hu (Arm Technology > China) > Subject: RE: [PATCH v7 3/3] test/ticketlock: add ticket lock test case >=20 >=20 >=20 > > > > Add test cases for ticket lock, recursive ticket lock, and ticket lock > > performance. > > > > Signed-off-by: Joyce Kong > > Reviewed-by: Gavin Hu > > Reviewed-by: Ruifeng Wang > > --- > > MAINTAINERS | 1 + > > app/test/Makefile | 1 + > > app/test/autotest_data.py | 6 + > > app/test/meson.build | 1 + > > app/test/test_ticketlock.c | 311 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 5 files changed, 320 insertions(+) > > create mode 100644 app/test/test_ticketlock.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS index 3521271..b1ed4cc 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -213,6 +213,7 @@ F: app/test/test_bitmap.c Ticketlock > > M: Joyce Kong > > F: lib/librte_eal/common/include/generic/rte_ticketlock.h > > +F: app/test/test_ticketlock.c > > > > ARM v7 > > M: Jan Viktorin diff --git > > a/app/test/Makefile b/app/test/Makefile index 89949c2..d6aa28b 100644 > > --- a/app/test/Makefile > > +++ b/app/test/Makefile > > @@ -65,6 +65,7 @@ SRCS-y +=3D test_barrier.c SRCS-y +=3D test_malloc.c > > SRCS-y +=3D test_cycles.c SRCS-y +=3D test_spinlock.c > > +SRCS-y +=3D test_ticketlock.c > > SRCS-y +=3D test_memory.c > > SRCS-y +=3D test_memzone.c > > SRCS-y +=3D test_bitmap.c > > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py > > index 5f87bb9..db25274 100644 > > --- a/app/test/autotest_data.py > > +++ b/app/test/autotest_data.py > > @@ -171,6 +171,12 @@ > > "Report": None, > > }, > > { > > + "Name": "Ticketlock autotest", > > + "Command": "ticketlock_autotest", > > + "Func": ticketlock_autotest, > > + "Report": None, > > + } > > + { > > "Name": "Byte order autotest", > > "Command": "byteorder_autotest", > > "Func": default_autotest, > > diff --git a/app/test/meson.build b/app/test/meson.build index > > 05e5dde..ddb4d09 100644 > > --- a/app/test/meson.build > > +++ b/app/test/meson.build > > @@ -107,6 +107,7 @@ test_sources =3D files('commands.c', > > 'test_timer.c', > > 'test_timer_perf.c', > > 'test_timer_racecond.c', > > + 'test_ticketlock.c', > > 'test_version.c', > > 'virtual_pmd.c' > > ) > > diff --git a/app/test/test_ticketlock.c b/app/test/test_ticketlock.c > > new file mode 100644 index 0000000..67281ce > > --- /dev/null > > +++ b/app/test/test_ticketlock.c > > @@ -0,0 +1,311 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2018-2019 Arm Limited */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "test.h" > > + > > +/* > > + * Ticketlock test > > + * =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > + * > > + * - There is a global ticketlock and a table of ticketlocks (one per = lcore). > > + * > > + * - The test function takes all of these locks and launches the > > + * ``test_ticketlock_per_core()`` function on each core (except the = master). > > + * > > + * - The function takes the global lock, display something, then rel= eases > > + * the global lock. > > + * - The function takes the per-lcore lock, display something, then > releases > > + * the per-core lock. > > + * > > + * - The main function unlocks the per-lcore locks sequentially and > > + * waits between each lock. This triggers the display of a message > > + * for each core, in the correct order. The autotest script checks t= hat > > + * this order is correct. > > + * > > + * - A load test is carried out, with all cores attempting to lock a s= ingle lock > > + * multiple times > > + */ > > + > > +static rte_ticketlock_t tl, tl_try; > > +static rte_ticketlock_t tl_tab[RTE_MAX_LCORE]; static > > +rte_ticketlock_recursive_t tlr; static unsigned int count; > > + > > +static rte_atomic32_t synchro; > > + > > +static int > > +test_ticketlock_per_core(__attribute__((unused)) void *arg) { > > + rte_ticketlock_lock(&tl); > > + printf("Global lock taken on core %u\n", rte_lcore_id()); > > + rte_ticketlock_unlock(&tl); > > + > > + rte_ticketlock_lock(&tl_tab[rte_lcore_id()]); > > + printf("Hello from core %u !\n", rte_lcore_id()); > > + rte_ticketlock_unlock(&tl_tab[rte_lcore_id()]); > > + > > + return 0; > > +} >=20 > I think that's probably no enough for functional testing. > Need something extra to ensure that it provides correct locking in MT env= . > Probably extend the perf test below to do both? > Something like that: >=20 > static uint64_t lcount __rte_cache_aligned; static uint64_t > lcore_count[RTE_MAX_LCORE] __rte_cache_aligned; >=20 > ... >=20 > load_loop_fn(...) > { > ... > rte_ticketlock_lock(&lk); > lcount++; > rte_ticketlock_unlock(&lk); > lcore_count[current_lcore]++; > } >=20 > Then in test_ticketlock_perf() make sure that sum of al lcore_count[] val= ues > equals to lcount value: > tcount =3D 0; > for (i =3D 0; i !=3D RTE_DIM(lcore_count); i++) > tcount +=3D lcore_count[i]; >=20 > if (tcount !=3D lcount) > >=20 > Same thought for trylock. > Konstantin >=20 Got your opinion and will do this in next version. > > + > > +static int > > +test_ticketlock_recursive_per_core(__attribute__((unused)) void *arg) > > +{ > > + unsigned int id =3D rte_lcore_id(); > > + > > + rte_ticketlock_recursive_lock(&tlr); > > + printf("Global recursive lock taken on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_lock(&tlr); > > + printf("Global recursive lock taken on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_lock(&tlr); > > + printf("Global recursive lock taken on core %u - count =3D %d\n", > > + id, tlr.count); > > + > > + printf("Hello from within recursive locks from core %u !\n", id); > > + > > + rte_ticketlock_recursive_unlock(&tlr); > > + printf("Global recursive lock released on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_unlock(&tlr); > > + printf("Global recursive lock released on core %u - count =3D %d\n", > > + id, tlr.count); > > + rte_ticketlock_recursive_unlock(&tlr); > > + printf("Global recursive lock released on core %u - count =3D %d\n", > > + id, tlr.count); > > + > > + return 0; > > +} > > + > > +static rte_ticketlock_t lk =3D RTE_TICKETLOCK_INITIALIZER; static > > +uint64_t lock_count[RTE_MAX_LCORE] =3D {0}; > > + > > +#define TIME_MS 100 > > + > > +static int > > +load_loop_fn(void *func_param) > > +{ > > + uint64_t time_diff =3D 0, begin; > > + uint64_t hz =3D rte_get_timer_hz(); > > + uint64_t lcount =3D 0; > > + const int use_lock =3D *(int *)func_param; > > + const unsigned int lcore =3D rte_lcore_id(); > > + > > + /* wait synchro for slaves */ > > + if (lcore !=3D rte_get_master_lcore()) > > + while (rte_atomic32_read(&synchro) =3D=3D 0) > > + ; > > + > > + begin =3D rte_get_timer_cycles(); > > + while (time_diff < hz * TIME_MS / 1000) { > > + if (use_lock) > > + rte_ticketlock_lock(&lk); > > + lcount++; > > + if (use_lock) > > + rte_ticketlock_unlock(&lk); > > + /* delay to make lock duty cycle slighlty realistic */ >=20 > Probably better to do here the same as in test spinlock patches: > - remove delay_us() > - move > time_diff =3D rte_get_timer_cycles() - begin; out of the loop and repo= rt > aggregate cycles. >=20 Will do the same as test spinlock patches in next version. > > + rte_delay_us(1); > > + time_diff =3D rte_get_timer_cycles() - begin; > > + } > > + lock_count[lcore] =3D lcount; > > + return 0; > > +} > > + > > +static int > > +test_ticketlock_perf(void) > > +{ > > + unsigned int i; > > + uint64_t total =3D 0; > > + int lock =3D 0; > > + const unsigned int lcore =3D rte_lcore_id(); > > + > > + printf("\nTest with no lock on single core...\n"); > > + load_loop_fn(&lock); > > + printf("Core [%u] count =3D %"PRIu64"\n", lcore, lock_count[lcore]); > > + memset(lock_count, 0, sizeof(lock_count)); > > + > > + printf("\nTest with lock on single core...\n"); > > + lock =3D 1; > > + load_loop_fn(&lock); > > + printf("Core [%u] count =3D %"PRIu64"\n", lcore, lock_count[lcore]); > > + memset(lock_count, 0, sizeof(lock_count)); > > + > > + printf("\nTest with lock on %u cores...\n", rte_lcore_count()); > > + > > + /* Clear synchro and start slaves */ > > + rte_atomic32_set(&synchro, 0); > > + rte_eal_mp_remote_launch(load_loop_fn, &lock, SKIP_MASTER); > > + > > + /* start synchro and launch test on master */ > > + rte_atomic32_set(&synchro, 1); > > + load_loop_fn(&lock); > > + > > + rte_eal_mp_wait_lcore(); > > + > > + RTE_LCORE_FOREACH(i) { > > + printf("Core [%u] count =3D %"PRIu64"\n", i, lock_count[i]); > > + total +=3D lock_count[i]; > > + } > > + > > + printf("Total count =3D %"PRIu64"\n", total); > > + > > + return 0; > > +} > > + > > +/* > > + * Use rte_ticketlock_trylock() to trylock a ticketlock object, > > + * If it could not lock the object successfully, it would > > + * return immediately and the variable of "count" would be > > + * increased by one per times. the value of "count" could be > > + * checked as the result later. > > + */ > > +static int > > +test_ticketlock_try(__attribute__((unused)) void *arg) { > > + if (rte_ticketlock_trylock(&tl_try) =3D=3D 0) { > > + rte_ticketlock_lock(&tl); > > + count++; > > + rte_ticketlock_unlock(&tl); > > + } > > + > > + return 0; > > +} > > + > > + > > +/* > > + * Test rte_eal_get_lcore_state() in addition to ticketlocks > > + * as we have "waiting" then "running" lcores. > > + */ > > +static int > > +test_ticketlock(void) > > +{ > > + int ret =3D 0; > > + int i; > > + > > + /* slave cores should be waiting: print it */ > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + printf("lcore %d state: %d\n", i, > > + (int) rte_eal_get_lcore_state(i)); > > + } > > + > > + rte_ticketlock_init(&tl); > > + rte_ticketlock_init(&tl_try); > > + rte_ticketlock_recursive_init(&tlr); > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_ticketlock_init(&tl_tab[i]); > > + } > > + > > + rte_ticketlock_lock(&tl); > > + > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_ticketlock_lock(&tl_tab[i]); > > + rte_eal_remote_launch(test_ticketlock_per_core, NULL, i); > > + } > > + > > + /* slave cores should be busy: print it */ > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + printf("lcore %d state: %d\n", i, > > + (int) rte_eal_get_lcore_state(i)); > > + } > > + rte_ticketlock_unlock(&tl); > > + > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_ticketlock_unlock(&tl_tab[i]); > > + rte_delay_ms(10); > > + } > > + > > + rte_eal_mp_wait_lcore(); > > + > > + rte_ticketlock_recursive_lock(&tlr); > > + > > + /* > > + * Try to acquire a lock that we already own > > + */ > > + if (!rte_ticketlock_recursive_trylock(&tlr)) { > > + printf("rte_ticketlock_recursive_trylock failed on a lock that " > > + "we already own\n"); > > + ret =3D -1; > > + } else > > + rte_ticketlock_recursive_unlock(&tlr); > > + > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_eal_remote_launch(test_ticketlock_recursive_per_core, > > + NULL, i); > > + } > > + rte_ticketlock_recursive_unlock(&tlr); > > + rte_eal_mp_wait_lcore(); > > + > > + /* > > + * Test if it could return immediately from try-locking a locked obje= ct. > > + * Here it will lock the ticketlock object first, then launch all the > > + * slave lcores to trylock the same ticketlock object. > > + * All the slave lcores should give up try-locking a locked object an= d > > + * return immediately, and then increase the "count" initialized with > > + * zero by one per times. > > + * We can check if the "count" is finally equal to the number of all > > + * slave lcores to see if the behavior of try-locking a locked > > + * ticketlock object is correct. > > + */ > > + if (rte_ticketlock_trylock(&tl_try) =3D=3D 0) > > + return -1; > > + > > + count =3D 0; > > + RTE_LCORE_FOREACH_SLAVE(i) { > > + rte_eal_remote_launch(test_ticketlock_try, NULL, i); > > + } > > + rte_eal_mp_wait_lcore(); > > + rte_ticketlock_unlock(&tl_try); > > + if (rte_ticketlock_is_locked(&tl)) { > > + printf("ticketlock is locked but it should not be\n"); > > + return -1; > > + } > > + rte_ticketlock_lock(&tl); > > + if (count !=3D (rte_lcore_count() - 1)) > > + ret =3D -1; > > + > > + rte_ticketlock_unlock(&tl); > > + > > + /* > > + * Test if it can trylock recursively. > > + * Use rte_ticketlock_recursive_trylock() to check if it can lock > > + * a ticketlock object recursively. Here it will try to lock a > > + * ticketlock object twice. > > + */ > > + if (rte_ticketlock_recursive_trylock(&tlr) =3D=3D 0) { > > + printf("It failed to do the first ticketlock_recursive_trylock " > > + "but it should able to do\n"); > > + return -1; > > + } > > + if (rte_ticketlock_recursive_trylock(&tlr) =3D=3D 0) { > > + printf("It failed to do the second ticketlock_recursive_trylock > " > > + "but it should able to do\n"); > > + return -1; > > + } > > + rte_ticketlock_recursive_unlock(&tlr); > > + rte_ticketlock_recursive_unlock(&tlr); > > + > > + if (test_ticketlock_perf() < 0) > > + return -1; > > + > > + return ret; > > +} > > + > > +REGISTER_TEST_COMMAND(ticketlock_autotest, test_ticketlock); > > -- > > 2.7.4