All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] eal: fix side effects in align mul macros
@ 2021-05-09 17:18 pbhagavatula
  2021-05-09 17:18 ` [dpdk-dev] [PATCH 2/2] eal: fix side effects in ptr align macros pbhagavatula
  2021-05-10 14:02 ` [dpdk-dev] [PATCH v2 1/2] eal: fix side effects in align mul macros pbhagavatula
  0 siblings, 2 replies; 19+ messages in thread
From: pbhagavatula @ 2021-05-09 17:18 UTC (permalink / raw)
  To: david.marchand; +Cc: dev, Pavan Nikhilesh, stable, Tyler Retzlaff

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

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 <roretzla@linux.microsoft.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 Retainded original author signoff, split patches as some of the changes
 involved in modifying drivers.

 app/test/test_common.c       | 27 ++++++++++++++++++++++++++-
 lib/eal/include/rte_common.h | 28 +++++++++++++++++++---------
 2 files changed, 45 insertions(+), 10 deletions(-)

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;                     \
 	})

 /**
--
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-07-14 16:37 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-09 17:18 [dpdk-dev] [PATCH 1/2] eal: fix side effects in align mul macros pbhagavatula
2021-05-09 17:18 ` [dpdk-dev] [PATCH 2/2] eal: fix side effects in ptr align macros pbhagavatula
2021-05-09 19:39   ` Stephen Hemminger
2021-05-10  9:50     ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-05-10 13:34       ` David Marchand
2021-05-10 14:02 ` [dpdk-dev] [PATCH v2 1/2] eal: fix side effects in align mul macros pbhagavatula
2021-05-10 14:02   ` [dpdk-dev] [PATCH v2 2/2] eal: fix side effects in ptr align macros pbhagavatula
2021-05-10 19:40   ` [dpdk-dev] [PATCH v3 1/2] eal: fix side effects in align mul macros pbhagavatula
2021-05-10 19:40     ` [dpdk-dev] [PATCH v3 2/2] eal: fix side effects in ptr align macros pbhagavatula
2021-05-11  2:12       ` Wang, Haiyue
2021-05-11  8:44         ` Pavan Nikhilesh Bhagavatula
2021-05-12  1:13           ` Wang, Haiyue
2023-07-03 18:55     ` [PATCH] eal: avoid issues in macro expansion of alignment Stephen Hemminger
2023-07-03 23:23       ` [PATCH v2] " Stephen Hemminger
2023-07-04  8:43         ` Morten Brørup
2023-07-04 16:00           ` Stephen Hemminger
2023-07-04 22:16             ` Morten Brørup
2023-07-14 16:37               ` Stephen Hemminger
2023-07-04 16:01     ` [dpdk-dev] [PATCH v3 1/2] eal: fix side effects in align mul macros Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.