[Xenomai 3.1 PATCH v2] process: update clockfreq when receive corresponding event.

[Xenomai 3.1 PATCH v2] process: update clockfreq when receive corresponding event.

[Hongzhan Chen]

1. When there is clockfreq param passed down via command line, we
   do not update clockfreq even if we receive event of updating clockfreq.
   Or else, we update the clockfreq with notified value.
2. At the same time, we would like to update clockfreq param showing
   in sys filesystem after apply updated clockfreq.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 include/cobalt/kernel/init.h  |  2 ++
 kernel/cobalt/init.c          | 12 ++++++++++++
 kernel/cobalt/posix/process.c |  3 ++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
index 41dd531a8..36d1ea290 100644
--- a/include/cobalt/kernel/init.h
+++ b/include/cobalt/kernel/init.h
@@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
 
 void cobalt_call_state_chain(enum cobalt_run_states newstate);
 
+void cobalt_update_clockfreq(unsigned long freq);
+
 #endif /* !_COBALT_KERNEL_INIT_H_ */
diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
index dbe321c3b..558030292 100644
--- a/kernel/cobalt/init.c
+++ b/kernel/cobalt/init.c
@@ -53,6 +53,16 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
 static unsigned long clockfreq_arg;
 module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
 
+static bool passed_clockfreq;
+
+void cobalt_update_clockfreq(unsigned long freq)
+{
+	if (!passed_clockfreq) {
+		xnclock_update_freq(freq);
+		clockfreq_arg = freq;
+	}
+}
+
 #ifdef CONFIG_SMP
 static unsigned long supported_cpus_arg = -1;
 module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
@@ -150,6 +160,8 @@ static int __init mach_setup(void)
 
 	if (clockfreq_arg == 0)
 		clockfreq_arg = sysinfo.sys_hrclock_freq;
