Fixed bugs 2789377 and 2789383.

git-svn-id: svn://svn.code.sf.net/p/chibios/svn/trunk@956 35acf78f-673a-0410-8e92-d51de3d6d3f4

docs/src/main.dox

@@ -83,6 +83,7 @@
  * - @subpage test_threads
  * - @subpage test_dynamic
  * - @subpage test_msg
+ * - @subpage test_mtx
  * - @subpage test_events
  * - @subpage test_mbox
  * - @subpage test_queues
@@ -230,8 +231,8 @@
  * In ChibiOS/RT the Unlock operations are always performed in Lock-reverse
  * order. The Unlock API does not even have a parameter, the mutex to unlock
  * is taken from an internal stack of owned mutexes.
- * This both improves the performance and is required by the priority
- * inheritance mechanism.
+ * This both improves the performance and is required by an efficient
+ * implementation of the priority inheritance mechanism.
  *
  * <h2>The priority inversion problem</h2>
  * The mutexes in ChibiOS/RT implements the <b>full</b> priority

readme.txt

@@ -75,6 +75,8 @@ Win32-MinGW            - ChibiOS/RT simulator and demo into a WIN32 process,
 - FIX: Found new instances of the obsolete function chSysGetTime() in the
   C++ wrapper and in the WEB demo (bug 2772237)(backported in stable branch).
 - FIX: Fixed macro in test.h (bug 2781176)(backported in stable branch).
+- FIX: Fixed sequence assertion in test.c (bug 2789377)(backported in stable
+  branch).
 - NEW: Abstract I/O Channels mechanism introduced. This mechanism allows to
   access I/O resources through a standard interface and hides implementation
   details. The existing serial drivers were modified to offer a standard

test/test.c

@@ -145,6 +145,7 @@ bool_t _test_assert_sequence(unsigned point, char *expected) {
   }
   if (*expected)
     return _test_fail(point);
+  clear_tokens();
   return FALSE;
 }
 
@@ -174,16 +175,17 @@ void test_wait_threads(void) {
     }
 }
 
