Skip to content

Added poll wakeup mechanism in the presence of rtos #9113

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

Closed

Conversation

embeddedteam103
Copy link
Contributor

Description

Added a mechanism to NOT busy wait in poll but instead register an event flag to the sigios of all file handles we wait upon and the polling thread is properly waiting.
Resolves #9112

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@ciarmcom ciarmcom requested review from a team December 16, 2018 16:00
@ciarmcom
Copy link
Member

@embeddedteam103, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

@kjbracey
Copy link
Contributor

Thanks for looking at this (and noticing it, and caring).

This is a sore point in the design, and if it was as simple as this patch, it would have been done some time ago.

poll is a particular case, and the less common one - we have the same issue in ordinary blocking reads and writes. Writes in particular are the most likely to cause problems, as people do tend to do writes to console from multiple threads. This sort of mechanism does not scale to that case - it can't wake multiple threads blocked on output when TX buffer space becomes available.

Now, I do have my own prototype solution, but it's been parked for a while, due to combination of lack of interest and some contention over design (mainly "does this encourage critical section use too much?", iirc). What I was aiming for with it:

  • fully copes with multiple threads blocked on a device
  • doesn't interfere with user's sigio API
  • can in principle work equally well without the RTOS

First attempt is here: https://github.com/kjbracey-arm/mbed-os/commits/cv_cs
And a follow-up trying to pull out the common logic as a helper is here: https://github.com/kjbracey-arm/mbed-os/commits/cv_cs_fhdwh, but I don't recall the details of how solid that ended up

Care to try that cv_cs branch? I can see about kicking it off as a proper PR for discussion.

@embeddedteam103
Copy link
Contributor Author

Let's move the disscussion to the issue #9112 so it won't be lost.

@embeddedteam103
Copy link
Contributor Author

Will be resolved by inclusion of branch cv_cs. See the disscussion in #9112 for the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants