[Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Ralf]

Hello,

i have found an error in the ipipe-patch above.
Here ist the error description:

Our system:
Linux 3.10.28 with xenomai 2.6 patch on a PowerPC MPC5200

Error description:
We use the xenomai-(posix-skin-)function clock_gettime(
CLOCK_HOST_REALTIME, ..) to read out our system time. Thus, we set up a
NTP clock that runs on an FPGA. Later, we compare the system time and
NTP time deviations. Unfortunately we recognize in any comparison with
the nanoseconds a deviation of at least 100 million nsecs. We then
started the xenomai test suite and get the following prints on our console:

root@hbm-000a0f(NFS):/$clocktest -D -C
42                                      
hostrt data area is
live                                                       
Sequence counter :
92323948                                                    
wall_time_sec    :
1395909476                                                  
wall_time_nsec   :
49                                                          
wall_to_monotonic                                                              

tv_sec : -1395754171                                                 
tv_nsec : 750313156                                                   
cycle_last       :
5127088924084                                               
mask             :
0xffffffffffffffff                                          
mult             :
508284933                                                   
shift            :
24                                                          
                                                                               

== Tested clock: 42
(CLOCK_HOST_REALTIME)                                      
CPU      ToD offset [us] ToD drift [us/s]      warps max delta
[us]            
--- -------------------- ---------------- ----------
--------------            
  0            -990145.8       -11538.531       2740         6253.3

As you can see, only 8 bits of the nanosecond value (wall_time_nsec)
will be displayed. Instrumentation (printks) has shown that these are
the most significant 8 bits of the value.

Reason:
The function tk_xtime(struct timekeeper *tk) (in file
include/linux/timekeeper_internal.h)

static inline struct timespec tk_xtime(struct timekeeper *tk)
{
    struct timespec ts;

    ts.tv_sec = tk->xtime_sec;
    ts.tv_nsec = (long)(tk->xtime_nsec >> tk->shift);
    return ts;
}

is applied twice to the system time. First in this function (in file
include/linux/timekeeper_internal.h):

static inline void update_vsyscall(struct timekeeper *tk)
{
    struct timespec xt;

    xt = tk_xtime(tk);
    update_vsyscall_old(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult);
}

and then in this function (in file /kernel/ipipe/timer.c):

void ipipe_update_hostrt(struct timekeeper *tk)
{
    struct ipipe_hostrt_data data;
    struct timespec xt;

    xt = tk_xtime(tk);
    ipipe_root_only();
    data.live = 1;
    data.cycle_last = tk->clock->cycle_last;
    data.mask = tk->clock->mask;
    data.mult = tk->mult;
    data.shift = tk->shift;
    data.wall_time_sec = xt.tv_sec;
    data.wall_time_nsec = xt.tv_nsec;
    data.wall_to_monotonic = tk->wall_to_monotonic;
    __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
}

tk_xtime() makes a shift to the right on the nanoseconds. This shift is
at our clock source 24. If this shift now run twice consecutively, that
explains naturally why we only get the most significant 8 bits of the
nanosecond value.

a fix that might solve the problem, looks like this

void ipipe_update_hostrt(struct timekeeper *tk)
{
    struct ipipe_hostrt_data data;
    struct timespec xt;

//     xt = tk_xtime(tk);
    ipipe_root_only();
    data.live = 1;
    data.cycle_last = tk->clock->cycle_last;
    data.mask = tk->clock->mask;
    data.mult = tk->mult;
    data.shift = tk->shift;
    data.wall_time_sec = tk->xtime_sec;
    data.wall_time_nsec = (long)tk->xtime_nsec;
    data.wall_to_monotonic = tk->wall_to_monotonic;
    __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
}

I have successfully tested this fix on our system.

Regards

Ralf

Re: [Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Ralf]

Am 28.03.2014 08:39, schrieb Ralf:
> Hello,
>
> i have found an error in the ipipe-patch above.
> Here ist the error description:
>
> Our system:
> Linux 3.10.28 with xenomai 2.6 patch on a PowerPC MPC5200
>
> Error description:
> We use the xenomai-(posix-skin-)function clock_gettime(
> CLOCK_HOST_REALTIME, ..) to read out our system time. Thus, we set up a
> NTP clock that runs on an FPGA. Later, we compare the system time and
> NTP time deviations. Unfortunately we recognize in any comparison with
> the nanoseconds a deviation of at least 100 million nsecs. We then
> started the xenomai test suite and get the following prints on our console:
>
> root@hbm-000a0f(NFS):/$clocktest -D -C
> 42                                      
> hostrt data area is
> live                                                       
> Sequence counter :
> 92323948                                                    
> wall_time_sec    :
> 1395909476                                                  
> wall_time_nsec   :
> 49                                                          
> wall_to_monotonic                                                              
>
> tv_sec : -1395754171                                                 
> tv_nsec : 750313156                                                   
> cycle_last       :
> 5127088924084                                               
> mask             :
> 0xffffffffffffffff                                          
> mult             :
> 508284933                                                   
> shift            :
> 24                                                          
>                                                                                
>
> == Tested clock: 42
> (CLOCK_HOST_REALTIME)                                      
> CPU      ToD offset [us] ToD drift [us/s]      warps max delta
> [us]            
> --- -------------------- ---------------- ----------
> --------------            
>   0            -990145.8       -11538.531       2740         6253.3
>
> As you can see, only 8 bits of the nanosecond value (wall_time_nsec)
> will be displayed. Instrumentation (printks) has shown that these are
> the most significant 8 bits of the value.
>
> Reason:
> The function tk_xtime(struct timekeeper *tk) (in file
> include/linux/timekeeper_internal.h)
>
> static inline struct timespec tk_xtime(struct timekeeper *tk)
> {
>     struct timespec ts;
>
>     ts.tv_sec = tk->xtime_sec;
>     ts.tv_nsec = (long)(tk->xtime_nsec >> tk->shift);
>     return ts;
> }
>
> is applied twice to the system time. First in this function (in file
> include/linux/timekeeper_internal.h):
>
> static inline void update_vsyscall(struct timekeeper *tk)
> {
>     struct timespec xt;
>
>     xt = tk_xtime(tk);
>     update_vsyscall_old(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult);
> }
>
> and then in this function (in file /kernel/ipipe/timer.c):
>
> void ipipe_update_hostrt(struct timekeeper *tk)
> {
>     struct ipipe_hostrt_data data;
>     struct timespec xt;
>
>     xt = tk_xtime(tk);
>     ipipe_root_only();
>     data.live = 1;
>     data.cycle_last = tk->clock->cycle_last;
>     data.mask = tk->clock->mask;
>     data.mult = tk->mult;
>     data.shift = tk->shift;
>     data.wall_time_sec = xt.tv_sec;
>     data.wall_time_nsec = xt.tv_nsec;
>     data.wall_to_monotonic = tk->wall_to_monotonic;
>     __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
> }
>
> tk_xtime() makes a shift to the right on the nanoseconds. This shift is
> at our clock source 24. If this shift now run twice consecutively, that
> explains naturally why we only get the most significant 8 bits of the
> nanosecond value.
>
> a fix that might solve the problem, looks like this
>
> void ipipe_update_hostrt(struct timekeeper *tk)
> {
>     struct ipipe_hostrt_data data;
>     struct timespec xt;
>
> //     xt = tk_xtime(tk);
>     ipipe_root_only();
>     data.live = 1;
>     data.cycle_last = tk->clock->cycle_last;
>     data.mask = tk->clock->mask;
>     data.mult = tk->mult;
>     data.shift = tk->shift;
>     data.wall_time_sec = tk->xtime_sec;
>     data.wall_time_nsec = (long)tk->xtime_nsec;
>     data.wall_to_monotonic = tk->wall_to_monotonic;
>     __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
> }
>
> I have successfully tested this fix on our system.
>
> Regards
>
> Ralf
>
>
> _______________________________________________
> Xenomai mailing list
> Xenomai@xenomai.org
> http://www.xenomai.org/mailman/listinfo/xenomai
>

