[ovs-dev] [PATCH] dpif-netdev: Allocate dp_netdev_pmd_thread struct by xzalloc_cacheline.

Ilya Maximets i.maximets at samsung.com
Fri Dec 8 13:53:47 UTC 2017


On 08.12.2017 16:45, Stokes, Ian wrote:
>> All instances of struct dp_netdev_pmd_thread are allocated by xzalloc and
>> therefore doesn't guarantee memory allocation aligned on CACHE_LINE_SIZE
>> boundary. Due to this any padding done inside the structure with this
>> assumption might create holes.
>>
>> This commit replaces xzalloc, free with xzalloc_cacheline and
>> free_cacheline. With the changes the memory is 64 byte aligned.
> 
> Thanks for this Bhanu,
> 
> I think this looks OK and I'm considering pushing to the DPDK_Merge branch but as there has been a fair bit of debate lately regarding memory and cache alignment I want to flag to others who have engaged to date to have their say before I apply it as there has been no input yet for the patch.
> 
> @Jan/Ilya, are you ok with this change?

OVS will likely crash on destroying non_pmd thread because it still
allocated by usual xzalloc, but freed with others by free_cacheline().

> 
> Thanks
> Ian
> 
>>
>> Before:
>>     With xzalloc, all the memory is 16 byte aligned.
>>
>>     (gdb) p pmd
>>     $1 = (struct dp_netdev_pmd_thread *) 0x7eff8a813010
>>     (gdb) p &pmd->cacheline0
>>     $2 = (OVS_CACHE_LINE_MARKER *) 0x7eff8a813010
>>     (gdb) p &pmd->cacheline1
>>     $3 = (OVS_CACHE_LINE_MARKER *) 0x7eff8a813050
>>     (gdb) p &pmd->flow_cache
>>     $4 = (struct emc_cache *) 0x7eff8a813090
>>     (gdb) p &pmd->flow_table
>>     $5 = (struct cmap *) 0x7eff8acb30d0
>>     (gdb)  p &pmd->stats
>>     $6 = (struct dp_netdev_pmd_stats *) 0x7eff8acb3110
>>     (gdb) p &pmd->port_mutex
>>     $7 = (struct ovs_mutex *) 0x7eff8acb3150
>>     (gdb) p &pmd->poll_list
>>     $8 = (struct hmap *) 0x7eff8acb3190
>>     (gdb) p &pmd->tnl_port_cache
>>     $9 = (struct hmap *) 0x7eff8acb31d0
>>     (gdb) p &pmd->stats_zero
>>     $10 = (unsigned long long (*)[5]) 0x7eff8acb3210
>>
>> After:
>>     With xzalloc_cacheline, all the memory is 64 byte aligned.
>>
>>     (gdb) p pmd
>>     $1 = (struct dp_netdev_pmd_thread *) 0x7f39e2365040
>>     (gdb) p &pmd->cacheline0
>>     $2 = (OVS_CACHE_LINE_MARKER *) 0x7f39e2365040
>>     (gdb) p &pmd->cacheline1
>>     $3 = (OVS_CACHE_LINE_MARKER *) 0x7f39e2365080
>>     (gdb) p &pmd->flow_cache
>>     $4 = (struct emc_cache *) 0x7f39e23650c0
>>     (gdb) p &pmd->flow_table
>>     $5 = (struct cmap *) 0x7f39e2805100
>>     (gdb) p &pmd->stats
>>     $6 = (struct dp_netdev_pmd_stats *) 0x7f39e2805140
>>     (gdb) p &pmd->port_mutex
>>     $7 = (struct ovs_mutex *) 0x7f39e2805180
>>     (gdb) p &pmd->poll_list
>>     $8 = (struct hmap *) 0x7f39e28051c0
>>     (gdb) p &pmd->tnl_port_cache
>>     $9 = (struct hmap *) 0x7f39e2805200
>>     (gdb) p &pmd->stats_zero
>>     $10 = (unsigned long long (*)[5]) 0x7f39e2805240
>>
>> Reported-by: Ilya Maximets <i.maximets at samsung.com>
>> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
>> ---
>>  lib/dpif-netdev.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index db78318..3e281ae
>> 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -3646,7 +3646,7 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>      FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>>          pmd = dp_netdev_get_pmd(dp, core->core_id);
>>          if (!pmd) {
>> -            pmd = xzalloc(sizeof *pmd);
>> +            pmd = xzalloc_cacheline(sizeof *pmd);
>>              dp_netdev_configure_pmd(pmd, dp, core->core_id, core-
>>> numa_id);
>>              pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
>>              VLOG_INFO("PMD thread on numa_id: %d, core id: %2d created.",
>> @@ -4574,7 +4574,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
>> *pmd)
>>      xpthread_cond_destroy(&pmd->cond);
>>      ovs_mutex_destroy(&pmd->cond_mutex);
>>      ovs_mutex_destroy(&pmd->port_mutex);
>> -    free(pmd);
>> +    free_cacheline(pmd);
>>  }
>>
>>  /* Stops the pmd thread, removes it from the 'dp->poll_threads',
>> --
>> 2.4.11
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> 
> 


More information about the dev mailing list