Use Token-based locking on XDE management operations
#761
Conversation
|
Attached is the source of a simple test program which exercises the deadlock here by creating ports, deleting ports, and spawning new processes. On |
| #[cfg(all(not(feature = "std"), not(test)))] | ||
| let curthread = unsafe { | ||
| NonNull::new(threadp()) | ||
| .expect("current thread *must* be a valid pointer") | ||
| }; | ||
|
|
||
| #[cfg(any(feature = "std", test))] | ||
| let curthread = std::thread::current().id(); | ||
|
|
||
| *thread_lock = Some(curthread); |
There was a problem hiding this comment.
This differs slightly from other times this pattern occurs in the kernel (e.g., nvme.c) -- here we drop the lock as soon as possible and stake our claim purely using the thread ID, whereas in the prior art the thread ID is only stored when making an upcall.
If having the same behaviour is crucial, we could shift the thread ID logic into a Token::upcall_context(&mut self, f: impl FnOnce()) and have Token hold a lock guard instead -- this would also be useful in proving that none of the inner data/locks are held in a given closure.
Still thinking on the token lock_sig...
This PR introduces a
TokenLocktype, which allows for a single thread to be in a critical section without actively holding aKMutexor a write on aKRwLock. This is used to ensure that we have at most one active ioctl handler performing management operations. Previously, we relied upon write locks to theunderlayandxde_devsto enforce this constraint.Underlay installation, port creation, and port destruction have been restructured to rely on the
TokenLockas the main means of mutual exclusion, and then to access other locked resources (which may be shared with the datapath) more granularly. This allows us to ensure that any calls to DLS (link name resolution, device creation) are made without inappropriately holding any lock, but are still appropriately atomic with respect to one another.Fixes #758.