Has anyone observed the same thing? Do you think that bugfix correct?
How can i bring it into mainline? I would be very grateful for some help.

Regards

Ralf

Re: [Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Philippe Gerum]

On 04/14/2014 08:55 AM, Ralf wrote:
> Am 28.03.2014 08:39, schrieb Ralf:
>> Hello,
>>
>> i have found an error in the ipipe-patch above.
>> Here ist the error description:
>>
>> Our system:
>> Linux 3.10.28 with xenomai 2.6 patch on a PowerPC MPC5200
>>
>> Error description:
>> We use the xenomai-(posix-skin-)function clock_gettime(
>> CLOCK_HOST_REALTIME, ..) to read out our system time. Thus, we set up a
>> NTP clock that runs on an FPGA. Later, we compare the system time and
>> NTP time deviations. Unfortunately we recognize in any comparison with
>> the nanoseconds a deviation of at least 100 million nsecs. We then
>> started the xenomai test suite and get the following prints on our console:
>>

[snip]

>> tk_xtime() makes a shift to the right on the nanoseconds. This shift is
>> at our clock source 24. If this shift now run twice consecutively, that
>> explains naturally why we only get the most significant 8 bits of the
>> nanosecond value.
>>
>> a fix that might solve the problem, looks like this
>>
>> void ipipe_update_hostrt(struct timekeeper *tk)
>> {
>>      struct ipipe_hostrt_data data;
>>      struct timespec xt;
>>
>> //     xt = tk_xtime(tk);
>>      ipipe_root_only();
>>      data.live = 1;
>>      data.cycle_last = tk->clock->cycle_last;
>>      data.mask = tk->clock->mask;
>>      data.mult = tk->mult;
>>      data.shift = tk->shift;
>>      data.wall_time_sec = tk->xtime_sec;
>>      data.wall_time_nsec = (long)tk->xtime_nsec;
>>      data.wall_to_monotonic = tk->wall_to_monotonic;
>>      __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
>> }
>>
> 
> Has anyone observed the same thing? Do you think that bugfix correct?
> How can i bring it into mainline? I would be very grateful for some help.
> 

