Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Posix - optionally disable use of some realtime features for debugging #67

Closed
skliper opened this issue Sep 30, 2019 · 17 comments
Closed
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

It was discovered that the pthreads library supplied by Xilinx for their Microblaze platform does not properly support the PTHREAD_PRIO_INHERIT attribute on mutexes.

The problem occurs when a higher-priority thread becomes blocked on a mutex owned by a lower-priority thread, in this pthreads implementation the higher priority task starts "spinning" and ultimately uses 100% CPU, locking out any other process, including the ability to kill or stop the process - reboot is the only recourse.

This bug is a problem in the Xilinx-supplied pthreads library, but until a real fix is done, we need to disable the PTHREAD_PRIO_INHERIT option. Disabling this feature may also be useful to others during debugging if another user runs into a similar issue.

It was also useful to disable RT scheduling entirely while debugging this problem.

This ticket will add compile-time macros that can be added to the "osconfig.h" file that will control these features. The default (if nothing is defined) will be to use the same features that are currently in place (no change).

@skliper skliper added this to the osal-5.0.0 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 44. Created by jphickey on 2015-05-12T09:38:21, last modified: 2019-07-26T15:13:27

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2015-05-12 09:42:53:

The EVA team is already running with INHERIT attribute disabled as a local modification. This change will make it so they can run with an unmodified/official OSAL revision.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2015-05-19 14:11:02:

Pushed commit [changeset:7e5de18] for review

The EVA team needs this as an interim workaround in order to run on the Microblaze platform

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by acudmore on 2015-06-01 13:21:11:

Is there a vxWorks implementation for disabling mutex priority inheritance?

Is an option going to be included to disable RT scheduling?

But I approve of the concept and the implementation of the mutex option for vxWorks and RTEMS.

Something to think about:
Would it make sense to have OS specific sections in osconfig.h? For example: have a section of RTEMS options, POSIX options, and vxWorks options ( FreeRTOS, ARINC, etc ). This would make it clearer that the osconfig.h option applied to a particular target OS rather than potentially one or more OSs. ( although the priority inheritance option in this ticket is actually a global option )

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by sstrege on 2015-06-02 12:27:12:

The change set does not show the new compile time macros in the osconfig.h file. Were these added under a different ticket? What is the default for these new macros?

Recommend/accept

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jwilmot on 2015-06-02 12:28:49:

Approve. Add comments/documentation as to why a user might do this. The default should be INHERIT = TRUE

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2015-06-02 12:41:20:

Replying to [comment:4 sstrege]:

The change set does not show the new compile time macros in the osconfig.h file. Were these added under a different ticket? What is the default for these new macros?

The macro is set up such that the //default// behavior (INHERIT == TRUE) is assumed if the macro is undefined. Therefore no changes are necessary to osconfig.h and compatibility is maintained with older config files.

That being said, I have been maintaining a sample mission in the cfs_test repo, and I did push a changeset which adds the options as "undef" along with a description.

Link to commit: [cfs_test:changeset:0909b8e]

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2015-08-25 11:58:53:

Approve.

I suspect any mission that has to turn this on will have a
comment above the definition saying ''unkind'' things about Xinlix.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-04-05 17:38:01:

Sending back to review, original changeset missing.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2019-04-11 16:58:41:

Checked into the status of this one.

The open issue/question is whether there is value in allowing "basic" mutex behavior via an osconfig.h option or not. Traditionally, OSAL always creates priority inversion safe mutexes, which escalate the priority of the owner task to that of the highest priority task blocked on the mutex.

From the CCB standpoint, here are the discussion points:

  • Right now this option exists in posix-ng and rtems-ng but not the others.
  • In the years since dealing with this I have not come across another platform with a similar bug, so it is probably not worth keeping this option around for that reason.
  • Other than as a workaround for a bug, do we foresee any general-purpose use case requiring "basic" mutex (non priority-inversion protected) rather than the default inversion-protected type?
  • if YES, then we just need to agree on a name for the option in osconfig.h and document it.
  • if NO, then I would propose just a bit of clean up to some existing #ifdef statements and this can be closed.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-04-24 12:31:59:

CCB 4/24/19 - discussed, and answer is "NO" there aren't valid use cases for non priority-inversion protected mutex

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2019-06-11 19:18:55:

Cleaned up in commit [changeset:fc64d0f], branch trac-44-cleanup-priority-inheritance.

Ready for review.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-06-13 11:27:49:

CCB 6/12/19 - CCB code reviewed and approved

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2019-06-25 11:55:01:

Merged into ic-ccb-20190612

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-07-10 13:23:34:

CCB 07/10/2019 - Approved for merge to dev

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2019-07-10 13:45:18:

Now in the "development" branch

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-07-26 15:13:27:

Milestone renamed

@skliper skliper closed this as completed Sep 30, 2019
@skliper skliper removed their assignment Sep 30, 2019
@skliper skliper modified the milestones: osal-5.0.0, 5.0.0 Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant