DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH] eal/reciprocal: fix off by one when divisor is 32bit
@ 2019-10-23 15:06 Pavan Nikhilesh Bhagavatula
  2019-10-23 15:16 ` Aaron Conole
  0 siblings, 1 reply; 8+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2019-10-23 15:06 UTC (permalink / raw)
  To: Aaron Conole
  Cc: David Marchand, Jerin Jacob Kollanukkaran, Stephen Hemminger,
	dev, dpdk stable

>Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com> writes:
>
>>>> >
>>>> >Any review?
>>>> >
>>>> >Are we missing an update in the unit test to catch this issue?
>>>> >Thanks.
>>>>
>>>> We actually caught it in a unit test
>>>> >test_reciprocal_division
>>>
>>>We had this problem since the very start then.
>>>
>>>Both reciprocal_division and reciprocal_division_perf are in the
>"perf"
>>>list.
>>>Can they be promoted to the standard list?
>>
>> Both of these tests take >=5 min depending on the platform in use
>which is why
>> they are placed in perf tests.
>
>Is it possible to write a smaller set of tests that can be put in the
>standard list?
>

Currently we have MAX_ITERATIONS  (1ULL << 32) for both the tests
maybe we can reduce it to 1 << 16 for the normal case?

>>>
>>>
>>>--
>>>David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/reciprocal: fix off by one when divisor is 32bit
@ 2019-10-23 15:35 Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 8+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2019-10-23 15:35 UTC (permalink / raw)
  To: Aaron Conole
  Cc: David Marchand, Jerin Jacob Kollanukkaran, Stephen Hemminger,
	dev, dpdk stable

>Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com> writes:
>
>>>Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com> writes:
>>>
>>>>>> >
>>>>>> >Any review?
>>>>>> >
>>>>>> >Are we missing an update in the unit test to catch this issue?
>>>>>> >Thanks.
>>>>>>
>>>>>> We actually caught it in a unit test
>>>>>> >test_reciprocal_division
>>>>>
>>>>>We had this problem since the very start then.
>>>>>
>>>>>Both reciprocal_division and reciprocal_division_perf are in the
>>>"perf"
>>>>>list.
>>>>>Can they be promoted to the standard list?
>>>>
>>>> Both of these tests take >=5 min depending on the platform in use
>>>which is why
>>>> they are placed in perf tests.
>>>
>>>Is it possible to write a smaller set of tests that can be put in the
>>>standard list?
>>>
>>
>> Currently we have MAX_ITERATIONS  (1ULL << 32) for both the tests
>> maybe we can reduce it to 1 << 16 for the normal case?
>
>Sounds good to me.  If you submit a patch, we can look at the run time
>from the robot submission as well.
>

Yup, I'll send a patch.

>>>>>
>>>>>
>>>>>--
>>>>>David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/reciprocal: fix off by one when divisor is 32bit
@ 2019-10-23  8:44 Pavan Nikhilesh Bhagavatula
  2019-10-23 14:06 ` Aaron Conole
  0 siblings, 1 reply; 8+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2019-10-23  8:44 UTC (permalink / raw)
  To: David Marchand
  Cc: Jerin Jacob Kollanukkaran, Stephen Hemminger, dev, dpdk stable,
	Aaron Conole

>> >
>> >Any review?
>> >
>> >Are we missing an update in the unit test to catch this issue?
>> >Thanks.
>>
>> We actually caught it in a unit test
>> >test_reciprocal_division
>
>We had this problem since the very start then.
>
>Both reciprocal_division and reciprocal_division_perf are in the "perf"
>list.
>Can they be promoted to the standard list?

Both of these tests take >=5 min depending on the platform in use which is why
they are placed in perf tests.

>
>
>--
>David Marchand

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH] eal/reciprocal: fix off by one when divisor is 32bit
@ 2019-09-03 19:16 pbhagavatula
  2019-10-15  8:45 ` David Marchand
  2019-10-26 14:33 ` David Marchand
  0 siblings, 2 replies; 8+ messages in thread
From: pbhagavatula @ 2019-09-03 19:16 UTC (permalink / raw)
  To: stephen, jerinj; +Cc: dev, Pavan Nikhilesh, stable

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Fix off by one error in 64bit reciprocal division when divisor is 32bit.

Fixes: 6d45659eacb8 ("eal: add u64-bit variant for reciprocal divide")
Cc: stable@dpdk.org

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---

Example:
   Division failed, 17358247066007716387/244 =
   		expected 71140356827900476 result 71140356827900477
   Division failed, 17541123788887206374/41475 =
   		expected 422932460250444 result 422932460250445

 lib/librte_eal/common/rte_reciprocal.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/rte_reciprocal.c b/lib/librte_eal/common/rte_reciprocal.c
index f017d0c28..1c6d10e73 100644
--- a/lib/librte_eal/common/rte_reciprocal.c
+++ b/lib/librte_eal/common/rte_reciprocal.c
@@ -133,12 +133,15 @@ rte_reciprocal_value_u64(uint64_t d)
 {
 	struct rte_reciprocal_u64 R;
 	uint64_t m;
+	uint64_t r;
 	int l;

 	l = 63 - __builtin_clzll(d);

-	m = divide_128_div_64_to_64((1ULL << l), 0, d, NULL) << 1;
-	m = (1ULL << l) - d ? m + 2 : 1;
+	m = divide_128_div_64_to_64((1ULL << l), 0, d, &r) << 1;
+	if (r << 1 < r || r << 1 >= d)
+		m++;
+	m = (1ULL << l) - d ? m + 1 : 1;
 	R.m = m;

 	R.sh1 = l > 1 ? 1 : l;
--
2.23.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-10-26 14:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23 15:06 [dpdk-dev] [PATCH] eal/reciprocal: fix off by one when divisor is 32bit Pavan Nikhilesh Bhagavatula
2019-10-23 15:16 ` Aaron Conole
  -- strict thread matches above, loose matches on Subject: below --
2019-10-23 15:35 Pavan Nikhilesh Bhagavatula
2019-10-23  8:44 Pavan Nikhilesh Bhagavatula
2019-10-23 14:06 ` Aaron Conole
2019-09-03 19:16 pbhagavatula
2019-10-15  8:45 ` David Marchand
2019-10-26 14:33 ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).