Re: [PATCH NET-NEXT 02/10] time sync: generic infrastructure to map between time stamps generated by a time counter and system time

[Patrick Ohly <patrick.ohly@intel.com>]

On Mon, 2009-02-09 at 21:27 +0200, John Stultz wrote:
> On Mon, 2009-02-09 at 18:02 +0100, Patrick Ohly wrote:
> > Is this revision of the patch okay? How should the two patches get
> > included in the main kernel - via netdev-next-2.6?
>
> Small comment below, but otherwise it looks ok to me. I usually push
> patches through Andrew, so I'd probably go that way. But I'd leave it to
> Dave if he's comfortable pushing them to Linus.

As you don't mind, I suggest to push through Dave as part of the
complete patch series. That way we don't need to worry about
coordinating two subtrees.

> Acked-by: John Stultz <johnstul@us.ibm.com>

Thanks! Will add that.

> > +             /* ignore negative durations */
> > +             sample.duration_target = ktime_to_ns(ktime_sub(end, start));
> > +             if (sample.duration_target >= 0) {
> 
> You may also want to checking the bounds on the duration_target. If
> preemption hits and the values are too out of whack, the symetric delay
> assumption below might be quite invalid.
> 
> I guess the outliers removal probably covers this as well, but seems
> some sanity checking might be good.

That would require more information, like "duration_target is usually in
the xxx-yyy range". This could be determined based on past measurements
or the median of the current sample set, but is this really better than
the current "remove longest 25%"?

In practice I haven't seen such a problem, therefore I'd prefer to keep
the code simple and not change it. It was tested under load conditions
(both CPU and network).

-- 
Bye, Patrick Ohly
--  
Patrick.Ohly@gmx.de
http://www.estamos.de/