[PATCH 1/3]: Inline for time delta

[PATCH 1/3]: Inline for time delta

[Gerrit Renker]

[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 <gerrit@erg.abdn.ac.uk>
---
 net/dccp/dccp.h |    5 +++++
 1 file changed, 5 insertions(+)

--- 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)
 {

Re: [PATCH 1/3]: Inline for time delta

[Ian McDonald]

On 6/10/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> [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 <gerrit@erg.abdn.ac.uk>
> ---
>  net/dccp/dccp.h |    5 +++++
>  1 file changed, 5 insertions(+)
>
> --- 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.

I know that it sounds like I'm being a pain but I've seen a few like
this rejected recently.

Great to see patches moving ahead again.

Ian
-- 
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

Re: [PATCH 1/3]: Inline for time delta

[Gerrit Renker]

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.

Re: [PATCH 1/3]: Inline for time delta

[Ian McDonald]

On 6/11/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
>
> 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.
>
I'm fine with it myself. Just commenting on recent stuff in netdev.
Keep it as is for now if nowhere else is better.

Ian
-- 
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

Re: [PATCH 1/3]: Inline for time delta

[Arnaldo Carvalho de Melo]

On 6/11/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> 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.

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.

Don't worry, I'll fixup the later patches, after a while we get
everything sorted out.

- Arnaldo

Re: [PATCH 1/3]: Inline for time delta

[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.

Re: [PATCH 1/3]: Inline for time delta

[Arnaldo Carvalho de Melo]

On 6/16/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> 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 */
-        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?

- Arnaldo

Re: [PATCH 1/3]: Inline for time delta

[Gerrit Renker]

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.

Re: [PATCH 1/3]: Inline for time delta

[Arnaldo Carvalho de Melo]

On 6/16/07, Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
> On 6/16/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> > 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 */
> -        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?

Nah, will just add ktime_add_us to the ktime infrastructure (also
ACKed by Thomas).

- Arnaldo

Re: [PATCH 1/3]: Inline for time delta

[Arnaldo Carvalho de Melo]

On 6/16/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> 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.

OK! I'm just adding this to ktime.h:

static inline ktime_t ktime_add_us(const ktime_t kt, const u64 usec)
{
        return ktime_add_ns(kt, usec * 1000);
}

And will use it where needed.

- Arnaldo