Hi Maxime and Chenbo, Do you have any suggestions for how to address this?
Looking forward to hearing from you! Thanks, Gongming > On Apr 3, 2024, at 11:52 PM, Gongming Chen <chengongming1...@outlook.com> > wrote: > > Hi Maxime, > Thanks for review. > >> On Apr 3, 2024, at 5:39 PM, Maxime Coquelin <maxime.coque...@redhat.com> >> wrote: >> >> Hi Gongming, >> >> It's the 9th time the patch has been sent. >> I'm not sure whether there are changes between them or these are just >> re-sends, but that's something to avoid. >> > > Sorry, there's something wrong with my mailbox. > I will send a v1 version as the latest patch, but they are actually the same. > >> If there are differences, you should use versionning to highlight it. >> If unsure, please check the contributions guidelines first. >> >> Regarding the patch itself, I don't know if this is avoidable, but I >> would prefer we do not introduce yet another lock in there. >> >> Thanks, >> Maxime >> > > I totally agree with your. > Therefore, initially I hoped to solve this problem without introducing > new lock. However, the result was not expected. > > 1. The vsocket is shared between the event and reconnect threads by > transmitting the vsocket pointer. Therefore, there is no way to protect > vsocket through a simple vsocket lock. > > 2. The event and reconnect threads can transmit vsocket pointers to > each other, so there is no way to ensure that vsocket will not be > accessed by locking the two threads separately. > > 3. Therefore, on the vsocket resource, event and reconnect are in the > same critical section. Only by locking two threads at the same time > can the vsocket be ensured that it will not be accessed and can be > freed safely. > > Currently, app config, event, and reconnect threads respectively have > locks corresponding to their own maintenance resources, > vhost_user.mutex, pfdset->fd_mutex, and reconn_list.mutex. > > I think there is a thread-level lock missing here to protect the > critical section between threads, just like the rcu scene protection. > > After app config acquires the write lock, it ensures that the event and > reconnect threads are outside the critical section. > This is to completely clean up the resources associated with vsocket > and safely free vsocket. > > Therefore, considering future expansion, if there may be more > resources like vsocket, this thread lock can also be used to ensure > that resources are safely released after complete cleanup. > > In this way, the threads will be clearer, and the complicated try lock > method is no longer needed. > > Thanks, > Gongming
