[ovs-dev] [PATCH v2 2/2] conntrack: replace conntrack lock form?mutex to spinlock

[Ben Pfaff]

On Fri, Jul 09, 2021 at 03:32:25PM +0200, Gaëtan Rivet wrote:
> On Fri, Jul 9, 2021, at 10:38, wenxu at ucloud.cn wrote:
> > From: wenxu <wenxu at ucloud.cn>
> > 
> > The ct_lock of conntrack is a global lock to protect the
> > conntrack table insert. Mutex lock will add more latency
> > for lock conflict and the poor performance for CPS of new
> > connections creating.
> > 
> > With our benchmark four pmd thread with simple conntrack
> > actions, The CPS is 300k. With this patch replace the
> > ct_lock to spinlock, the CPS improves to 500K.
> > 
> 
> Hello,
> 
> Using spinlocks in userland is a difficult subject.
> The common wisdom is to avoid them. Some exceptions exists, when every
> thread sharing the lock would be affined and isolated, thus with no chance
> to be preempted.
> 
> This is almost the case here, PMDs are affined and should be isolated.
> However ct_clean is not. It should not be possible to fall into the basic
> worst case scenario for spinlock: holding thread preempted and spinning
> threads consuming all CPU resources, preventing the holding thread from being
> scheduled again. I cannot say however that there won't be worse edge-cases due
> to the unfair lock.
> 
> Adaptive mutexes should reduce the locking cost in low-contention,
> without the downsides of spinlocks.
> 
> The core issue however is that the userland conntrack should not lead to
> so much contention. I have reworked its locking in a series already:
> https://patchwork.ozlabs.org/project/openvswitch/list/?series=249027
> 
> I think it should be explored before considering spinlocks.

+1 on all this.