Werner's Miscellanea
Sign in or create your account | Project List | Help
Werner's Miscellanea Git Source Tree
Root/
Source at commit f6fd776f528905e346c7f1caec97945535c7ca43 created 12 years 4 months ago. By Werner Almesberger, m1/patches/rtems/: Milkymist-specific patches are in upstream (update by Xiangfu) | |
---|---|
1 | This issue is under review: |
2 | https://www.rtems.org/bugzilla/show_bug.cgi?id=1961 |
3 | |
4 | If it's permissible to call rtems_message_queue_send from an |
5 | interrupt, then there is at least one race condition in the core |
6 | message subsystem. |
7 | |
8 | This created the MIDI/mouse hang we love so much on M1. |
9 | |
10 | The problem is as follows: RTEMS queues use pre-allocated message |
11 | buffers that are kept on an "inactive" (free) list. When enqueuing |
12 | a message, a buffer is first removed from the inactive list, data |
13 | it copied to it, and it is then added to the pending list. |
14 | |
15 | The reverse happens when dequeuing. Besides these two queues, there |
16 | is also a counter called number_of_pending_messages keeping track, |
17 | as the name suggests, of the number of pending messages. It is |
18 | updated atomically together with changes to the pending buffers |
19 | list. |
20 | |
21 | From the above it is clear that the counter will be out of sync with |
22 | the inactive list between the beginning and the end of an enqueue or |
23 | dequeue operation. |
24 | |
25 | In order to minimize interrupt latency, RTEMS disables interrupts |
26 | only when adding and removing buffers from lists, but not throughout |
27 | the whole enqueuing/dequeuing operation. Instead, it disables the |
28 | scheduler during these operations, but this doesn't prevent |
29 | interrupts. |
30 | |
31 | This means that the inconsistency between number_of_pending_messages |
32 | and the inactive list can be observed from an interrupt handler if |
33 | enqueuing or dequeuing is in progress. |
34 | |
35 | _CORE_message_queue_Submit checks whether there is still room in the |
36 | queue by reading number_of_pending_messages. If there is room, it |
37 | then calls _CORE_message_queue_Allocate_message_buffer to obtain a |
38 | free buffer. |
39 | |
40 | Given that number_of_pending_messages and the list of inactive |
41 | buffers 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 |
44 | free buffer despite the prior test. |
45 | |
46 | _CORE_message_queue_Allocate_message_buffer can detect a lack of |
47 | free buffers and indicates it by returning a NULL pointer. Checking |
48 | whether NULL has been returned instead of a buffer is optional and |
49 | depends on RTEMS_DEBUG. |
50 | |
51 | If no check is performed, _CORE_message_queue_Submit will then try |
52 | to use the buffer. In the absence of hardware detecting the |
53 | de-referencing of NULL pointers, the wounded system will limp on a |
54 | little further until, at least in the case of M1, it finally hangs |
55 | somewhere. |
56 | |
57 | The patch below avoids the problem in the scenario described above |
58 | by not using number_of_pending_messages as an indicator of whether |
59 | free buffers are available, but by simply trying to get a buffer, |
60 | and handling the result of failure. |
61 | |
62 | This is similar to how _CORE_message_queue_Seize works. |
63 | |
64 | Another possibility would be to make testing of the_message no |
65 | longer optional. But then, there would basically be two tests for |
66 | the same condition, which is ugly. |
67 | |
68 | - Werner |
69 | |
70 | Index: rtems/cpukit/score/src/coremsgsubmit.c |
71 | =================================================================== |
72 | --- rtems.orig/cpukit/score/src/coremsgsubmit.c 2011-11-12 09:15:12.000000000 -0300 |
73 | +++ rtems/cpukit/score/src/coremsgsubmit.c 2011-11-12 09:15:17.000000000 -0300 |
74 | @@ -101,21 +101,9 @@ |
75 | * No one waiting on the message queue at this time, so attempt to |
76 | * queue the message up for a future receive. |
77 | */ |
78 | - if ( the_message_queue->number_of_pending_messages < |
79 | - the_message_queue->maximum_pending_messages ) { |
80 | - |
81 | - the_message = |
82 | - _CORE_message_queue_Allocate_message_buffer( the_message_queue ); |
83 | - |
84 | - #if defined(RTEMS_DEBUG) |
85 | - /* |
86 | - * NOTE: If the system is consistent, this error should never occur. |
87 | - */ |
88 | - |
89 | - if ( !the_message ) |
90 | - return CORE_MESSAGE_QUEUE_STATUS_UNSATISFIED; |
91 | - #endif |
92 | - |
93 | + the_message = |
94 | + _CORE_message_queue_Allocate_message_buffer( the_message_queue ); |
95 | + if ( the_message ) { |
96 | _CORE_message_queue_Copy_buffer( |
97 | buffer, |
98 | the_message->Contents.buffer, |
99 |
Branches:
master