Good catch, thanks. This bug is introduced by a work-around specific to the few architectures which do not implement the newest VSYSCALL interface yet, like powerpc. However, the suggested change would break others which do, could you test the patch below instead? TIA,

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index e1214ba..5b01356 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -749,7 +749,7 @@ static cycle_t timebase_read(struct clocksource *cs)
  * API.
  */
 static inline void update_hostrt(struct timespec *wall_time, struct timespec *wtm,
-				 struct clocksource *clock, u32 mult)
+				 struct clocksource *clock, u32 mult, u32 shift)
 {
 	/*
 	 * This is a temporary work-around until powerpc implements
@@ -761,7 +761,7 @@ static inline void update_hostrt(struct timespec *wall_time, struct timespec *wt
 		.shift = clock->shift,
 		.mult = mult,
 		.xtime_sec = wall_time->tv_sec,
-		.xtime_nsec = wall_time->tv_nsec,
+		.xtime_nsec = wall_time->tv_nsec << shift,
 		.wall_to_monotonic = *wtm,
 	};
 
@@ -771,7 +771,7 @@ static inline void update_hostrt(struct timespec *wall_time, struct timespec *wt
 #endif
 
 void update_vsyscall_old(struct timespec *wall_time, struct timespec *wtm,
-			struct clocksource *clock, u32 mult)
+			 struct clocksource *clock, u32 mult, u32 shift)
 {
 	u64 new_tb_to_xs, new_stamp_xsec;
 	u32 frac_sec;
@@ -814,7 +814,7 @@ void update_vsyscall_old(struct timespec *wall_time, struct timespec *wtm,
 	smp_wmb();
 	++(vdso_data->tb_update_count);
 
-	update_hostrt(wall_time, wtm, clock, mult);
+	update_hostrt(wall_time, wtm, clock, mult, shift);
 }
 
 void update_vsyscall_tz(void)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index c1825eb..93ee8d6 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -89,7 +89,7 @@ extern void update_vsyscall_tz(void);
 #elif defined(CONFIG_GENERIC_TIME_VSYSCALL_OLD)
 
 extern void update_vsyscall_old(struct timespec *ts, struct timespec *wtm,
-				struct clocksource *c, u32 mult);
+				struct clocksource *c, u32 mult, u32 shift);
 extern void update_vsyscall_tz(void);
 
 static inline void update_vsyscall(struct timekeeper *tk)
@@ -97,7 +97,7 @@ static inline void update_vsyscall(struct timekeeper *tk)
 	struct timespec xt;
 
 	xt = tk_xtime(tk);
-	update_vsyscall_old(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult);
+	update_vsyscall_old(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult, tk->shift);
 }
 
 #else

-- 
Philippe.

Re: [Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Ralf]

Am 14.04.2014 09:57, schrieb Philippe Gerum:
> On 04/14/2014 08:55 AM, Ralf wrote:
>> Am 28.03.2014 08:39, schrieb Ralf:
>>> Hello,
>>>
>>> i have found an error in the ipipe-patch above.
>>> Here ist the error description:
>>>
>>> Our system:
>>> Linux 3.10.28 with xenomai 2.6 patch on a PowerPC MPC5200
>>>
>>> Error description:
>>> We use the xenomai-(posix-skin-)function clock_gettime(
>>> CLOCK_HOST_REALTIME, ..) to read out our system time. Thus, we set up a
>>> NTP clock that runs on an FPGA. Later, we compare the system time and
>>> NTP time deviations. Unfortunately we recognize in any comparison with
>>> the nanoseconds a deviation of at least 100 million nsecs. We then
>>> started the xenomai test suite and get the following prints on our console:
>>>
> [snip]
>
>>> tk_xtime() makes a shift to the right on the nanoseconds. This shift is
>>> at our clock source 24. If this shift now run twice consecutively, that
>>> explains naturally why we only get the most significant 8 bits of the
>>> nanosecond value.
>>>
>>> a fix that might solve the problem, looks like this
>>>
>>> void ipipe_update_hostrt(struct timekeeper *tk)
>>> {
>>>      struct ipipe_hostrt_data data;
>>>      struct timespec xt;
>>>
>>> //     xt = tk_xtime(tk);
>>>      ipipe_root_only();
>>>      data.live = 1;
>>>      data.cycle_last = tk->clock->cycle_last;
>>>      data.mask = tk->clock->mask;
>>>      data.mult = tk->mult;
>>>      data.shift = tk->shift;
>>>      data.wall_time_sec = tk->xtime_sec;
>>>      data.wall_time_nsec = (long)tk->xtime_nsec;
>>>      data.wall_to_monotonic = tk->wall_to_monotonic;
>>>      __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
>>> }
>>>
>> Has anyone observed the same thing? Do you think that bugfix correct?
>> How can i bring it into mainline? I would be very grateful for some help.
>>
> Good catch, thanks. This bug is introduced by a work-around specific to the few architectures which do not implement the newest VSYSCALL interface yet, like powerpc. However, the suggested change would break others which do, could you test the patch below instead? TIA,
>
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index e1214ba..5b01356 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -749,7 +749,7 @@ static cycle_t timebase_read(struct clocksource *cs)
>   * API.
>   */
>  static inline void update_hostrt(struct timespec *wall_time, struct timespec *wtm,
> -				 struct clocksource *clock, u32 mult)
> +				 struct clocksource *clock, u32 mult, u32 shift)
>  {
>  	/*
>  	 * This is a temporary work-around until powerpc implements
> @@ -761,7 +761,7 @@ static inline void update_hostrt(struct timespec *wall_time, struct timespec *wt
>  		.shift = clock->shift,
>  		.mult = mult,
>  		.xtime_sec = wall_time->tv_sec,
> -		.xtime_nsec = wall_time->tv_nsec,
> +		.xtime_nsec = wall_time->tv_nsec << shift,
Unfortunately, this solution does not work on our powerpc, because
"wall_time->tv_nsec" is only a 32 bit value and "xtime_nsec" a 64 bit
value. It seems that the shift on the 32 bit value happens first and
only then the assignment to the 64 bit value. So the shifted bits
greater than 32 bit are lost.
A cast avoids this problem:

                        .xtime_nsec = (u64)wall_time->tv_nsec << shift;

After that everything works well.
>  		.wall_to_monotonic = *wtm,
>  	};
>  
> @@ -771,7 +771,7 @@ static inline void update_hostrt(struct timespec *wall_time, struct timespec *wt
>  #endif
>  
>  void update_vsyscall_old(struct timespec *wall_time, struct timespec *wtm,
> -			struct clocksource *clock, u32 mult)
> +			 struct clocksource *clock, u32 mult, u32 shift)
>  {
>  	u64 new_tb_to_xs, new_stamp_xsec;
>  	u32 frac_sec;
> @@ -814,7 +814,7 @@ void update_vsyscall_old(struct timespec *wall_time, struct timespec *wtm,
>  	smp_wmb();
>  	++(vdso_data->tb_update_count);
>  
> -	update_hostrt(wall_time, wtm, clock, mult);
> +	update_hostrt(wall_time, wtm, clock, mult, shift);
>  }
>  
>  void update_vsyscall_tz(void)
> diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
> index c1825eb..93ee8d6 100644
> --- a/include/linux/timekeeper_internal.h
> +++ b/include/linux/timekeeper_internal.h
> @@ -89,7 +89,7 @@ extern void update_vsyscall_tz(void);
>  #elif defined(CONFIG_GENERIC_TIME_VSYSCALL_OLD)
>  
>  extern void update_vsyscall_old(struct timespec *ts, struct timespec *wtm,
> -				struct clocksource *c, u32 mult);
> +				struct clocksource *c, u32 mult, u32 shift);
>  extern void update_vsyscall_tz(void);
>  
>  static inline void update_vsyscall(struct timekeeper *tk)
> @@ -97,7 +97,7 @@ static inline void update_vsyscall(struct timekeeper *tk)
>  	struct timespec xt;
>  
>  	xt = tk_xtime(tk);
> -	update_vsyscall_old(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult);
> +	update_vsyscall_old(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult, tk->shift);
>  }
>  
>  #else
>

Re: [Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Philippe Gerum]

On 04/14/2014 02:10 PM, Ralf wrote:
> Am 14.04.2014 09:57, schrieb Philippe Gerum:
>> On 04/14/2014 08:55 AM, Ralf wrote:
>>> Am 28.03.2014 08:39, schrieb Ralf:
>>>> Hello,
>>>>
>>>> i have found an error in the ipipe-patch above.
>>>> Here ist the error description:
>>>>
>>>> Our system:
>>>> Linux 3.10.28 with xenomai 2.6 patch on a PowerPC MPC5200
>>>>
>>>> Error description:
>>>> We use the xenomai-(posix-skin-)function clock_gettime(
>>>> CLOCK_HOST_REALTIME, ..) to read out our system time. Thus, we set up a
>>>> NTP clock that runs on an FPGA. Later, we compare the system time and
>>>> NTP time deviations. Unfortunately we recognize in any comparison with
>>>> the nanoseconds a deviation of at least 100 million nsecs. We then
>>>> started the xenomai test suite and get the following prints on our console:
>>>>
>> [snip]
>>
>>>> tk_xtime() makes a shift to the right on the nanoseconds. This shift is
>>>> at our clock source 24. If this shift now run twice consecutively, that
>>>> explains naturally why we only get the most significant 8 bits of the
>>>> nanosecond value.
>>>>
>>>> a fix that might solve the problem, looks like this
>>>>
>>>> void ipipe_update_hostrt(struct timekeeper *tk)
>>>> {
>>>>       struct ipipe_hostrt_data data;
>>>>       struct timespec xt;
>>>>
>>>> //     xt = tk_xtime(tk);
>>>>       ipipe_root_only();
>>>>       data.live = 1;
>>>>       data.cycle_last = tk->clock->cycle_last;
>>>>       data.mask = tk->clock->mask;
>>>>       data.mult = tk->mult;
>>>>       data.shift = tk->shift;
>>>>       data.wall_time_sec = tk->xtime_sec;
>>>>       data.wall_time_nsec = (long)tk->xtime_nsec;
>>>>       data.wall_to_monotonic = tk->wall_to_monotonic;
>>>>       __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
>>>> }
>>>>
>>> Has anyone observed the same thing? Do you think that bugfix correct?
>>> How can i bring it into mainline? I would be very grateful for some help.
>>>
>> Good catch, thanks. This bug is introduced by a work-around specific to the few architectures which do not implement the newest VSYSCALL interface yet, like powerpc. However, the suggested change would break others which do, could you test the patch below instead? TIA,
>>
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index e1214ba..5b01356 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -749,7 +749,7 @@ static cycle_t timebase_read(struct clocksource *cs)
>>    * API.
>>    */
>>   static inline void update_hostrt(struct timespec *wall_time, struct timespec *wtm,
>> -				 struct clocksource *clock, u32 mult)
>> +				 struct clocksource *clock, u32 mult, u32 shift)
>>   {
>>   	/*
>>   	 * This is a temporary work-around until powerpc implements
>> @@ -761,7 +761,7 @@ static inline void update_hostrt(struct timespec *wall_time, struct timespec *wt
>>   		.shift = clock->shift,
>>   		.mult = mult,
>>   		.xtime_sec = wall_time->tv_sec,
>> -		.xtime_nsec = wall_time->tv_nsec,
>> +		.xtime_nsec = wall_time->tv_nsec << shift,
> Unfortunately, this solution does not work on our powerpc, because
> "wall_time->tv_nsec" is only a 32 bit value and "xtime_nsec" a 64 bit
> value. It seems that the shift on the 32 bit value happens first and
> only then the assignment to the 64 bit value. So the shifted bits
> greater than 32 bit are lost.
> A cast avoids this problem:
>
>                          .xtime_nsec = (u64)wall_time->tv_nsec << shift;
>
> After that everything works well.

This make sense. I'll merge this fix, thanks.

-- 
Philippe.

Re: [Xenomai] error / bugfix in ipipe-core-3-10.28-powerpc-1.patch (from eldk 5.5, company denx)

[Philippe Gerum]

On 04/14/2014 02:20 PM, Philippe Gerum wrote:
> On 04/14/2014 02:10 PM, Ralf wrote:
>> Am 14.04.2014 09:57, schrieb Philippe Gerum:
>>> On 04/14/2014 08:55 AM, Ralf wrote:
>>>> Am 28.03.2014 08:39, schrieb Ralf:
>>>>> Hello,
>>>>>
>>>>> i have found an error in the ipipe-patch above.
>>>>> Here ist the error description:
>>>>>
>>>>> Our system:
>>>>> Linux 3.10.28 with xenomai 2.6 patch on a PowerPC MPC5200
>>>>>
>>>>> Error description:
>>>>> We use the xenomai-(posix-skin-)function clock_gettime(
>>>>> CLOCK_HOST_REALTIME, ..) to read out our system time. Thus, we set
>>>>> up a
>>>>> NTP clock that runs on an FPGA. Later, we compare the system time and
>>>>> NTP time deviations. Unfortunately we recognize in any comparison with
>>>>> the nanoseconds a deviation of at least 100 million nsecs. We then
>>>>> started the xenomai test suite and get the following prints on our
>>>>> console:
>>>>>
>>> [snip]
>>>
>>>>> tk_xtime() makes a shift to the right on the nanoseconds. This
>>>>> shift is
>>>>> at our clock source 24. If this shift now run twice consecutively,
>>>>> that
>>>>> explains naturally why we only get the most significant 8 bits of the
>>>>> nanosecond value.
>>>>>
>>>>> a fix that might solve the problem, looks like this
>>>>>
>>>>> void ipipe_update_hostrt(struct timekeeper *tk)
>>>>> {
>>>>>       struct ipipe_hostrt_data data;
>>>>>       struct timespec xt;
>>>>>
>>>>> //     xt = tk_xtime(tk);
>>>>>       ipipe_root_only();
>>>>>       data.live = 1;
>>>>>       data.cycle_last = tk->clock->cycle_last;
>>>>>       data.mask = tk->clock->mask;
>>>>>       data.mult = tk->mult;
>>>>>       data.shift = tk->shift;
>>>>>       data.wall_time_sec = tk->xtime_sec;
>>>>>       data.wall_time_nsec = (long)tk->xtime_nsec;
>>>>>       data.wall_to_monotonic = tk->wall_to_monotonic;
>>>>>       __ipipe_notify_kevent(IPIPE_KEVT_HOSTRT, &data);
>>>>> }
>>>>>
>>>> Has anyone observed the same thing? Do you think that bugfix correct?
>>>> How can i bring it into mainline? I would be very grateful for some
>>>> help.
>>>>
>>> Good catch, thanks. This bug is introduced by a work-around specific
>>> to the few architectures which do not implement the newest VSYSCALL
>>> interface yet, like powerpc. However, the suggested change would
>>> break others which do, could you test the patch below instead? TIA,
>>>
>>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>>> index e1214ba..5b01356 100644
>>> --- a/arch/powerpc/kernel/time.c
>>> +++ b/arch/powerpc/kernel/time.c
>>> @@ -749,7 +749,7 @@ static cycle_t timebase_read(struct clocksource *cs)
>>>    * API.
>>>    */
>>>   static inline void update_hostrt(struct timespec *wall_time, struct
>>> timespec *wtm,
>>> -                 struct clocksource *clock, u32 mult)
>>> +                 struct clocksource *clock, u32 mult, u32 shift)
>>>   {
>>>       /*
>>>        * This is a temporary work-around until powerpc implements
>>> @@ -761,7 +761,7 @@ static inline void update_hostrt(struct timespec
>>> *wall_time, struct timespec *wt
>>>           .shift = clock->shift,
>>>           .mult = mult,
>>>           .xtime_sec = wall_time->tv_sec,
>>> -        .xtime_nsec = wall_time->tv_nsec,
>>> +        .xtime_nsec = wall_time->tv_nsec << shift,
>> Unfortunately, this solution does not work on our powerpc, because
>> "wall_time->tv_nsec" is only a 32 bit value and "xtime_nsec" a 64 bit
>> value. It seems that the shift on the 32 bit value happens first and
>> only then the assignment to the 64 bit value. So the shifted bits
>> greater than 32 bit are lost.
>> A cast avoids this problem:
>>
>>                          .xtime_nsec = (u64)wall_time->tv_nsec << shift;
>>
>> After that everything works well.
>
> This make sense.

And this also makes sense without the typo. Oh, well...

  I'll merge this fix, thanks.
>

-- 
Philippe.