: Inline for time delta

[DCCP]: Inline for time delta

This provides a reusable time difference function which returns the difference in microseconds, as often used in the DCCP code.

Signed-off-by Gerrit Renker <[email protected]>

---
 net/dccp/dccp.h |    5 +++++
 1 file changed, 5 insertions(+)

net/dccp/dccp.h +0 -0

@@ -421,6 +421,11 @@ static inline suseconds_t timeval_delta(
        return secs * USEC_PER_SEC + usecs;

Neither place seems appropriate for above function, it is more specific than the ones in ktime.h. I am at a loss where else if not the DCCP code, where it is specifically used, to place it.

The name was chosen for consistency with timeval_delta(), which is the current function for microsecond time differences in net/dccp/dccp.h.

Quoting Ian McDonald:

|  > --- a/net/dccp/dccp.h
|  > +++ b/net/dccp/dccp.h
|  > @@ -421,6 +421,11 @@ static inline suseconds_t timeval_delta(
|  > return secs * USEC_PER_SEC + usecs;
|  > }
|  >
|  > +static inline s64 ktime_delta(ktime_t later, ktime_t earlier)
|  > +{
|  > + return ktime_to_us(ktime_sub(later, earlier));
|  > +}
|  > +
|  > static inline void timeval_add_usecs(struct timeval *tv,
|  > const suseconds_t usecs)
|  > {
|
|  Judging from patches I've seen rejected recently I'd say this should
|  be shifted into the same place as ktime_sub is defined. Also rename to
|  ktime_sub_us.
|

ktime_sub is defined in include/linux/ktime.h - the file only contains generic definitions, everything is in nanoseconds, struct timeval, or struct timespec.

The other place is include/linux/skbuff.h, where net_timedelta() is defined. This is probably due to the skb->tstamp field - as a utility function to be used with skbs.

Neither place seems appropriate for above function, it is more specific than the ones in ktime.h. I am at a loss where else if not the DCCP code, where it is specifically used, to place it.

The name was chosen for consistency with timeval_delta(), which is the current function for microsecond time differences in net/dccp/dccp.h.

Quoting Arnaldo Carvalho de Melo:

|  > The name was chosen for consistency with timeval_delta(), which is the current
|  > function for microsecond time differences in net/dccp/dccp.h.
|
|  I'm going to rename ktime_delta() to ktime_us_delta(), as it receives
|  two ktime_t values, but returns a delta in microseconds, not in
|  ktime_t.

I am going to change that in my tree as well, since I am busy updating with regard to most recent changes anyway.

Quoted 9 lines · Gerrit Renker

Quoting Arnaldo Carvalho de Melo:

|  > The name was chosen for consistency with timeval_delta(), which is the current
|  > function for microsecond time differences in net/dccp/dccp.h.
|
|  I'm going to rename ktime_delta() to ktime_us_delta(), as it receives
|  two ktime_t values, but returns a delta in microseconds, not in
|  ktime_t.

I am going to change that in my tree as well, since I am busy updating with regard to most recent changes anyway.

I just checked with Thomas Gleixner, the ktime_t guy and he is ok with adding ktime_us_delta() to ktime.h, which I have in my tree already.

I'm now staring at this:

   /* set the nominal send time for the next following packet */

(quoted diff) +2 -1

-        timeval_add_usecs(&hctx->ccid3hctx_t_nom, hctx->ccid3hctx_t_ipi);
+       hctx->ccid3hctx_t_nom = ktime_add_ns(hctx->ccid3hctx_t_nom,
+                                            hctx->ccid3hctx_t_ipi * 1000);

I'm thinking about just keeping t_ipi in nanoseconds to avoid doing the multiplies back and forth, but haven't fully looked at the other t_ipi uses, quick thoughts? Stupid idea?

Quoting Arnaldo Carvalho de Melo:

|  > I am going to change that in my tree as well, since I am busy updating with regard
|  > to most recent changes anyway.
|
|  I just checked with Thomas Gleixner, the ktime_t guy and he is ok with
|  adding ktime_us_delta() to ktime.h, which I have in my tree already.

Ok when that comes through, the patch can simply be dropped.

|  I'm now staring at this:
|   /* set the nominal send time for the next following packet */
|  - timeval_add_usecs(&hctx->ccid3hctx_t_nom, hctx->ccid3hctx_t_ipi);
|  + hctx->ccid3hctx_t_nom = ktime_add_ns(hctx->ccid3hctx_t_nom,
|  + hctx->ccid3hctx_t_ipi * 1000);
|
|  I'm thinking about just keeping t_ipi in nanoseconds to avoid doing
|  the multiplies back and forth, but haven't fully looked at the other
|  t_ipi uses, quick thoughts? Stupid idea?

Please keep it for the moment. The packet scheduler needs an overhaul in any case, so the above IMO is the simplest way of aligning the old code with the new interface.

Overhauling the packet scheduler will cost some work, but it is one of the next items that should be addressed when through with the current set of patches.