Date:2011-11-12 13:22:48 (7 years 7 months ago)
Author:Werner Almesberger
Commit:aa0022d97376840b33f6945d8fcac2bea6ab5796
Message:m1/patches/rtems/: pending RTEMS patches (relative to latest CVS)

Files: m1/patches/rtems/chain-first-last.patch (1 diff)
m1/patches/rtems/coremsgsubmit-race.patch (1 diff)
m1/patches/rtems/lm32-stack-alignment.patch (1 diff)
m1/patches/rtems/rbtree-container-of.patch (1 diff)
m1/patches/rtems/series (1 diff)

Change Details

m1/patches/rtems/chain-first-last.patch
1This issue is under consideration:
2http://www.rtems.org/pipermail/rtems-users/2011-November/009130.html
3
4Doubly-linked lists ("chains") in RTEMS have a "control" block that
5looks like the next/prev link pair in an element. The list elements
6link both ways to this control block.
7
8_Chain_Is_first and _Chain_Is_last only probed if the link to the
9next element - which would be the control block - is non-NULL.
10Telling by the function description and given that there are already
11functions called _Chain_Is_head and _Chain_Is_tail (which could be
12simplified), this is probably not the intended behaviour.
13
14This also affects the aliases rtems_chain_is_first and
15rtems_chain_is_last.
16
17These functions are not used a lot and I haven't seen any immediate
18effect on M1 after changing them, so I can't say whether this patch
19may unearth other problems.
20
21- Werner
22
23Index: rtems/cpukit/score/inline/rtems/score/chain.inl
24===================================================================
25--- rtems.orig/cpukit/score/inline/rtems/score/chain.inl 2011-11-12 09:12:46.000000000 -0300
26@@ -297,7 +297,7 @@
27   const Chain_Node *the_node
28 )
29 {
30- return (the_node->previous == NULL);
31+ return the_node->previous->previous == NULL;
32 }
33
34 /** @brief Is this the Last Node on the Chain
35@@ -314,7 +314,7 @@
36   const Chain_Node *the_node
37 )
38 {
39- return (the_node->next == NULL);
40+ return the_node->next->next == NULL;
41 }
42
43 /** @brief Does this Chain have only One Node
m1/patches/rtems/coremsgsubmit-race.patch
1This issue is under review:
2https://www.rtems.org/bugzilla/show_bug.cgi?id=1961
3
4If it's permissible to call rtems_message_queue_send from an
5interrupt, then there is at least one race condition in the core
6message subsystem.
7
8This created the MIDI/mouse hang we love so much on M1.
9
10The problem is as follows: RTEMS queues use pre-allocated message
11buffers that are kept on an "inactive" (free) list. When enqueuing
12a message, a buffer is first removed from the inactive list, data
13it copied to it, and it is then added to the pending list.
14
15The reverse happens when dequeuing. Besides these two queues, there
16is also a counter called number_of_pending_messages keeping track,
17as the name suggests, of the number of pending messages. It is
18updated atomically together with changes to the pending buffers
19list.
20
21From the above it is clear that the counter will be out of sync with
22the inactive list between the beginning and the end of an enqueue or
23dequeue operation.
24
25In order to minimize interrupt latency, RTEMS disables interrupts
26only when adding and removing buffers from lists, but not throughout
27the whole enqueuing/dequeuing operation. Instead, it disables the
28scheduler during these operations, but this doesn't prevent
29interrupts.
30
31This means that the inconsistency between number_of_pending_messages
32and the inactive list can be observed from an interrupt handler if
33enqueuing or dequeuing is in progress.
34
35_CORE_message_queue_Submit checks whether there is still room in the
36queue by reading number_of_pending_messages. If there is room, it
37then calls _CORE_message_queue_Allocate_message_buffer to obtain a
38free buffer.
39
40Given that number_of_pending_messages and the list of inactive
41buffers can disagree, e.g., if _CORE_message_queue_Seize or another
42_CORE_message_queue_Submit is executing concurrently,
43_CORE_message_queue_Allocate_message_buffer may fail to obtain a
44free buffer despite the prior test.
45
46_CORE_message_queue_Allocate_message_buffer can detect a lack of
47free buffers and indicates it by returning a NULL pointer. Checking
48whether NULL has been returned instead of a buffer is optional and
49depends on RTEMS_DEBUG.
50
51If no check is performed, _CORE_message_queue_Submit will then try
52to use the buffer. In the absence of hardware detecting the
53de-referencing of NULL pointers, the wounded system will limp on a
54little further until, at least in the case of M1, it finally hangs
55somewhere.
56
57The patch below avoids the problem in the scenario described above
58by not using number_of_pending_messages as an indicator of whether
59free buffers are available, but by simply trying to get a buffer,
60and handling the result of failure.
61
62This is similar to how _CORE_message_queue_Seize works.
63
64Another possibility would be to make testing of the_message no
65longer optional. But then, there would basically be two tests for
66the same condition, which is ugly.
67
68- Werner
69
70Index: rtems/cpukit/score/src/coremsgsubmit.c
71===================================================================
72--- rtems.orig/cpukit/score/src/coremsgsubmit.c 2011-11-12 09:15:12.000000000 -0300
73@@ -101,21 +101,9 @@
74    * No one waiting on the message queue at this time, so attempt to
75    * queue the message up for a future receive.
76    */
77- if ( the_message_queue->number_of_pending_messages <
78- the_message_queue->maximum_pending_messages ) {
79-
80- the_message =
81- _CORE_message_queue_Allocate_message_buffer( the_message_queue );
82-
83- #if defined(RTEMS_DEBUG)
84- /*
85- * NOTE: If the system is consistent, this error should never occur.
86- */
87-
88- if ( !the_message )
89- return CORE_MESSAGE_QUEUE_STATUS_UNSATISFIED;
90- #endif
91-
92+ the_message =
93+ _CORE_message_queue_Allocate_message_buffer( the_message_queue );
94+ if ( the_message ) {
95     _CORE_message_queue_Copy_buffer(
96       buffer,
97       the_message->Contents.buffer,
m1/patches/rtems/lm32-stack-alignment.patch
1This patch is under review:
2https://www.rtems.org/bugzilla/show_bug.cgi?id=1956
3
4The comments in cpukit/score/cpu/lm32/rtems/score/cpu.h state that
5CPU_STACK_ALIGNMENT should either be 0 or >= CPU_ALIGNMENT. The
6latter was 8, the former is 4.
7
8Further investigation revealed that, contrary to what the comment
9says, 0 is not a valid value for CPU_STACK_ALIGNMENT.
10
11This patch sets CPU_ALIGNMENT to 4, since we don't have any machine
12word larger than that. (doubles and long longs are handled by
13software and either extremely slow already or rare.)
14
15The patch also corrects the misleading comment before
16CPU_STACK_ALIGNMENT.
17
18I'm not sure if this fix has any real-life impact on M1 behaviour,
19but I guess it can't hurt.
20
21- Werner
22
23Index: rtems/cpukit/score/cpu/lm32/rtems/score/cpu.h
24===================================================================
25--- rtems.orig/cpukit/score/cpu/lm32/rtems/score/cpu.h 2011-11-12 03:01:35.000000000 -0300
26@@ -637,7 +637,7 @@
27  *
28  * XXX document implementation including references if appropriate
29  */
30-#define CPU_ALIGNMENT 8
31+#define CPU_ALIGNMENT 4
32
33 /**
34  * This number corresponds to the byte alignment requirement for the
35@@ -687,9 +687,10 @@
36  * stack. This alignment requirement may be stricter than that for the
37  * data types alignment specified by @ref CPU_ALIGNMENT. If the
38  * @ref CPU_ALIGNMENT is strict enough for the stack, then this should be
39- * set to 0.
40+ * set to @ref CPU_ALIGNMENT.
41  *
42- * @note This must be a power of 2 either 0 or greater than @ref CPU_ALIGNMENT.
43+ * @note This must be a power of 2 either equal to or greater than
44+ * @ref CPU_ALIGNMENT.
45  *
46  * Port Specific Information:
47  *
m1/patches/rtems/rbtree-container-of.patch
1This issue is under consideration:
2http://www.rtems.org/pipermail/rtems-users/2011-November/009097.html
3
4There seem to be two issues in the original code:
5
6- the "node" argument of the macro is not protected, which could lead
7  to very hard to find errors (this doesn't seem to cause any
8  immediate problems just now, but it's a lousy risk to take)
9
10- more seriously, "offsetof" counts in bytes while arithmentic on the
11  "node" pointer counts in multiples of whatever size that object has
12
13RTEMS with this patch applied runs well (on Milkymist), but I don't
14know if the code in question actually is executed.
15
16- Werner
17
18Index: rtems/cpukit/score/include/rtems/score/rbtree.h
19===================================================================
20--- rtems.orig/cpukit/score/include/rtems/score/rbtree.h 2011-11-12 08:52:50.000000000 -0300
21@@ -90,7 +90,9 @@
22  *
23  */
24 #define _RBTree_Container_of(node,container_type, node_field_name) \
25- ((container_type*) (node - offsetof(container_type,node_field_name)))
26+ ((container_type*) ((void *) (node) - \
27+ offsetof(container_type,node_field_name)))
28+
29
30 /**
31  * This type indicates the direction.
m1/patches/rtems/series
1lm32-stack-alignment.patch
2rbtree-container-of.patch
3chain-first-last.patch
4coremsgsubmit-race.patch

Archive Download the corresponding diff file

Branches:
master



interactive