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

RFC: standardize on a synchronization mechanism for device access #26073

Open
pabigot opened this issue Jun 9, 2020 · 4 comments
Open

RFC: standardize on a synchronization mechanism for device access #26073

pabigot opened this issue Jun 9, 2020 · 4 comments
Labels
Needs review This PR needs attention from Zephyr's maintainers RFC Request For Comments: want input from the community

Comments

@pabigot
Copy link
Collaborator

pabigot commented Jun 9, 2020

There are three types of synchronization necessary for devices, several of which are considered in #22941 and its draft PRs particularly #24511:

  • Access control, to ensure multiple threads do not attempt to use a device instance concurrently;
  • Internal synchronization, to allow the driver to suspend while an operation it depends on is in progress;
  • External synchronization, to return control to the caller with notification performed by the driver when the requested operation completes.

The topic of this issue is access control, which historically has been implemented using k_sem. As originally raised in this comment I'd like to propose that was use k_mutex instead, as this offers:

  • priority inheritance, which would allow a block high-priority thread to get access to the resources it needs faster by allowing the current owner to temporarily run with the blocked thread's priority;
  • recursive locking, which may simplify some use cases particularly in drivers with multiple user-facing API calls.

The first question is whether this the right move. A follow-up question is whether the generic device API should provide support for any or all of the above synchronizations.

If we use k_mutex for access control we would need #25678 to be worked so its operations can be initiated from interrupt context, as is already allowed for k_sem.

@pabigot pabigot added the RFC Request For Comments: want input from the community label Jun 9, 2020
@pabigot pabigot added this to the v2.4.0 milestone Jun 9, 2020
@andrewboie
Copy link
Contributor

While we're thinking about this topic, it looks the the PREEMPT_RT patch to Linux goes even further and replaces some usages of spinlocks with RT-mutexes. There are caveats and I'm not sure how applicable this is to Zephyr, but interesting nonetheless https://lwn.net/Articles/146861/

@pabigot pabigot removed this from the v2.4.0 milestone Aug 22, 2020
@pabigot pabigot changed the title RFC: standardize on k_mutex as synchronization mechanism for device access RFC: standardize on a synchronization mechanism for device access Nov 14, 2020
@carlescufi
Copy link
Member

API meeting

  • Device APIs' thread-safety shall be enforced and documented per device class
  • Thread safety guarantees shall be the same for all device classes that offer them (priority inheritance for example)
  • Synchronization shall be disabled when CONFIG_MULTITHREADING is n

@jfischer-no
Copy link
Collaborator

  • Synchronization shall be disabled when CONFIG_MULTITHREADING is n

I am against forcing it for all drivers APIs.

@gregshue
Copy link

When we need exclusive access to resources shared with interrupt context we need to lock interrupts to the level of the ISR(s) accessing it (or we need to disable the ISRs that access it - causing priority inversion). This cannot be done by a k_mutex, so we will need at least two mechanisms. Which one depends on whether the project configures the interrupt service routine to run in interrupt context or meta-irq context.

Also, don't forget about access in an SMP environment and any issues with access originating from User threads.

@zephyrbot zephyrbot added the Needs review This PR needs attention from Zephyr's maintainers label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs review This PR needs attention from Zephyr's maintainers RFC Request For Comments: want input from the community
Projects
Status: No status
Development

No branches or pull requests

6 participants