mbox series

[ovs-dev,0/2] northd: Hash locks for dp groups + thread safety.

Message ID 20230202123530.945140-1-i.maximets@ovn.org
Headers show
Series northd: Hash locks for dp groups + thread safety. | expand

Message

Ilya Maximets Feb. 2, 2023, 12:35 p.m. UTC 
While running tests with ovn-heater it was observed that locking and
unlocking dpg_lock can take more than 20% of CPU cycles in northd even
without any contention.

This series is trying to address that issue and add thread safety
static analysis.

Ilya Maximets (2):
  northd: Use larger hash bucket locks instead of dp group ones.
  northd: Add thread safety analysis to lflow hash locks.

 northd/northd.c | 198 ++++++++++++++++++++++++++++++------------------
 1 file changed, 125 insertions(+), 73 deletions(-)

Comments

Dumitru Ceara Feb. 6, 2023, 11:49 a.m. UTC | #1 
On 2/2/23 13:35, Ilya Maximets wrote:
> While running tests with ovn-heater it was observed that locking and
> unlocking dpg_lock can take more than 20% of CPU cycles in northd even
> without any contention.
> 
> This series is trying to address that issue and add thread safety
> static analysis.
> 

Hi Ilya,

Thanks for these patches, this looks like a very nice performance boost
indeed!

Before doing a proper review, would it be possible to rebase them on top
of the current main branch?  I just pushed Ales' '7b94d212f694 ("northd:
Refactor build_lrouter_nat_flows_for_lb function")' v6 and that created
some conflicts in northd.c

Thanks,
Dumitru
Ilya Maximets Feb. 7, 2023, 11:45 a.m. UTC | #2 
On 2/6/23 12:49, Dumitru Ceara wrote:
> On 2/2/23 13:35, Ilya Maximets wrote:
>> While running tests with ovn-heater it was observed that locking and
>> unlocking dpg_lock can take more than 20% of CPU cycles in northd even
>> without any contention.
>>
>> This series is trying to address that issue and add thread safety
>> static analysis.
>>
> 
> Hi Ilya,
> 
> Thanks for these patches, this looks like a very nice performance boost
> indeed!
> 
> Before doing a proper review, would it be possible to rebase them on top
> of the current main branch?  I just pushed Ales' '7b94d212f694 ("northd:
> Refactor build_lrouter_nat_flows_for_lb function")' v6 and that created
> some conflicts in northd.c

Done.  Though I've gone a bit wild with the set. :)  The conflict wasn't
an easy one to resolve.

Best regards, Ilya Maximets.
Dumitru Ceara Feb. 7, 2023, 12:37 p.m. UTC | #3 
On 2/7/23 12:45, Ilya Maximets wrote:
> On 2/6/23 12:49, Dumitru Ceara wrote:
>> On 2/2/23 13:35, Ilya Maximets wrote:
>>> While running tests with ovn-heater it was observed that locking and
>>> unlocking dpg_lock can take more than 20% of CPU cycles in northd even
>>> without any contention.
>>>
>>> This series is trying to address that issue and add thread safety
>>> static analysis.
>>>
>>
>> Hi Ilya,
>>
>> Thanks for these patches, this looks like a very nice performance boost
>> indeed!
>>
>> Before doing a proper review, would it be possible to rebase them on top
>> of the current main branch?  I just pushed Ales' '7b94d212f694 ("northd:
>> Refactor build_lrouter_nat_flows_for_lb function")' v6 and that created
>> some conflicts in northd.c
> 
> Done.  Though I've gone a bit wild with the set. :)  The conflict wasn't
> an easy one to resolve.

Sorry about that and thanks for taking care of it!  I'll try to review
v2 as soon as possible.

Regards,
Dumitru