-void test_cpu_pulse(unsigned ms) {
+#if CH_DBG_THREADS_PROFILING
+void test_cpu_pulse(unsigned duration) {
 
-  systime_t duration = MS2ST(ms);
-  systime_t start = chTimeNow();
-  while (chTimeIsWithin(start, start + duration)) {
+  systime_t end = chThdSelf()->p_time + MS2ST(duration);
+  while (chThdSelf()->p_time < end) {
 #if defined(WIN32)
     ChkIntSources();
 #endif
   }
 }
+#endif
 
 systime_t test_wait_tick(void) {
 

test/test.h

@@ -62,8 +62,10 @@ extern "C" {
   void test_terminate_threads(void);
   void test_wait_threads(void);
   systime_t test_wait_tick(void);
-  void test_cpu_pulse(unsigned ms);
   void test_start_timer(unsigned ms);
+#if CH_DBG_THREADS_PROFILING
+  void test_cpu_pulse(unsigned duration);
+#endif
 #if defined(WIN32)
   void ChkIntSources(void);
 #endif
@@ -72,7 +74,7 @@ extern "C" {
 #endif
 
 #define test_fail(point) {                                              \
-  _test_fail(point);                                                     \
+  _test_fail(point);                                                    \
   return;                                                               \
 }
 

test/testdyn.c

@@ -30,7 +30,7 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the dynamic APIs code
- * as a necessary step in order to assess their readyness.
+ * as a necessary step in order to assess their maturity.
  *
  * <h2>Preconditions</h2>
  * The module requires the following kernel options:

test/testevt.c

@@ -29,7 +29,7 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the @ref Events subsystem
- * code as a necessary step in order to assess its readyness.
+ * code as a necessary step in order to assess its maturity level.
  *
  * <h2>Preconditions</h2>
  * The module requires the following kernel options:

test/testheap.c

@@ -29,7 +29,7 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the @ref Heap subsystem
- * code as a necessary step in order to assess its readyness.
+ * code as a necessary step in order to assess its maturity level.
  *
  * <h2>Preconditions</h2>
  * The module requires the following kernel options:

test/testmbox.c

@@ -29,7 +29,10 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the @ref Mailboxes
- * subsystem code as a necessary step in order to assess its readyness.
+ * subsystem code as a necessary step in order to assess its maturity
+ * level.<br>
+ * Note that the @ref Mailboxes subsystem depends on the @ref Semaphores
+ * subsystem that has to met its testing objectives as well.
  *
  * <h2>Preconditions</h2>
  * The module requires the following kernel options:

test/testmsg.c

@@ -29,7 +29,7 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the @ref Messages
- * subsystem code as a necessary step in order to assess its readyness.
+ * subsystem code as a necessary step in order to assess its maturity level.
  *
  * <h2>Preconditions</h2>
  * The module requires the following kernel options:

test/testmtx.c

@@ -21,6 +21,44 @@
 
 #include "test.h"
 
+/**
+ * @page test_mtx Mutexes test
+ *
+ * <h2>Description</h2>
+ * This module implements the test sequence for the @ref Mutexes and
+ * @ref CondVars subsystems.<br>
+ * Tests on those subsystems are particularly critical because the system-wide
+ * implications of the Priority Inheritance mechanism.
+ *
+ * <h2>Objective</h2>
+ * Objective of the test module is to cover 100% of the subsystem code
+ * as a necessary step in order to assess their maturity level.
+ *
+ * <h2>Preconditions</h2>
+ * The module requires the following kernel options:
+ * - @p CH_USE_MUTEXES
+ * - @p CH_USE_CONDVARS
+ * - @p CH_DBG_THREADS_PROFILING
+ * .
+ * In case some of the required options are not enabled then some or all tests
+ * may be skipped.
+ *
+ * <h2>Test Cases</h2>
+ * - @subpage test_mtx_001
+ * - @subpage test_mtx_002
+ * - @subpage test_mtx_003
+ * - @subpage test_mtx_004
+ * - @subpage test_mtx_005
+ * - @subpage test_mtx_006
+ * - @subpage test_mtx_007
+ * - @subpage test_mtx_008
+ * .
+ * @file testmtx.c
+ * @brief @ref Mutexes and @ref CondVars test source file
+ * @file testmtx.h
+ * @brief @ref Mutexes and @ref CondVars test header file
+ */
+
 #if CH_USE_MUTEXES
 
 #define ALLOWED_DELAY 5
@@ -28,6 +66,15 @@
 static Mutex m1, m2;
 static CondVar c1;
 
+/**
+ * @page test_mtx_001 Priority enqueuing test
+ *
+ * <h2>Description</h2>
+ * Five threads, with increasing priority, are enqueued on a locked mutex then
+ * the mutex is unlocked.<br>
+ * The test expects the threads to perform their operations in increasing
+ * priority order regardless of the initial order.
+ */
 static char *mtx1_gettest(void) {
 
   return "Mutexes, priority enqueuing test";
@@ -68,6 +115,38 @@ const struct testcase testmtx1 = {
   mtx1_execute
 };
 
+/**
+ * @page test_mtx_002 Priority inheritance, simple case
+ *
+ * <h2>Description</h2>
+ * Three threads are involved in the classic priority inversion scenario, a
+ * medium priority thread tries to starve an high priority thread by
+ * blocking a low priority thread into a mutex lock zone.<br>
+ * The test expects the threads to perform their operations in increasing
+ * priority order by rearranging their priorities in order to avoid the
+ * priority inversion trap.
+ *
+ * <h2>Scenario</h2>
+ * This weird looking diagram should explain what happens in the test case:
+ * @code
+ * Time ----> 0     10    20    30    40    50    60    70    80    90    100
+ *    0 ......AL++++++++++............2+++++++++++AU0---------------++++++G...
+ *    1 ..................++++++++++++------------------++++++++++++G.........
+ *    2  .............................AL..........++++++AUG...................
+ *                                    ^           ^
+ *
+ * Legend:
+ *   0..2 - Priority levels
+ *   +++  - Running
+ *   ---  - Ready
+ *   ...  - Waiting
+ *   AL   - Lock operation on mutex A
+ *   AUn  - Unlock operation on mutex A with priority going to level 'n'
+ *   G    - Goal
+ *   ^    - Priority transition (boost or return).
+ * @endcode
+ */
+
 static char *mtx2_gettest(void) {
 
   return "Mutexes, priority inheritance, simple case";
@@ -78,45 +157,48 @@ static void mtx2_setup(void) {
   chMtxInit(&m1);
 }
 
-static msg_t thread2(void *p) {
+/* Low priority thread */
+static msg_t thread2L(void *p) {
 
-  chThdSleepMilliseconds(10);
   chMtxLock(&m1);
+  test_cpu_pulse(40);
   chMtxUnlock();
-  test_emit_token(*(char *)p);
+  test_cpu_pulse(10);
+  test_emit_token('C');
   return 0;
 }
 
-static msg_t thread3(void *p) {
-
-  chMtxLock(&m1);
-  chThdSleepMilliseconds(40);
-  chMtxUnlock();
-  test_emit_token(*(char *)p);
-  return 0;
-}
-
-static msg_t thread4(void *p) {
+/* Medium priority thread */
+static msg_t thread2M(void *p) {
 
   chThdSleepMilliseconds(20);
-  test_cpu_pulse(50);
-  test_emit_token(*(char *)p);
+  test_cpu_pulse(40);
+  test_emit_token('B');
   return 0;
 }
 
-/*
- * Time
- *    0 ++++++++++++++++++AL+....2++++++++++++++AU0------------------------------
- *    1 .....................++--------------------------------------------------
- *    2 .......................++AL.............+++++++++AU++++++++++++++++++++++
- */
-static void mtx2_execute(void) {
+/* High priority thread */
+static msg_t thread2H(void *p) {
 
-  threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()-1, thread2, "A");
-  threads[1] = chThdCreateStatic(wa[1], WA_SIZE, chThdGetPriority()-3, thread3, "C");
-  threads[2] = chThdCreateStatic(wa[2], WA_SIZE, chThdGetPriority()-2, thread4, "B");
+  chThdSleepMilliseconds(40);
+  chMtxLock(&m1);
+  test_cpu_pulse(10);
+  chMtxUnlock();
+  test_emit_token('A');
+  return 0;
+}
+
+static void mtx2_execute(void) {
+  systime_t time;
+
+  test_wait_tick();
+  time = chTimeNow();
+  threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()-1, thread2H, 0);
+  threads[1] = chThdCreateStatic(wa[1], WA_SIZE, chThdGetPriority()-3, thread2L, 0);
+  threads[2] = chThdCreateStatic(wa[2], WA_SIZE, chThdGetPriority()-2, thread2M, 0);
   test_wait_threads();
   test_assert_sequence(1, "ABC");
+  test_assert_time_window(2, time + MS2ST(100), time + MS2ST(100) + ALLOWED_DELAY);
 }
 
 const struct testcase testmtx2 = {
@@ -126,6 +208,38 @@ const struct testcase testmtx2 = {
   mtx2_execute
 };
 
+/**
+ * @page test_mtx_003 Priority inheritance, complex case
+ *
+ * <h2>Description</h2>
+ * Five threads are involved in the complex priority inversion scenario,
+ * please refer to the diagram below for the complete scenario.<br>
+ * The test expects the threads to perform their operations in increasing
+ * priority order by rearranging their priorities in order to avoid the
+ * priority inversion trap.
+ *
+ * <h2>Scenario</h2>
+ * This weird looking diagram should explain what happens in the test case:
+ * @code
+ * Time ---->
+ *         0     10    20        30   40    50
+ *    0 +++BL++------------------2++++------4+++++BU0--------------------------
+ *    1 .......++AL++--2+++++++++BL.........4.....++++++++BU4++++AU1-----------
+ *    2 .............++AL............................................------++AU
+ *    3 ..............................++++-------------------------------++....
+ *    4 ..................................++AL...................++++AU++......
+ *                                    ^              ^
+ *
+ * Legend:
+ *   0..2 - Priority levels
+ *   +++  - Running
+ *   ---  - Ready
+ *   ...  - Waiting
+ *   AL   - Lock operation on mutex A
+ *   AUn  - Unlock operation on mutex A with priority going to level 'n'
+ *   ^    - Priority transition (boost or return).
+ * @endcode
+ */
 static char *mtx3_gettest(void) {
 
   return "Mutexes, priority inheritance, complex case";

test/testqueues.c

@@ -32,7 +32,7 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the @ref IOQueues code
- * as a necessary step in order to assess its readyness.<br>
+ * as a necessary step in order to assess its maturity level.<br>
  * Note that the @ref IOQueues subsystem depends on the @ref Semaphores
  * subsystem that has to met its testing objectives as well.
  *

test/testserial.c

@@ -32,7 +32,7 @@
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the @ref Serial code
- * as a necessary step in order to assess its readyness.<br>
+ * as a necessary step in order to assess its maturity level.<br>
  * Note that the @ref Serial subsystem depends on the @ref Semaphores and
  * @ref Events subsystems that have to met their testing objectives as well.
  *

test/testthd.c

@@ -28,12 +28,12 @@
  * This module implements the test sequence for the @ref Scheduler,
  * @ref Threads and @ref Time subsystems.<br>
  * Note that the tests on those subsystems are formally required but most of
- * their functionality is already demostrated because the test suite itself
- * depends on them, anyway doublecheck is good.
+ * their functionality is already demonstrated because the test suite itself
+ * depends on them, anyway double check is good.
  *
  * <h2>Objective</h2>
  * Objective of the test module is to cover 100% of the subsystems code
- * as a necessary step in order to assess their readyness.
+ * as a necessary step in order to assess their maturity level.
  *
  * <h2>Preconditions</h2>
  * None.

todo.txt

@@ -8,11 +8,11 @@ After 1.2.0:
 * Abstract I/O channels rather than just serial ports.
   ? Move the serial drivers implementations in library. Better keep the core
     as compact as possible.
+X Add tests documentation to the general documentation via doxygen.
 - Static object initializers.
 - Remove any instance of unnamed structures/unions.
 - Objects registry in the kernel.
 - OSEK-style simple tasks within the idle thread.
-- Add tests documentation to the general documentation via doxygen.
 - Code examples into the documentation.
 - Dedicated syscalls.c support for newlib users.
 - Threads Pools manager in the library.