CYGPKG_CPULOAD fixes

CYGPKG_CPULOAD fixes

[Lars Povlsen]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

Here's a small fix to the cpuload package (and idle thread).

In short, a missing 'volatile' attribute to the loop counter causes
newer (arm-eabi GCC 4.2.3/eCosentric toolchain) to produce wrong
results.

This patch adds a volatile modifier to idle_thread_loops[], and updates
the reference to it from the cpuload package. (which was also using a
non-array declaration).

Cheers,

Lars Povlsen
Vitesse Semiconductor Corp


[-- Attachment #2: cpuload.diff --]
[-- Type: application/octet-stream, Size: 4371 bytes --]

Index: kernel/current/ChangeLog
===================================================================
RCS file: /import/dk_cvs/exbit/sw/3rd_party/eCos/packages/kernel/current/ChangeLog,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 ChangeLog
--- kernel/current/ChangeLog	11 Feb 2009 13:32:59 -0000	1.1.1.7
+++ kernel/current/ChangeLog	29 Mar 2010 09:16:39 -0000
@@ -1,3 +1,8 @@
+2010-03-29  Lars Povlsen  <lpovlsen@vitesse.com>
+
+	* src/common/thread.cxx: Add volatile to idle_thread_loops to make
+	cpuload package able to produce something useful.
+
 2008-12-18  Jonathan Larmour  <jifl@eCosCentric.com>
 
 	* tests/tm_basic.cxx (STACK_SIZE): Revert accidental change of
Index: kernel/current/src/common/thread.cxx
===================================================================
RCS file: /import/dk_cvs/exbit/sw/3rd_party/eCos/packages/kernel/current/src/common/thread.cxx,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 thread.cxx
--- kernel/current/src/common/thread.cxx	11 Feb 2009 13:33:11 -0000	1.1.1.3
+++ kernel/current/src/common/thread.cxx	29 Mar 2010 09:03:38 -0000
@@ -1211,7 +1211,7 @@
 #endif // CYGNUM_HAL_STACK_SIZE_MINIMUM
 
 // Loop counter for debugging/housekeeping
-cyg_uint32 idle_thread_loops[CYGNUM_KERNEL_CPU_MAX];
+cyg_uint32 volatile idle_thread_loops[CYGNUM_KERNEL_CPU_MAX];
 
 static char idle_thread_stack[CYGNUM_KERNEL_CPU_MAX][CYGNUM_KERNEL_THREADS_IDLE_STACK_SIZE];
 
Index: services/cpuload/current/ChangeLog
===================================================================
RCS file: /import/dk_cvs/exbit/sw/3rd_party/eCos/packages/services/cpuload/current/ChangeLog,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 ChangeLog
--- services/cpuload/current/ChangeLog	11 Feb 2009 13:46:12 -0000	1.1.1.2
+++ services/cpuload/current/ChangeLog	29 Mar 2010 09:14:03 -0000
@@ -1,3 +1,7 @@
+2010-03-29  Lars Povlsen  <lpovlsen@vitesse.com>
+
+	* src/cpuload.cxx: Update idle_thread_loops reference (volatile, arrray)
+
 2003-02-24  Jonathan Larmour  <jifl@eCosCentric.com>
 
 	* cdl/cpuload.cdl: Add doc link.
Index: services/cpuload/current/src/cpuload.cxx
===================================================================
RCS file: /import/dk_cvs/exbit/sw/3rd_party/eCos/packages/services/cpuload/current/src/cpuload.cxx,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 cpuload.cxx
--- services/cpuload/current/src/cpuload.cxx	11 Feb 2009 13:46:12 -0000	1.1.1.2
+++ services/cpuload/current/src/cpuload.cxx	29 Mar 2010 09:03:51 -0000
@@ -58,6 +58,7 @@
 #include <cyg/kernel/kapi.h>
 #include <cyg/hal/hal_arch.h> 
 #include <cyg/cpuload/cpuload.h>
+#include <cyg/kernel/sched.hxx>
 
 /* Here we run the idle thread as a high priority thread for 0.1
    second.  We see how much the counter in the idle loop is
@@ -65,7 +66,7 @@
    running at priority 1 and 2 */
 
 static cyg_thread thread;
-externC cyg_uint32 idle_thread_loops;
+externC cyg_uint32 volatile idle_thread_loops[CYGNUM_KERNEL_CPU_MAX];
 char idle_stack[CYGNUM_HAL_STACK_SIZE_MINIMUM];
 
 extern void idle_thread_main( CYG_ADDRESS data );
@@ -108,7 +109,7 @@
   cyg_alarm_initialize(alarmH,cyg_current_time()+10,0);
   cyg_alarm_enable(alarmH);
   
-  idle_loops_start = idle_thread_loops;
+  idle_loops_start = idle_thread_loops[CYG_KERNEL_CPU_THIS()];
   
   /* Dont be decieved, remember this is a multithreaded system ! */
   old_priority = cyg_thread_get_priority(cyg_thread_self());
@@ -116,7 +117,7 @@
   cyg_thread_resume(idleH);
   cyg_thread_set_priority(cyg_thread_self(),old_priority);
   
-  *calibration = idle_thread_loops - idle_loops_start;
+  *calibration = idle_thread_loops[CYG_KERNEL_CPU_THIS()] - idle_loops_start;
   cyg_alarm_delete(alarmH);
   cyg_thread_kill(idleH);
   cyg_thread_delete(idleH);
@@ -125,7 +126,7 @@
 static void 
 cpuload_alarm_func(cyg_handle_t alarm,cyg_addrword_t data) { 
   cyg_cpuload_t * cpuload = (cyg_cpuload_t *)data;
-  cyg_uint32 idle_loops_now = idle_thread_loops;
+  cyg_uint32 idle_loops_now = idle_thread_loops[CYG_KERNEL_CPU_THIS()];
   cyg_uint32 idle_loops;
   cyg_uint32 load;
   
@@ -162,7 +163,7 @@
   cpuload->average_1s = 0;
   cpuload->average_10s = 0;
   cpuload->calibration = calibration;
-  cpuload->last_idle_loops = idle_thread_loops;
+  cpuload->last_idle_loops = idle_thread_loops[CYG_KERNEL_CPU_THIS()];
 
   cyg_clock_to_counter(cyg_real_time_clock(),&counter);
   cyg_alarm_create(counter,

Re: CYGPKG_CPULOAD fixes

[John Dallaway]

Lars Povlsen wrote:

> Here's a small fix to the cpuload package (and idle thread).
> 
> In short, a missing 'volatile' attribute to the loop counter causes
> newer (arm-eabi GCC 4.2.3/eCosentric toolchain) to produce wrong
> results.
> 
> This patch adds a volatile modifier to idle_thread_loops[], and updates
> the reference to it from the cpuload package. (which was also using a
> non-array declaration).

This all seems reasonable. Presumably GCC 4 is holding the idle loop
count in a register. Any other comments before I check this in?

John Dallaway
eCos maintainer

Re: CYGPKG_CPULOAD fixes

[John Dallaway]

Hi Lars

John Dallaway wrote:

> Lars Povlsen wrote:
> 
>> Here's a small fix to the cpuload package (and idle thread).
>>
>> In short, a missing 'volatile' attribute to the loop counter causes
>> newer (arm-eabi GCC 4.2.3/eCosentric toolchain) to produce wrong
>> results.
>>
>> This patch adds a volatile modifier to idle_thread_loops[], and updates
>> the reference to it from the cpuload package. (which was also using a
>> non-array declaration).
> 
> This all seems reasonable. Presumably GCC 4 is holding the idle loop
> count in a register. Any other comments before I check this in?

Sanity tested on ARM and checked in. Thank you for the patch.

There are still issues with the cpuload package when built at "-O2"
optimisation. Ref:

  http://ecos.sourceware.org/ml/ecos-discuss/2007-06/msg00175.html

John Dallaway
eCos maintainer