zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.71k stars 6.54k forks source link

Runtime locking correctness validator #51073

Open desowin opened 2 years ago

desowin commented 2 years ago

Is your enhancement proposal related to a problem? Please describe. Sometimes a function expects that the caller holds specific lock (e.g. mutex). Zephyr currently doesn't have any runtime method of checking locking violations (e.g. caller does not hold mutex).

Describe the solution you'd like Something similar to Linux Runtime locking correctness validator, especially the lockdep_assert_held().

Describe alternatives you've considered __ASSERT_NO_MSG(mutex->owner == z_current()); but accessing the owner field directly seems wrong. Also, the locking correctness validation is something that should specifically have to be enabled.

Additional context Linux kernel lockdep can do much more than just asserting that a lock is held, e.g. it can detect deadlock possibility before actually hitting the deadlock. The full lockdep functionality is probably not possible in Zephyr due to target device constraints, but just having equivalent to lockdep_assert_held() would already be helpful.

henrikbrixandersen commented 2 years ago

CC: @andyross

cfriedt commented 2 years ago

Just a couple of thoughts:

andyross commented 2 years ago

There's something like this in spinlocks already: CONFIG_SPIN_VALIDATE. And in the almost-ready-I-swear Zync PR[1], there's a broadly equivalent CONFIG_ZYNC_VALIDATE. It doesn't provide the ownership as an API though, because that requires storing the owner, which is a comparatively heavy operation for an RTOS.

[1] Check the andyross/zephyr/zynctmp branch if you want to see the most recent work. It has dozens of WIP patches that need to be squashed still, but is much closer to releasable state.

andyross commented 2 years ago

Sorry, for clarity: it stores the owner when it needs to store the owner (for recursive k_mutex behavior, or when VALIDATE is enabled). But we absolutely want to preserve the optimization opportunity when it's not needed.

luisacicolini commented 1 year ago

@andyross thank you very much for your comments. I took some time to dig deeper into the theory of runtime validators and locking classes and to look at your repository. As far as I've seen, Zync contains some relevant modifications with respect to the original Zephir kernel, would it be more helpful if I worked on the validator directly on Zync? Thanks :)

zephyrbot commented 8 months ago

Hi @peter-mitsis, @andyross,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@desowin you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!