-
Notifications
You must be signed in to change notification settings - Fork 1.4k
net/local: replace net_lock with local_lock #17733
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
base: master
Are you sure you want to change the base?
Conversation
dup2 and setsockopt can use the lock in conn to protect resources, the lock in accept is originally used to protect the connection status. however, only the send, recv, netpoll, and connect processes will check this flag. only when the interface returns will the corresponding conn structure be exposed to the caller, and then the above operations can be performed. Therefore, this net_lock is not necessary. Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
remove the use of net_lock in the local module and decouple it from other network modules. Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
anchao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not reuse net_lock and extended it to support lock instances? if in this way, there's no need to add unlock/lock operations in each branch, which can avoid lock imbalance issues and eliminate the need to worry about whether to call lock or unlock at the code level.
@anchao If we directly modify the net_lock prototype, too much code needs to be changed, moreover, it's possible that some net_lock in external repository drivers may be affected, leading to compilation errors. Therefore, each relatively independent protocol has added its own global lock. I will submit a patch for code format optimization later, changing all device-related protocol code to a format similar to the previous net_xxx_wait. For local socket, I can also add a unified interface with parameters and breaklock to optimize the code in the unlock/wait/lock format. Do you think this is reasonable? |
net_lock/net_unlock need keep the prototype without change since many net driver call net_lock/net_unlock and we don't have any plan to change them. |
|
@xiaoxiang781216 @zhhyu7 extension
I have another question, if net_lock in the board code is not properly adjusted, wouldn't this lead to deadlocks or race conditions? This is because after the modification, there will be 2 separate locks (one for the protocol stack and one for the board), and I am concerned that some callback strategies may result in ABBA dead lock |
@anchao Yes, If there is a callback strategy that may lead to ABBA deadlock, the corresponding driver will need to make relevant adaptations. Currently, we can only ensure that processes that net_lock is not first attempted to be acquired in the callback is compatible. |
Summary
remove the use of net_lock in the local module and decouple it from other network modules.
Impact
net::local
Testing
sim:matter with test code:
NuttX log: