On Tue, Nov 28, 2017 at 04:22:46PM +0300, Ilya Maximets wrote: > On 28.11.2017 01:54, Ben Pfaff wrote: > > On Mon, Nov 27, 2017 at 02:59:29PM +0300, Ilya Maximets wrote: > >>> I also verified the other case when posix_memalign isn't available and > >>> even in that case > >>> it returns the address aligned on CACHE_LINE_SIZE boundary. I will send > >>> out a patch to use > >>> xzalloc_cacheline for allocating the memory. > >> > >> I don't know how you tested this, because it is impossible: > >> > >> 1. OVS allocates some memory: > >> base = xmalloc(...); > >> > >> 2. Rounds it up to the cache line start: > >> payload = (void **) ROUND_UP((uintptr_t) base, CACHE_LINE_SIZE); > >> > >> 3. Returns the pointer increased by 8 bytes: > >> return (char *) payload + MEM_ALIGN; > >> > >> So, unless you redefined MEM_ALIGN to zero, you will never get aligned > >> memory > >> address while allocating by xmalloc_cacheline() on system without > >> posix_memalign(). > > > > Maybe we should make the non-HAVE_POSIX_MEMALIGN case better. Something > > like this (compile tested only); > > That's interesting, but the question here is a bit different. > We definitely can apply 5-10 more patches to fix this unaligned memory > allocation, > paddings, holes in case of different cache line sizes / mutex sizes, whatever > else > we forget to mention, but *do we need this*? Why we're making big efforts to > make > this work as it should?
For what it's worth, I was addressing the xmalloc_cacheline() code independent of your comments on the PMD code. I don't know the PMD code well enough to take a position. I'll let others comment on that. > Beside that, looking at the code below: > Do we care about false sharing on alloc/free operations? That is the > difference between > old and new implementations. The new one will write and read bytes possibly > placed > on a shared cacheline on alloc/free. I'd say that the value of the change that I'm proposing is that it allows programmers to design their structures knowing that the first cacheline ends after 64 bytes, not after 64-8 bytes in some cases. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
