From patchwork Mon May 10 19:40:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavan Nikhilesh Bhagavatula X-Patchwork-Id: 93114 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C41E1A0C4E; Mon, 10 May 2021 21:40:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3E55F40140; Mon, 10 May 2021 21:40:19 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by mails.dpdk.org (Postfix) with ESMTP id D46DB4003E; Mon, 10 May 2021 21:40:17 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 14AJZPwN029486; Mon, 10 May 2021 12:40:16 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=7efDC48kBp8n08vkkR3af57UYRmIAf9LJLF+48DBOt8=; b=DjBwgdkOTzkU/RLvJnH43FMORX+s+0w9EG3W05KPrZN8ihelvU1KkMv3cpNJpwi8z/1R ElTV+Hc2dTVL3gq7G50+8y9llMLv+fN0yFy8YrQordOXLutNWcVklHSIeyWAEVDKuUPj Qlf2k6C21eZoeMyWjYItb8hZkV9+pM6e7gdAJsaJAeK4I3pjO1WskuxMMRN5oRJUr1K6 cHBpNxJRocy1MkNpTNEUwog3518de2LzvHXLKvAHk+7kekFnsn0c9v6FotalbgrjqCxR KPBwfGZdq8aEv3cwYEeFHNXEwC/pnvJ7MDWKFTEczqWRydlJfBeD4CrFsKpbNi+OW5WK fw== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0b-0016f401.pphosted.com with ESMTP id 38eygyadhx-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 10 May 2021 12:40:16 -0700 Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 10 May 2021 12:40:14 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Mon, 10 May 2021 12:40:14 -0700 Received: from BG-LT7430.marvell.com (BG-LT7430.marvell.com [10.28.177.176]) by maili.marvell.com (Postfix) with ESMTP id ECA336737B; Mon, 10 May 2021 12:40:11 -0700 (PDT) From: To: CC: , Pavan Nikhilesh , , Tyler Retzlaff Date: Tue, 11 May 2021 01:10:06 +0530 Message-ID: <20210510194008.403-1-pbhagavatula@marvell.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210510140214.2627-1-pbhagavatula@marvell.com> References: <20210510140214.2627-1-pbhagavatula@marvell.com> MIME-Version: 1.0 X-Proofpoint-GUID: qCSKO03YMLqOenym0xFuodDgIfQb0MTa X-Proofpoint-ORIG-GUID: qCSKO03YMLqOenym0xFuodDgIfQb0MTa X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.761 definitions=2021-05-10_12:2021-05-10, 2021-05-10 signatures=0 Subject: [dpdk-dev] [PATCH v3 1/2] eal: fix side effects in align mul macros X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" From: Pavan Nikhilesh Avoid expanding parameters inside the macro multiple times. For example: RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ); Here rte_rdtsc() call is expanded multiple times in the macro causing it to return different values that leads to unintended side effects. Also, update common_autotest to detect macro side effects. Fixes: f56e551485d5 ("eal: add macro to align value to the nearest multiple") Cc: stable@dpdk.org Signed-off-by: Tyler Retzlaff Signed-off-by: David Marchand Signed-off-by: Pavan Nikhilesh --- v3 Changes: - Fix examples/bbdev_app compilation. v2 Changes: - Fix mlx5 compilation. app/test/test_common.c | 27 ++++++++++++++++++++++++++- lib/eal/include/rte_common.h | 28 +++++++++++++++++++--------- 2 files changed, 45 insertions(+), 10 deletions(-) -- 2.17.1 diff --git a/app/test/test_common.c b/app/test/test_common.c index 12bd1cad90..0dbb87e741 100644 --- a/app/test/test_common.c +++ b/app/test/test_common.c @@ -18,6 +18,13 @@ {printf(x "() test failed!\n");\ return -1;} +static uintptr_t callcount; +static uintptr_t +inc_callcount(void) +{ + return callcount++; +} + /* this is really a sanity check */ static int test_macros(int __rte_unused unused_parm) @@ -29,7 +36,19 @@ test_macros(int __rte_unused unused_parm) {printf(#x "() test failed!\n");\ return -1;} +#define TEST_SIDE_EFFECT_2(macro, type1, type2) \ + do { \ + callcount = 0; \ + (void)macro((type1)inc_callcount(), (type2)inc_callcount()); \ + if (callcount != 2) { \ + printf(#macro " has side effects: callcount=%u\n", \ + (unsigned int)callcount); \ + ret = -1; \ + } \ + } while (0) + uintptr_t unused = 0; + int ret = 0; RTE_SET_USED(unused); @@ -47,7 +66,13 @@ test_macros(int __rte_unused unused_parm) if (strncmp(RTE_STR(test), "test", sizeof("test"))) FAIL_MACRO(RTE_STR); - return 0; + TEST_SIDE_EFFECT_2(RTE_PTR_ADD, void *, size_t); + TEST_SIDE_EFFECT_2(RTE_PTR_DIFF, void *, void *); + TEST_SIDE_EFFECT_2(RTE_PTR_SUB, void *, size_t); + TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_CEIL, unsigned int, unsigned int); + TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_FLOOR, unsigned int, unsigned int); + TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_NEAR, unsigned int, unsigned int); + return ret; } static int diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h index d5a32c66a5..a142596587 100644 --- a/lib/eal/include/rte_common.h +++ b/lib/eal/include/rte_common.h @@ -329,27 +329,37 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void) * value will be of the same type as the first parameter and will be no lower * than the first parameter. */ -#define RTE_ALIGN_MUL_CEIL(v, mul) \ - ((((v) + (typeof(v))(mul) - 1) / ((typeof(v))(mul))) * (typeof(v))(mul)) +#define RTE_ALIGN_MUL_CEIL(v, mul) \ + __extension__({ \ + typeof(v) _vc = (v); \ + typeof(v) _mc = (mul); \ + ((_vc + _mc - 1) / _mc) * _mc; \ + }) /** * Macro to align a value to the multiple of given value. The resultant * value will be of the same type as the first parameter and will be no higher * than the first parameter. */ -#define RTE_ALIGN_MUL_FLOOR(v, mul) \ - (((v) / ((typeof(v))(mul))) * (typeof(v))(mul)) +#define RTE_ALIGN_MUL_FLOOR(v, mul) \ + __extension__({ \ + typeof(v) _vf = (v); \ + typeof(v) _mf = (mul); \ + (_vf / _mf) * _mf; \ + }) /** * Macro to align value to the nearest multiple of the given value. * The resultant value might be greater than or less than the first parameter * whichever difference is the lowest. */ -#define RTE_ALIGN_MUL_NEAR(v, mul) \ - ({ \ - typeof(v) ceil = RTE_ALIGN_MUL_CEIL(v, mul); \ - typeof(v) floor = RTE_ALIGN_MUL_FLOOR(v, mul); \ - (ceil - (v)) > ((v) - floor) ? floor : ceil; \ +#define RTE_ALIGN_MUL_NEAR(v, mul) \ + __extension__({ \ + typeof(v) _v = (v); \ + typeof(v) _m = (mul); \ + typeof(v) floor = RTE_ALIGN_MUL_FLOOR(_v, _m); \ + typeof(v) ceil = RTE_ALIGN_MUL_CEIL(_v, _m); \ + (ceil - _v) > (_v - floor) ? floor : ceil; \ }) /**