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 4989BA04B1 for ; Wed, 23 Sep 2020 14:47:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 15B8B1D9ED; Wed, 23 Sep 2020 14:47:17 +0200 (CEST) Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id B8BC11D948 for ; Wed, 23 Sep 2020 14:47:14 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20200923124713euoutp01ca0077b324e86ec30b51c1d876d161bc~3anLUiuHH1126811268euoutp01o for ; Wed, 23 Sep 2020 12:47:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20200923124713euoutp01ca0077b324e86ec30b51c1d876d161bc~3anLUiuHH1126811268euoutp01o DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1600865234; bh=+HLeItKvBijaoMAHMHXb9VBBNAdLJvNi8pjw60YJDj0=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=SFSYiRVo0amrU+Stglwut2icXk2hlngYoBNbw5DsOBtVmmDO+dqrNNCpEVTFf/Q31 srZt6Il0PVT1vuiD8ewBVC9OpHbH9n1xyKcwsx5hFe9veuCq7bcCQ5Pn38F2PSdQSj T6m1/FNUK2UZxXX4JE9C3J1tO+ysAeCmNp23Xtgg= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20200923124713eucas1p15ae3cb0a6a4d8e89def770e1ce40b659~3anK_dwVh0810508105eucas1p1D; Wed, 23 Sep 2020 12:47:13 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 02.0C.06318.1D34B6F5; Wed, 23 Sep 2020 13:47:13 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20200923124713eucas1p2cc251699a50d574197518fbe5e661703~3anKjYbGt2162221622eucas1p23; Wed, 23 Sep 2020 12:47:13 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20200923124713eusmtrp21aa2efe22511148750e91a6970f895eb~3anKiyWW20164001640eusmtrp27; Wed, 23 Sep 2020 12:47:13 +0000 (GMT) X-AuditID: cbfec7f5-371ff700000018ae-32-5f6b43d1c714 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id F9.F3.06017.1D34B6F5; Wed, 23 Sep 2020 13:47:13 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20200923124712eusmtip24b42df5a6bb268ce2da00c8ab6fb2ac0~3anJy-xiA0440204402eusmtip2s; Wed, 23 Sep 2020 12:47:12 +0000 (GMT) To: Honnappa Nagarahalli Cc: David Hunt , Bruce Richardson , "dev@dpdk.org" , "stable@dpdk.org" , nd , "\"'Lukasz Wojciechowski'\"," From: Lukasz Wojciechowski Message-ID: Date: Wed, 23 Sep 2020 14:47:11 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrOKsWRmVeSWpSXmKPExsWy7djPc7oXnbPjDc4fVra4screom/SRyaL d5+2M1nMfNrCbvGsZx2jxZnlPcwW/zr+sDuwe6yZt4bR49eCpawei/e8ZPI4+G4PUwBLFJdN SmpOZllqkb5dAlfG0uYu5oLZ1hWrn39nbWB8ptfFyMkhIWAicWJDN3MXIxeHkMAKRom5x5ay QDhfGCV+/zrMCuF8ZpS43fqBBaZly6vvUFXLGSV29j2Fct4ySjx70ccKUiUsECGxdcFuNhBb RMBcYuKeXWBFzAIfGSX+rjgNVsQmYCtxZOZXMJtXwE3i+bvVYDaLgKrEiVdbmEFsUYE4iWOn HrFA1AhKnJz5BMzmFIiV2LX+PdgCZgF5ieats5khbHGJW0/mM4EskxDYxC6x7vN2doi7XSR2 L/0FZQtLvDq+BcqWkfi/E6ZhG6PE1d8/GSGc/YwS13tXQFVZSxz+9xtoHQfQCk2J9bv0QUwJ AUeJQ3uSIEw+iRtvBSFu4JOYtG06M0SYV6KjTQhihp7E056pjDBb/6x9wjKBUWkWks9mIflm FpJvZiGsXcDIsopRPLW0ODc9tdg4L7Vcrzgxt7g0L10vOT93EyMw+Zz+d/zrDsZ9f5IOMQpw MCrx8HLoZscLsSaWFVfmHmKU4GBWEuF1Ons6Tog3JbGyKrUoP76oNCe1+BCjNAeLkjiv8aKX sUIC6YklqdmpqQWpRTBZJg5OqQZGmcoNrr/2iXJ2/HjImxAdMj/8k9H3U3P77/nYSmvdOnXB z9k9cfq9i3tn/fxR7+r8dEX7Omuxs4t+2/9sn8Ed9qy5sF+D08mn75GIorZz7utJTY+PPFZ9 dPje+enaGYcOeuqf4Vtgu9LHbEPZOV3RFymLJ+sU8CxPbufQKCro85a7pTad1yFeiaU4I9FQ i7moOBEADnEtZDoDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrJIsWRmVeSWpSXmKPExsVy+t/xe7oXnbPjDTb+ZrW4screom/SRyaL d5+2M1nMfNrCbvGsZx2jxZnlPcwW/zr+sDuwe6yZt4bR49eCpawei/e8ZPI4+G4PUwBLlJ5N UX5pSapCRn5xia1StKGFkZ6hpYWekYmlnqGxeayVkamSvp1NSmpOZllqkb5dgl7G0uYu5oLZ 1hWrn39nbWB8ptfFyMkhIWAiseXVd5YuRi4OIYGljBKdL74ydzFyACVkJD5cEoCoEZb4c62L DaLmNaPE4V1fGUESwgIRElsX7GYDsUUEzCUm7tkFNohZ4COjxO8pM1ghOp4wSbza9psVpIpN wFbiyMyvYDavgJvE83erwWwWAVWJE6+2MIPYogJxEmd6XrBB1AhKnJz5hAXE5hSIldi1/j1Y nFnATGLe5ofMELa8RPPW2VC2uMStJ/OZJjAKzULSPgtJyywkLbOQtCxgZFnFKJJaWpybnlts pFecmFtcmpeul5yfu4kRGG3bjv3csoOx613wIUYBDkYlHl4O3ex4IdbEsuLK3EOMEhzMSiK8 TmdPxwnxpiRWVqUW5ccXleakFh9iNAV6biKzlGhyPjAR5JXEG5oamltYGpobmxubWSiJ83YI HIwREkhPLEnNTk0tSC2C6WPi4JRqYPQ4K29fqtR8Vn9ehWuoQE72hw7Zfa9X5GyvdfrKUxXw iDv8xY3zng+ClZJVz1VsEtpbcbD+Z3fSTH6zCWuqDNgfBc8P3Halfk3TxOX1tn0fN+suePHv zcE30/eVf6uemHCg6SGHlUq9s/nEFl/pRRPyOZ5cY3rxUGQZ65+ZrNbHQhOVFhZwzlJiKc5I NNRiLipOBABVH6iuzAIAAA== X-CMS-MailID: 20200923124713eucas1p2cc251699a50d574197518fbe5e661703 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200923014719eucas1p2f26000109e86a649796e902c30e58bf0 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200923014719eucas1p2f26000109e86a649796e902c30e58bf0 References: <20200915193449.13310-1-l.wojciechow@partner.samsung.com> <20200923014713.16932-1-l.wojciechow@partner.samsung.com> <20200923014713.16932-3-l.wojciechow@partner.samsung.com> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2 2/8] app/test: synchronize statistics between lcores 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" W dniu 23.09.2020 o 06:30, Honnappa Nagarahalli pisze: > > >> 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. > What exactly do you mean by invalid values? Can you elaborate? I mean values that shouldn't be there which are obviously not related to number of packets handled by workers. I reverted the patch and run stress tests. Failures without this patch look like these: === Test flush fn with worker shutdown (burst) === Worker 0 handled 0 packets Worker 1 handled 0 packets Worker 2 handled 0 packets Worker 3 handled 0 packets Worker 4 handled 32 packets Worker 5 handled 0 packets Worker 6 handled 6 packets Line 519: Error, not all packets flushed. Expected 32, got 38 Test Failed or: === Sanity test of worker shutdown === Worker 0 handled 0 packets Worker 1 handled 0 packets Worker 2 handled 0 packets Worker 3 handled 0 packets Worker 4 handled 0 packets Worker 5 handled 64 packets Worker 6 handled 149792 packets Line 466: Error, not all packets flushed. Expected 64, got 149856 Test Failed The 6 or 149792 packets reported by worker 6 were never sent to or processed by the workers. >> 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 >> --- >> app/test/test_distributor.c | 39 ++++++++++++++++++++++++------------- >> 1 file changed, 26 insertions(+), 13 deletions(-) >> >> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c index >> 35b25463a..0e49e3714 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); >> return count; >> } >> >> @@ -52,6 +53,7 @@ static inline void >> clear_packet_count(void) >> { >> memset(&worker_stats, 0, sizeof(worker_stats)); >> + rte_atomic_thread_fence(__ATOMIC_RELEASE); >> } >> >> /* this is the basic worker function for sanity test @@ -72,13 +74,13 @@ >> handle_work(void *arg) >> num = rte_distributor_get_pkt(db, id, buf, buf, num); >> 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; >> @@ -134,7 +136,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 */ @@ -159,7 +162,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"); >> } >> >> @@ -185,7 +190,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); @@ -280,15 >> +286,17 @@ handle_work_with_free_mbufs(void *arg) >> buf[i] = NULL; >> num = rte_distributor_get_pkt(d, id, buf, buf, num); >> 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, buf, num); >> } >> - 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; >> } >> @@ -363,8 +371,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, >> @@ -379,10 +388,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. >> @@ -394,10 +404,11 @@ handle_work_for_shutdown_test(void *arg) >> id, buf, buf, num); >> >> while (!quit) { >> - worker_stats[id].handled_packets += num; >> count += num; >> rte_pktmbuf_free(pkt); >> num = rte_distributor_get_pkt(d, id, buf, buf, num); >> + >> __atomic_fetch_add(&worker_stats[id].handled_packets, >> + num, __ATOMIC_ACQ_REL); >> } >> returned = rte_distributor_return_pkt(d, >> id, buf, num); >> @@ -461,7 +472,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. " >> @@ -514,7 +526,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