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 <[email protected]> >> Signed-off-by: Bhanuprakash Bodireddy <[email protected]> >> --- >> 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 >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
