#3000 closed defect (fixed)
Setting interrupt level in the mode arg on SMP returns RTEMS_UNSATISFIED
Reported by: | Chris Johns | Owned by: | Joel Sherrill |
---|---|---|---|
Priority: | normal | Milestone: | 5.1 |
Component: | score | Version: | 5 |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: |
Description (last modified by Chris Johns)
If for any reason a user sets the interrupt level in the mode on an SMP build the error RTEMS_UNSATISFIED
is returned. The documentation indicates this is a lack of stack and this confusing.
The reason this happens is the SMP check for an interrupt level being set is in the score's _Thread_Initialize
. I propose that and is_preemptible
check be converted to an assert
and checks be added to the Classic API to catch these errors and report suitable error codes.
There is no meaningful error code available without abusing an existing one so I propose adding RTEMS_INVALID_MODE
.
Change History (19)
comment:1 follow-up: 2 Changed on 04/19/17 at 03:28:50 by Joel Sherrill
comment:2 Changed on 04/19/17 at 04:42:23 by Chris Johns
Replying to Joel Sherrill:
I think this is a generally a good idea but there are a number of pieces to do this completely:
Great and yes.
- the status has to also be returned by rtems_task_mode()
OK.
- add status to list in rtems status to string
Done in the posted patch.
- add status to list in Ada binding. Should be mechanical.
Is there any documentation on how to do this?
Answer: Edit rtems.ads and add it. It is just an enumerated
list. You won't have trouble. c/src/ada/rtems.ads around line
353.
- add status to status table in Classic API manual
- add new status to rtems_task_create() documentation
- add new status to rtems_task_mode() documentation
I will create a docs patch once I have a suitable patch for master
.
- add test for rtems_task_create() new error
- add test for rtems_task_mode() new error
Yes however the tests are only running in the single core mode even when built with SMP so the error is not able to be tripped.
ANSWER: Sounds like a new SMP test is needed.
I am unsure if the assert() is debug mode only. That needs to be clarified.
That is correct. The assert's catch a bug in a user of the score API when built with RTEMS_DEBUG
. The users of the call need to protect the score thread initialise call.
Sorry to be pedantic with the list but this looks like the type of change that is small but has tentacles that would be easy to miss pieces. I suspect my list isn't complete yet.
This looks the list to me.
comment:3 follow-up: 4 Changed on 05/02/17 at 05:34:57 by Sebastian Huber
See smpunsupported01 test.
comment:4 follow-up: 5 Changed on 05/02/17 at 06:28:22 by Chris Johns
Replying to Sebastian Huber:
See smpunsupported01 test.
What what? Could you please elaborate a little more?
Thanks
comment:5 Changed on 05/02/17 at 06:54:04 by Sebastian Huber
Replying to Chris Johns:
Replying to Sebastian Huber:
See smpunsupported01 test.
What what? Could you please elaborate a little more?
This is the test program the new error cases should be added.
comment:6 Changed on 10/12/17 at 17:46:04 by Chris Johns
Description: | modified (diff) |
---|
comment:7 Changed on 10/12/17 at 18:46:16 by Joel Sherrill
Updated work list:
- Whine that this was easier to report than to fix.
- CPUKit Changes
- (PREVIOUSLY COMMITTED) Add status to enumeration
- (PREVIOUSLY COMMITTED) Add status to list in rtems status to string
- (PREVIOUSLY COMMITTED) Add status to list in Ada binding. Should be mechanical
- (PREVIOUSLY COMMITTED) Add both error checks to rtems_task_create()
- (CLEAN UP PASS) Add both error checks to rtems_task_mode()
- Documentation
- (ADDED) Add status to status table in Classic API manual
- (ADDED) Add new status to rtems_task_create() documentation
- (ADDED) Add new status to rtems_task_mode() documentation
- Testing
- (PREVIOUSLY COMMITTED) Add rtems_task_create() test case(s) to smpunsupported01
- (ONE PREVIOUSLY COMMITTED, ONE ADDED) Add rtems_task_mode() test case(s) to smpunsupported01
Patches for "CLEAN UP PASS" and "ADDED" submitted to devel@ mailing list. Pending review to be committed and close this.
comment:8 Changed on 10/12/17 at 18:51:18 by Joel Sherrill
Owner: | changed from Chris Johns to Joel Sherrill |
---|
comment:9 Changed on 11/09/17 at 06:27:14 by Sebastian Huber
Milestone: | 4.12.0 → 5.1 |
---|
Milestone renamed
comment:11 Changed on 12/06/17 at 18:49:57 by Joel Sherrill <joel@…>
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:12 Changed on 12/07/17 at 08:22:31 by Sebastian Huber
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Tests tm04 and tm20 fail now with --enable-smp.
comment:14 Changed on 01/19/18 at 06:46:33 by Sebastian Huber <sebastian.huber@…>
In a5b4db4b/rtems:
comment:15 Changed on 04/13/18 at 01:46:14 by Chris Johns
Can this ticket be closed? If it cannot be closed what is outstanding to be done?
comment:16 Changed on 10/14/18 at 00:29:16 by Joel Sherrill
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:18 Changed on 01/09/19 at 09:37:14 by Sebastian Huber <sebastian.huber@…>
In 1f285186/rtems:
comment:19 Changed on 01/09/19 at 09:37:17 by Sebastian Huber <sebastian.huber@…>
In 3bd39999/rtems:
I think this is a generally a good idea but there are a number of pieces to do this completely:
I am unsure if the assert() is debug mode only. That needs to be clarified.
Sorry to be pedantic with the list but this looks like the type of change that is small but has tentacles that would be easy to miss pieces. I suspect my list isn't complete yet.