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 A26C4A04DC for ; Sat, 17 Oct 2020 05:24:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2B1E8E2D1; Sat, 17 Oct 2020 05:24:11 +0200 (CEST) Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id EAE7CE2C3 for ; Sat, 17 Oct 2020 05:24:07 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20201017032356euoutp013ad82c1fe89444b3b86ae309e4b95d0f~_qaNWLvUd3000330003euoutp01J for ; Sat, 17 Oct 2020 03:23:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20201017032356euoutp013ad82c1fe89444b3b86ae309e4b95d0f~_qaNWLvUd3000330003euoutp01J DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1602905036; bh=rUaLg1FCctUs/NJZbfIMXEiQz8k1zu1PIq8DRpfrAis=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=Y8Li3g+/QmHnMx8Be5fUAowrddCs3UWOTWESu6JQy7UTyIJRmehIwHw6AGOWLkCwx OVogLeG28p9kBWzuFifl35h0fFvpKIKfqcfXC0GBJYKxqy4r48lgk1XdTJ0+g9HNbi 9RAj/EISXieMYP94CW3XO81BO5uzuSyLUHhDInGk= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20201017032350eucas1p10c4f508cb08144f2f6eabf8c2c8aeb99~_qaH157f32571725717eucas1p1-; Sat, 17 Oct 2020 03:23:50 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 94.F5.06318.6C36A8F5; Sat, 17 Oct 2020 04:23:50 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20201017032349eucas1p1e4c3ff31ce5b59bbcb2f3235aa7dbfdb~_qaG7n1QH2571825718eucas1p1A; Sat, 17 Oct 2020 03:23:49 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20201017032349eusmtrp286de70f6f81a7e7dae9f423d3c829436~_qaG7D0Ap2813028130eusmtrp27; Sat, 17 Oct 2020 03:23:49 +0000 (GMT) X-AuditID: cbfec7f5-371ff700000018ae-91-5f8a63c63f5a Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 85.A9.06314.5C36A8F5; Sat, 17 Oct 2020 04:23:49 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20201017032348eusmtip1ae40425e285af7333094be35717feca8~_qaGPbXtl3071330713eusmtip1h; Sat, 17 Oct 2020 03:23:48 +0000 (GMT) To: Honnappa Nagarahalli , David Hunt , Bruce Richardson Cc: "dev@dpdk.org" , "stable@dpdk.org" , nd , "\"'Lukasz Wojciechowski'\"," From: Lukasz Wojciechowski Message-ID: <71e69517-a4f6-eb86-8328-ef696824d8c6@partner.samsung.com> Date: Sat, 17 Oct 2020 05:23:47 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrBKsWRmVeSWpSXmKPExsWy7djP87rHkrviDT6vELW4screom/SRyaL d5+2M1nMfNrCbvGsZx2jxZnlPcwW/zr+sDuwe6yZt4bR49eCpawei/e8ZPI4+G4PUwBLFJdN SmpOZllqkb5dAldG48JtLAWTrSt62g+yNzDO1u9i5OSQEDCRuNvWzt7FyMUhJLCCUeLn9hYW COcLo8S21feYIJzPjBKnls5lgmlpaX8L1bKcUaKh+TkzSEJI4C2jxPe9wiC2sECUxO3+LjaQ IhGBbkaJSWfPMII4zAJzGSV2TmtkBKliE7CVODLzKyuIzSvgJrH+4U4WEJtFQFXi9JydYHFR gTiJCRtbWCBqBCVOznwCZnMKxEocnnEFrIZZQF6ieetsZghbXOLWk/lgd0sIbGKXOLhmKQvE 3S4SDbsmQ/0gLPHq+BZ2CFtG4v9OmIZtjBJXf/9khHD2M0pc710BVWUtcfjfb6CHOIBWaEqs 3wUNP0eJnt5LTCBhCQE+iRtvBSGO4JOYtG06M0SYV6KjTQiiWk/iac9URpi1f9Y+YZnAqDQL yWuzkLwzC8k7sxD2LmBkWcUonlpanJueWmycl1quV5yYW1yal66XnJ+7iRGYfk7/O/51B+O+ P0mHGAU4GJV4eDmWdsYLsSaWFVfmHmKU4GBWEuF1Ons6Tog3JbGyKrUoP76oNCe1+BCjNAeL kjiv8aKXsUIC6YklqdmpqQWpRTBZJg5OqQZGp2sGU1VypRdfjv19r0P2EV9XhUjsLP01eg8S D72o/fxhrn9xvtu8F3tvhfIVvcie+SQ1Y9n6qZ9yjjAEF2dOi1ZaeHf91bsfjoZsFDz/OO/B nhP7G0q/x2wr+JWyrPiCM7vSkZ5FNb5Rb6Iaajeq8qgaLToX++jE2eaukr+Nk+K3zO+4yzAh UImlOCPRUIu5qDgRAKUpqdU7AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrBIsWRmVeSWpSXmKPExsVy+t/xu7pHk7viDQ43SFncWGVv0TfpI5PF u0/bmSxmPm1ht3jWs47R4szyHmaLfx1/2B3YPdbMW8Po8WvBUlaPxXteMnkcfLeHKYAlSs+m KL+0JFUhI7+4xFYp2tDCSM/Q0kLPyMRSz9DYPNbKyFRJ384mJTUnsyy1SN8uQS+jceE2loLJ 1hU97QfZGxhn63cxcnJICJhItLS/Ze9i5OIQEljKKLF2wWS2LkYOoISMxIdLAhA1whJ/rnWx QdS8ZpTYM2MVC0iNsECUxJSroiBxEYFeRontXXOZQBxmgbmMElMbnzKCdAsJPGGS+PVVB8Rm E7CVODLzKyuIzSvgJrH+4U4WEJtFQFXi9JydYHFRgTiJHxN72SBqBCVOznwCVsMpECtxeMYV sBpmATOJeZsfMkPY8hLNW2dD2eISt57MZ5rAKDQLSfssJC2zkLTMQtKygJFlFaNIamlxbnpu saFecWJucWleul5yfu4mRmCsbTv2c/MOxksbgw8xCnAwKvHwbljUGS/EmlhWXJl7iFGCg1lJ hNfp7Ok4Id6UxMqq1KL8+KLSnNTiQ4ymQM9NZJYSTc4HpoG8knhDU0NzC0tDc2NzYzMLJXHe DoGDMUIC6YklqdmpqQWpRTB9TBycUg2MRisWCbC1v36TqXj+kK4zu8rJiY4c/6IO7dzC+0BW 0rQ5y2vux/Wvrzk/e+95tZPj8B3G6t8Bq595SLBMvezXL+x3rY017+v84uNTBEuiEqT1pC83 Xshe/r+4pkGn9oAwGzvH7CfbdqufuPFo/tn7u5olQ6+55PYVH97ySWpSRXGeqf6hWRO5lViK MxINtZiLihMBgHdFB8sCAAA= X-CMS-MailID: 20201017032349eucas1p1e4c3ff31ce5b59bbcb2f3235aa7dbfdb X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20201010160528eucas1p2b9b8189aef51c18d116f97ccebf5719c X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20201010160528eucas1p2b9b8189aef51c18d116f97ccebf5719c References: <20201009220202.20834-1-l.wojciechow@partner.samsung.com> <20201010160508.19709-1-l.wojciechow@partner.samsung.com> <20201010160508.19709-7-l.wojciechow@partner.samsung.com> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v7 06/16] test/distributor: synchronize lcores statistics X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi Honnappa, W dniu 16.10.2020 o 07:13, Honnappa Nagarahalli pisze: > Hi Lukasz, > I see that in commit 8/16, the same code is changed again (updating the counters using the RELAXED memory order). It is better to pull the statistics changes from 8/16 into this commit. I reordered patches: "synchronize lcores statistics" and "fix freeing mbufs" to avoid changing same code. Many thanks for the review Lukasz > > Thanks, > Honnappa > >> -----Original Message----- >> From: dev On Behalf Of Lukasz Wojciechowski >> Sent: Saturday, October 10, 2020 11:05 AM >> To: David Hunt ; Bruce Richardson >> >> Cc: dev@dpdk.org; l.wojciechow@partner.samsung.com; stable@dpdk.org >> Subject: [dpdk-dev] [PATCH v7 06/16] test/distributor: synchronize lcores >> statistics >> >> Statistics of handled packets are cleared and read on main lcore, while they >> are increased in workers handlers on different lcores. >> >> Without synchronization occasionally showed invalid values. >> This patch uses atomic acquire/release mechanisms to synchronize. >> >> Fixes: c3eabff124e6 ("distributor: add unit tests") >> Cc: bruce.richardson@intel.com >> Cc: stable@dpdk.org >> >> Signed-off-by: Lukasz Wojciechowski >> Acked-by: David Hunt >> --- >> app/test/test_distributor.c | 43 +++++++++++++++++++++++++------------ >> 1 file changed, 29 insertions(+), 14 deletions(-) >> >> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c index >> 6cd7a2edd..838459392 100644 >> --- a/app/test/test_distributor.c >> +++ b/app/test/test_distributor.c >> @@ -43,7 +43,8 @@ total_packet_count(void) { >> unsigned i, count = 0; >> for (i = 0; i < worker_idx; i++) >> - count += worker_stats[i].handled_packets; >> + count += >> __atomic_load_n(&worker_stats[i].handled_packets, >> + __ATOMIC_ACQUIRE); > For ex: this line is changed in commit 8/16 as well. It is better to pull the changes from 8/16 to this commit. > >> return count; >> } >> >> @@ -51,7 +52,10 @@ total_packet_count(void) static inline void >> clear_packet_count(void) >> { >> - memset(&worker_stats, 0, sizeof(worker_stats)); >> + unsigned int i; >> + for (i = 0; i < RTE_MAX_LCORE; i++) >> + __atomic_store_n(&worker_stats[i].handled_packets, 0, >> + __ATOMIC_RELEASE); >> } >> >> /* this is the basic worker function for sanity test @@ -69,13 +73,13 @@ >> handle_work(void *arg) >> num = rte_distributor_get_pkt(db, id, buf, NULL, 0); >> while (!quit) { >> __atomic_fetch_add(&worker_stats[id].handled_packets, >> num, >> - __ATOMIC_RELAXED); >> + __ATOMIC_ACQ_REL); >> count += num; >> num = rte_distributor_get_pkt(db, id, >> buf, buf, num); >> } >> __atomic_fetch_add(&worker_stats[id].handled_packets, num, >> - __ATOMIC_RELAXED); >> + __ATOMIC_ACQ_REL); >> count += num; >> rte_distributor_return_pkt(db, id, buf, num); >> return 0; >> @@ -131,7 +135,8 @@ sanity_test(struct worker_params *wp, struct >> rte_mempool *p) >> >> for (i = 0; i < rte_lcore_count() - 1; i++) >> printf("Worker %u handled %u packets\n", i, >> - worker_stats[i].handled_packets); >> + __atomic_load_n(&worker_stats[i].handled_packets, >> + __ATOMIC_ACQUIRE)); >> printf("Sanity test with all zero hashes done.\n"); >> >> /* pick two flows and check they go correctly */ @@ -156,7 +161,9 >> @@ sanity_test(struct worker_params *wp, struct rte_mempool *p) >> >> for (i = 0; i < rte_lcore_count() - 1; i++) >> printf("Worker %u handled %u packets\n", i, >> - worker_stats[i].handled_packets); >> + __atomic_load_n( >> + &worker_stats[i].handled_packets, >> + __ATOMIC_ACQUIRE)); >> printf("Sanity test with two hash values done\n"); >> } >> >> @@ -182,7 +189,8 @@ sanity_test(struct worker_params *wp, struct >> rte_mempool *p) >> >> for (i = 0; i < rte_lcore_count() - 1; i++) >> printf("Worker %u handled %u packets\n", i, >> - worker_stats[i].handled_packets); >> + __atomic_load_n(&worker_stats[i].handled_packets, >> + __ATOMIC_ACQUIRE)); >> printf("Sanity test with non-zero hashes done\n"); >> >> rte_mempool_put_bulk(p, (void *)bufs, BURST); @@ -275,14 >> +283,16 @@ handle_work_with_free_mbufs(void *arg) >> >> num = rte_distributor_get_pkt(d, id, buf, NULL, 0); >> while (!quit) { >> - worker_stats[id].handled_packets += num; >> count += num; >> + __atomic_fetch_add(&worker_stats[id].handled_packets, >> num, >> + __ATOMIC_ACQ_REL); >> for (i = 0; i < num; i++) >> rte_pktmbuf_free(buf[i]); >> num = rte_distributor_get_pkt(d, id, buf, NULL, 0); >> } >> - worker_stats[id].handled_packets += num; >> count += num; >> + __atomic_fetch_add(&worker_stats[id].handled_packets, num, >> + __ATOMIC_ACQ_REL); >> rte_distributor_return_pkt(d, id, buf, num); >> return 0; >> } >> @@ -358,8 +368,9 @@ handle_work_for_shutdown_test(void *arg) >> /* wait for quit single globally, or for worker zero, wait >> * for zero_quit */ >> while (!quit && !(id == zero_id && zero_quit)) { >> - worker_stats[id].handled_packets += num; >> count += num; >> + __atomic_fetch_add(&worker_stats[id].handled_packets, >> num, >> + __ATOMIC_ACQ_REL); >> for (i = 0; i < num; i++) >> rte_pktmbuf_free(buf[i]); >> num = rte_distributor_get_pkt(d, id, buf, NULL, 0); @@ - >> 373,10 +384,11 @@ handle_work_for_shutdown_test(void *arg) >> >> total += num; >> } >> - worker_stats[id].handled_packets += num; >> count += num; >> returned = rte_distributor_return_pkt(d, id, buf, num); >> >> + __atomic_fetch_add(&worker_stats[id].handled_packets, num, >> + __ATOMIC_ACQ_REL); >> if (id == zero_id) { >> /* for worker zero, allow it to restart to pick up last packet >> * when all workers are shutting down. >> @@ -387,7 +399,8 @@ handle_work_for_shutdown_test(void *arg) >> num = rte_distributor_get_pkt(d, id, buf, NULL, 0); >> >> while (!quit) { >> - worker_stats[id].handled_packets += num; >> + >> __atomic_fetch_add(&worker_stats[id].handled_packets, >> + num, __ATOMIC_ACQ_REL); >> count += num; >> rte_pktmbuf_free(pkt); >> num = rte_distributor_get_pkt(d, id, buf, NULL, 0); >> @@ -454,7 +467,8 @@ sanity_test_with_worker_shutdown(struct >> worker_params *wp, >> >> for (i = 0; i < rte_lcore_count() - 1; i++) >> printf("Worker %u handled %u packets\n", i, >> - worker_stats[i].handled_packets); >> + __atomic_load_n(&worker_stats[i].handled_packets, >> + __ATOMIC_ACQUIRE)); >> >> if (total_packet_count() != BURST * 2) { >> printf("Line %d: Error, not all packets flushed. " >> @@ -507,7 +521,8 @@ test_flush_with_worker_shutdown(struct >> worker_params *wp, >> zero_quit = 0; >> for (i = 0; i < rte_lcore_count() - 1; i++) >> printf("Worker %u handled %u packets\n", i, >> - worker_stats[i].handled_packets); >> + __atomic_load_n(&worker_stats[i].handled_packets, >> + __ATOMIC_ACQUIRE)); >> >> if (total_packet_count() != BURST) { >> printf("Line %d: Error, not all packets flushed. " >> -- >> 2.17.1 -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com