On 25/11/2016 06:22, "Ilya Maximets" <i.maxim...@samsung.com> wrote:
>On 16.11.2016 03:45, Daniele Di Proietto wrote:
>> We keep all the per-port classifiers around, since they can be reused,
>> but when a pmd thread is destroyed we should free them.
>>
>> Found using valgrind.
>>
>> Fixes: 3453b4d62a98("dpif-netdev: dpcls per in_port with sorted
>> subtables")
>>
>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>> ---
>> lib/dpif-netdev.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index c477248..c19d3e8 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -3330,6 +3330,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd)
>> /* All flows (including their dpcls_rules) have been deleted already */
>> CMAP_FOR_EACH (cls, node, &pmd->classifiers) {
>> dpcls_destroy(cls);
>> + free(cls);
>
>free should be postponed using rcu.
>This change exists in my incremental patch:
>https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325404.html
I think that it should be safe to call free() immediately here, because nobody
else
is accessing the cmap:
* CMAP_FOR_EACH is "safe" in the sense of HMAP_FOR_EACH_SAFE. That is, it is
* safe to free the current node before going on to the next iteration. Most
* of the time, though, this doesn't matter for a cmap because node
* deallocation has to be postponed until the next grace period. This means
* that this guarantee is useful only in deallocation code already executing at
* postponed time, when it is known that the RCU grace period has already
* expired.
That said, it's probably better to postpone the free() than to remember this
special case, especially if we end up moving that code somewhere else, so I
took your advice.
Thanks,
Daniele
>
>> }
>> cmap_destroy(&pmd->classifiers);
>> cmap_destroy(&pmd->flow_table);
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
