On 28 Apr 2023, at 9:01, Simon Horman wrote:
> On Mon, Apr 24, 2023 at 12:37:04PM +0200, Simon Horman wrote:
>> On Thu, Apr 20, 2023 at 09:39:28AM +0200, Eelco Chaudron wrote:
>>> When doing performance testing with OVS v3.1 we ran into a deadlock
>>> situation with the netdev_hmap_rwlock read/write lock. After some
>>> debugging, it was discovered that the netdev_hmap_rwlock read lock
>>> was taken recursively. And well in the folowing sequence of events:
>>>
>>> netdev_ports_flow_get()
>>> It takes the read lock, while it walks all the ports
>>> in the port_to_netdev hmap and calls:
>>> - netdev_flow_get() which will call:
>>> - netdev_tc_flow_get() which will call:
>>> - netdev_ifindex_to_odp_port()
>>> This function also takes the same read lock to
>>> walk the ifindex_to_port hmap.
>>>
>>> In OVS a read/write lock does not support recursive readers. For details
>>> see the comments in ovs-thread.h. If you do this, it will lock up,
>>> mainly due to OVS setting the PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
>>> attribute to the lock.
>>>
>>> The solution with this patch is to use two separate read/write
>>> locks, with an order guarantee to avoid another potential deadlock.
>>>
>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2182541
>>> Fixes: 9fe21a4fc12a ("netdev-offload: replace netdev_hmap_mutex to
>>> netdev_hmap_rwlock")
>>> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>>
>> Reviewed-by: Simon Horman <simon.hor...@corigine.com>
>
> As a follow-up:
>
> I have a question about lock annotations.
>
> 1. I'm unsure how to exercise them. Some guidance would be appreciated.
> 2. Should we consider using them more/less?
Thanks for the review! If you build with CLANG they are executed, however,
CLANG does not find them in all cases. Especially the cases where functions get
called by pointers.
//Eelco
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