+	else
+		passed_clockfreq = clockfreq_arg != 0;
 
 	if (clockfreq_arg == 0) {
 		printk(XENO_ERR "null clock frequency? Aborting.\n");
diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index 6d1c1c427..f762fef2e 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -38,6 +38,7 @@
 #include <linux/kallsyms.h>
 #include <linux/ipipe.h>
 #include <linux/ipipe_tickdev.h>
+#include <cobalt/kernel/init.h>
 #include <cobalt/kernel/sched.h>
 #include <cobalt/kernel/heap.h>
 #include <cobalt/kernel/synch.h>
@@ -1382,7 +1383,7 @@ static inline int handle_clockfreq_event(unsigned int *p)
 {
 	unsigned int newfreq = *p;
 
-	xnclock_update_freq(newfreq);
+	cobalt_update_clockfreq(newfreq);
 
 	return KEVENT_PROPAGATE;
 }
-- 
2.17.1

[I-PIPE PATCH v2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Hongzhan Chen]

When there is refined tsc clock, notify Xenomai to apply it.
Linux may schedule a delayed work to refine tsc clock and update
tsc_khz which happen after Xenomai finsih init but tsc_scale and
tsc_shift still keep the value depending on origianl tsc clock
which is outdated. The difference between two clocks may cause
unexpected timing drift.

For example:
  [ 0.001731] tsc: Detected 2899.886 MHz TSC
  [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
  cat /sys/module/xenomai/parameters/clockfreq
  2899886000
  After patching, we like to use 2903.999 MHz.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 arch/x86/kernel/tsc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 835856efd71f..37faaf9a9e6c 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 	u64 tsc_stop, ref_stop, delta;
 	unsigned long freq;
 	int cpu;
+	unsigned int ipipe_freq;
 
 	/* Don't bother refining TSC on unstable systems */
 	if (tsc_unstable)
@@ -1345,6 +1346,9 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 	/* Inform the TSC deadline clockevent devices about the recalibration */
 	lapic_update_tsc_freq();
 
+	ipipe_freq = tsc_khz * 1000;
+	 __ipipe_report_clockfreq_update(ipipe_freq);
+
 	/* Update the sched_clock() rate to match the clocksource one */
 	for_each_possible_cpu(cpu)
 		set_cyc2ns_scale(tsc_khz, cpu, tsc_stop);
-- 
2.17.1

Re: [Xenomai 3.1 PATCH v2] process: update clockfreq when receive corresponding event.

[Jan Kiszka]

On 16.06.22 02:18, Hongzhan Chen via Xenomai wrote:
> 1. When there is clockfreq param passed down via command line, we
>    do not update clockfreq even if we receive event of updating clockfreq.
>    Or else, we update the clockfreq with notified value.
> 2. At the same time, we would like to update clockfreq param showing
>    in sys filesystem after apply updated clockfreq.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> ---
>  include/cobalt/kernel/init.h  |  2 ++
>  kernel/cobalt/init.c          | 12 ++++++++++++
>  kernel/cobalt/posix/process.c |  3 ++-
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
> index 41dd531a8..36d1ea290 100644
> --- a/include/cobalt/kernel/init.h
> +++ b/include/cobalt/kernel/init.h
> @@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
>  
>  void cobalt_call_state_chain(enum cobalt_run_states newstate);
>  
> +void cobalt_update_clockfreq(unsigned long freq);
> +
>  #endif /* !_COBALT_KERNEL_INIT_H_ */
> diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
> index dbe321c3b..558030292 100644
> --- a/kernel/cobalt/init.c
> +++ b/kernel/cobalt/init.c
> @@ -53,6 +53,16 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
>  static unsigned long clockfreq_arg;
>  module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
>  
> +static bool passed_clockfreq;
> +
> +void cobalt_update_clockfreq(unsigned long freq)
> +{
> +	if (!passed_clockfreq) {
> +		xnclock_update_freq(freq);
> +		clockfreq_arg = freq;
> +	}
> +}
> +
>  #ifdef CONFIG_SMP
>  static unsigned long supported_cpus_arg = -1;
>  module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
> @@ -150,6 +160,8 @@ static int __init mach_setup(void)
>  
>  	if (clockfreq_arg == 0)
>  		clockfreq_arg = sysinfo.sys_hrclock_freq;
> +	else
> +		passed_clockfreq = clockfreq_arg != 0;

If you assign in the conditional branch, you don't need to check
clockfreq_arg, just set it true.

>  
>  	if (clockfreq_arg == 0) {
>  		printk(XENO_ERR "null clock frequency? Aborting.\n");
> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
> index 6d1c1c427..f762fef2e 100644
> --- a/kernel/cobalt/posix/process.c
> +++ b/kernel/cobalt/posix/process.c
> @@ -38,6 +38,7 @@
>  #include <linux/kallsyms.h>
>  #include <linux/ipipe.h>
>  #include <linux/ipipe_tickdev.h>
> +#include <cobalt/kernel/init.h>
>  #include <cobalt/kernel/sched.h>
>  #include <cobalt/kernel/heap.h>
>  #include <cobalt/kernel/synch.h>
> @@ -1382,7 +1383,7 @@ static inline int handle_clockfreq_event(unsigned int *p)
>  {
>  	unsigned int newfreq = *p;
>  
> -	xnclock_update_freq(newfreq);
> +	cobalt_update_clockfreq(newfreq);
>  
>  	return KEVENT_PROPAGATE;
>  }


-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [I-PIPE PATCH v2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Jan Kiszka]

On 16.06.22 02:18, Hongzhan Chen via Xenomai wrote:
> When there is refined tsc clock, notify Xenomai to apply it.
> Linux may schedule a delayed work to refine tsc clock and update
> tsc_khz which happen after Xenomai finsih init but tsc_scale and
> tsc_shift still keep the value depending on origianl tsc clock
> which is outdated. The difference between two clocks may cause
> unexpected timing drift.
> 
> For example:
>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>   cat /sys/module/xenomai/parameters/clockfreq
>   2899886000
>   After patching, we like to use 2903.999 MHz.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> ---
>  arch/x86/kernel/tsc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 835856efd71f..37faaf9a9e6c 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>  	u64 tsc_stop, ref_stop, delta;
>  	unsigned long freq;
>  	int cpu;
> +	unsigned int ipipe_freq;
>  
>  	/* Don't bother refining TSC on unstable systems */
>  	if (tsc_unstable)
> @@ -1345,6 +1346,9 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>  	/* Inform the TSC deadline clockevent devices about the recalibration */
>  	lapic_update_tsc_freq();
>  
> +	ipipe_freq = tsc_khz * 1000;

You are still using a separate variable.

> +	 __ipipe_report_clockfreq_update(ipipe_freq);
> +
>  	/* Update the sched_clock() rate to match the clocksource one */
>  	for_each_possible_cpu(cpu)
>  		set_cyc2ns_scale(tsc_khz, cpu, tsc_stop);

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [I-PIPE PATCH v2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Jan Kiszka]

On 17.06.22 08:09, Jan Kiszka via Xenomai wrote:
> On 16.06.22 02:18, Hongzhan Chen via Xenomai wrote:
>> When there is refined tsc clock, notify Xenomai to apply it.
>> Linux may schedule a delayed work to refine tsc clock and update
>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>> tsc_shift still keep the value depending on origianl tsc clock
>> which is outdated. The difference between two clocks may cause
>> unexpected timing drift.
>>
>> For example:
>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>   cat /sys/module/xenomai/parameters/clockfreq
>>   2899886000
>>   After patching, we like to use 2903.999 MHz.
>>
>> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
>> ---
>>  arch/x86/kernel/tsc.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
>> index 835856efd71f..37faaf9a9e6c 100644
>> --- a/arch/x86/kernel/tsc.c
>> +++ b/arch/x86/kernel/tsc.c
>> @@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>>  	u64 tsc_stop, ref_stop, delta;
>>  	unsigned long freq;
>>  	int cpu;
>> +	unsigned int ipipe_freq;
>>  
>>  	/* Don't bother refining TSC on unstable systems */
>>  	if (tsc_unstable)
>> @@ -1345,6 +1346,9 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>>  	/* Inform the TSC deadline clockevent devices about the recalibration */
>>  	lapic_update_tsc_freq();
>>  
>> +	ipipe_freq = tsc_khz * 1000;
> 
> You are still using a separate variable.
> 

Seems I missed a reply on that in the list - spam filters or who knows...

__ipipe_report_clockfreq_update() is defined in an unhandy way to
enforce this, I see. Would have been better to define it like this:

#define __ipipe_report_clockfreq_update(freq)			\
	do {							\
		unsigned int __freq = (freq);			\
		__ipipe_notify_kevent(IPIPE_KEVT_CLOCKFREQ, &__freq); \
        } while (0)

OTH, we are in maintenance mode, so let's ignore this.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [I-PIPE PATCH v2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Jan Kiszka]

On 16.06.22 02:18, Hongzhan Chen via Xenomai wrote:
> When there is refined tsc clock, notify Xenomai to apply it.
> Linux may schedule a delayed work to refine tsc clock and update
> tsc_khz which happen after Xenomai finsih init but tsc_scale and
> tsc_shift still keep the value depending on origianl tsc clock
> which is outdated. The difference between two clocks may cause
> unexpected timing drift.
> 
> For example:
>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>   cat /sys/module/xenomai/parameters/clockfreq
>   2899886000
>   After patching, we like to use 2903.999 MHz.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> ---
>  arch/x86/kernel/tsc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 835856efd71f..37faaf9a9e6c 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>  	u64 tsc_stop, ref_stop, delta;
>  	unsigned long freq;
>  	int cpu;
> +	unsigned int ipipe_freq;
>  
>  	/* Don't bother refining TSC on unstable systems */
>  	if (tsc_unstable)
> @@ -1345,6 +1346,9 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>  	/* Inform the TSC deadline clockevent devices about the recalibration */
>  	lapic_update_tsc_freq();
>  
> +	ipipe_freq = tsc_khz * 1000;
> +	 __ipipe_report_clockfreq_update(ipipe_freq);
       ^^^
Wrong indention - fixed up on merge.

Jan

> +
>  	/* Update the sched_clock() rate to match the clocksource one */
>  	for_each_possible_cpu(cpu)
>  		set_cyc2ns_scale(tsc_khz, cpu, tsc_stop);

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [Xenomai 3.1 PATCH v2] process: update clockfreq when receive corresponding event.

[Jan Kiszka]

On 17.06.22 08:08, Jan Kiszka via Xenomai wrote:
> On 16.06.22 02:18, Hongzhan Chen via Xenomai wrote:
>> 1. When there is clockfreq param passed down via command line, we
>>    do not update clockfreq even if we receive event of updating clockfreq.
>>    Or else, we update the clockfreq with notified value.
>> 2. At the same time, we would like to update clockfreq param showing
>>    in sys filesystem after apply updated clockfreq.
>>
>> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
>> ---
>>  include/cobalt/kernel/init.h  |  2 ++
>>  kernel/cobalt/init.c          | 12 ++++++++++++
>>  kernel/cobalt/posix/process.c |  3 ++-
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
>> index 41dd531a8..36d1ea290 100644
>> --- a/include/cobalt/kernel/init.h
>> +++ b/include/cobalt/kernel/init.h
>> @@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
>>  
>>  void cobalt_call_state_chain(enum cobalt_run_states newstate);
>>  
>> +void cobalt_update_clockfreq(unsigned long freq);
>> +
>>  #endif /* !_COBALT_KERNEL_INIT_H_ */
>> diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
>> index dbe321c3b..558030292 100644
>> --- a/kernel/cobalt/init.c
>> +++ b/kernel/cobalt/init.c
>> @@ -53,6 +53,16 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
>>  static unsigned long clockfreq_arg;
>>  module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
>>  
>> +static bool passed_clockfreq;
>> +
>> +void cobalt_update_clockfreq(unsigned long freq)
>> +{
>> +	if (!passed_clockfreq) {
>> +		xnclock_update_freq(freq);
>> +		clockfreq_arg = freq;
>> +	}
>> +}
>> +
>>  #ifdef CONFIG_SMP
>>  static unsigned long supported_cpus_arg = -1;
>>  module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
>> @@ -150,6 +160,8 @@ static int __init mach_setup(void)
>>  
>>  	if (clockfreq_arg == 0)
>>  		clockfreq_arg = sysinfo.sys_hrclock_freq;
>> +	else
>> +		passed_clockfreq = clockfreq_arg != 0;
> 
> If you assign in the conditional branch, you don't need to check
> clockfreq_arg, just set it true.
> 

Fixed up while merging.

Both patches now applied. This one to stable/v3.1.x and stable/v3.0.x.
The I-pipe patch to 5.4, 4.19-cip and 4.4-cip. Tests are running, please
also check.

Thanks,
Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux