2008-07-11 13:59:38

by David Howells

Subject: [PATCH] Fix dccp_timestamp()'s use of do_div()

Fix dccp_timestamps()'s use of do_div() on an s64 by making delta a u64
instead and dividing that.

Possibly this should be guarded lest the interval calculation turn up negative,
but the possible negativity of the result of the division ignored anyway, and,
indeed, should not happen.

This was introduced by patch 4c70f383e0c0273c4092c4efdb414be0966978b7.

Signed-off-by: David Howells <[email protected]>
---

net/dccp/timer.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)


diff --git a/net/dccp/timer.c b/net/dccp/timer.c
index 8703a79..27218c5 100644
--- a/net/dccp/timer.c
+++ b/net/dccp/timer.c
@@ -300,8 +300,10 @@ static ktime_t dccp_timestamp_seed;
*/
u32 dccp_timestamp(void)
{
- s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
+ u64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);

+ /* This will produce huge values if the current time should happen to
+ * be earlier than the initial timestamp, but that shouldn't happen */
do_div(delta, 10);
return delta;
}

2008-07-12 13:15:44

by Gerrit Renker

Subject: Re: [PATCH] Fix dccp_timestamp()'s use of do_div()

Quoting David Howells:
| Fix dccp_timestamps()'s use of do_div() on an s64 by making delta a u64
| instead and dividing that.
|
| Possibly this should be guarded lest the interval calculation turn up negative,
| but the possible negativity of the result of the division ignored anyway, and,
| indeed, should not happen.
|
| This was introduced by patch 4c70f383e0c0273c4092c4efdb414be0966978b7.
|
| Signed-off-by: David Howells <[email protected]>
| ---
|
| net/dccp/timer.c | 4 +++-
| 1 files changed, 3 insertions(+), 1 deletions(-)
|
|
| diff --git a/net/dccp/timer.c b/net/dccp/timer.c
| index 8703a79..27218c5 100644
| --- a/net/dccp/timer.c
| +++ b/net/dccp/timer.c
| @@ -300,8 +300,10 @@ static ktime_t dccp_timestamp_seed;
| */
| u32 dccp_timestamp(void)
| {
| - s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
| + u64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
|
| + /* This will produce huge values if the current time should happen to
| + * be earlier than the initial timestamp, but that shouldn't happen */
| do_div(delta, 10);
| return delta;
| }
|
I agree that the use of do_div() is problematic but I don't this is a
good solution. It would be better to use div_u64() with the same effect
as shown below (taken from a patch soon to be submitted).

--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -74,6 +74,9 @@ extern void dccp_time_wait(struct sock *
#define DCCP_TIMEWAIT_LEN (60 * HZ) /* how long to wait to destroy TIME-WAIT
* state, about 60 seconds */

+/* DCCP base time resolution - 10 microseconds (RFC 4340, 13.1 ... 13.3) */
+#define DCCP_TIME_RESOLUTION 10
+
/* RFC 1122, 4.2.3.1 initial RTO value */
#define DCCP_TIMEOUT_INIT ((unsigned)(3 * HZ))

--- a/net/dccp/timer.c
+++ b/net/dccp/timer.c
@@ -281,8 +281,7 @@ u32 dccp_timestamp(void)
{
s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);

- do_div(delta, 10);
- return delta;
+ return div_u64(delta, DCCP_TIME_RESOLUTION);
}
EXPORT_SYMBOL_GPL(dccp_timestamp);