> On Jul 19, 2014, at 8:27 PM, Ben Pfaff <b...@nicira.com> wrote: > > We could make some operations lockless, like read and store,
These would be enough for RCU cmap and classifier, right? Refcount needs the read-modify-write operations, though. > but I don't > know how to make read-modify-write operations lockless generically. > What do you have in mind? > >> On Sat, Jul 19, 2014 at 11:37:42AM +0300, Jarno Rajahalme wrote: >> Would it be a bad idea to make the trivial types with sizes less than >> void pointer lockless in ovs-atomic-pthread just like in gcc4+, at >> least for x86/64? >> >> Jarno >> >>> On Jul 19, 2014, at 10:05 AM, Ben Pfaff <b...@nicira.com> wrote: >>> >>> I guess not, I see that xenddk-6.0.0-50762, at least, doesn't have a >>> libatomic. >>> >>>> On Sat, Jul 19, 2014 at 12:04:25AM -0700, Ben Pfaff wrote: >>>> Does it make a difference if you add -latomic to the linker flags? >>>> >>>>> On Fri, Jul 18, 2014 at 05:32:39PM -0700, Jarno Rajahalme wrote: >>>>> From config.log: >>>>> >>>>> configure:13921: checking whether gcc -std=gnu99 supports GCC 4.0+ atomic >>>>> built-ins >>>>> configure:13988: gcc -std=gnu99 -o conftest -g -O2 conftest.c -lrt -lm >>>>> >&5 >>>>> /tmp/ccZ2M4lr.o: In function `main': >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:57: undefined >>>>> reference to `__sync_fetch_and_add_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:57: undefined >>>>> reference to `__sync_fetch_and_sub_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:57: undefined >>>>> reference to `__sync_fetch_and_or_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:57: undefined >>>>> reference to `__sync_fetch_and_and_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:57: undefined >>>>> reference to `__sync_fetch_and_xor_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:58: undefined >>>>> reference to `__sync_fetch_and_add_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:58: undefined >>>>> reference to `__sync_fetch_and_sub_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:58: undefined >>>>> reference to `__sync_fetch_and_or_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:58: undefined >>>>> reference to `__sync_fetch_and_and_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:58: undefined >>>>> reference to `__sync_fetch_and_xor_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:59: undefined >>>>> reference to `__sync_fetch_and_add_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:59: undefined >>>>> reference to `__sync_fetch_and_sub_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:59: undefined >>>>> reference to `__sync_fetch_and_or_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:59: undefined >>>>> reference to `__sync_fetch_and_and_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:59: undefined >>>>> reference to `__sync_fetch_and_xor_1' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:60: undefined >>>>> reference to `__sync_fetch_and_add_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:60: undefined >>>>> reference to `__sync_fetch_and_sub_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:60: undefined >>>>> reference to `__sync_fetch_and_or_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:60: undefined >>>>> reference to `__sync_fetch_and_and_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:60: undefined >>>>> reference to `__sync_fetch_and_xor_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:61: undefined >>>>> reference to `__sync_fetch_and_add_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:61: undefined >>>>> reference to `__sync_fetch_and_sub_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:61: undefined >>>>> reference to `__sync_fetch_and_or_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:61: undefined >>>>> reference to `__sync_fetch_and_and_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:61: undefined >>>>> reference to `__sync_fetch_and_xor_2' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:62: undefined >>>>> reference to `__sync_fetch_and_add_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:62: undefined >>>>> reference to `__sync_fetch_and_sub_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:62: undefined >>>>> reference to `__sync_fetch_and_or_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:62: undefined >>>>> reference to `__sync_fetch_and_and_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:62: undefined >>>>> reference to `__sync_fetch_and_xor_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:63: undefined >>>>> reference to `__sync_fetch_and_add_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:63: undefined >>>>> reference to `__sync_fetch_and_sub_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:63: undefined >>>>> reference to `__sync_fetch_and_or_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:63: undefined >>>>> reference to `__sync_fetch_and_and_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:63: undefined >>>>> reference to `__sync_fetch_and_xor_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:64: undefined >>>>> reference to `__sync_fetch_and_add_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:64: undefined >>>>> reference to `__sync_fetch_and_sub_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:64: undefined >>>>> reference to `__sync_fetch_and_or_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:64: undefined >>>>> reference to `__sync_fetch_and_and_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:64: undefined >>>>> reference to `__sync_fetch_and_xor_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:65: undefined >>>>> reference to `__sync_fetch_and_add_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:65: undefined >>>>> reference to `__sync_fetch_and_sub_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:65: undefined >>>>> reference to `__sync_fetch_and_or_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:65: undefined >>>>> reference to `__sync_fetch_and_and_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:65: undefined >>>>> reference to `__sync_fetch_and_xor_4' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:66: undefined >>>>> reference to `__sync_fetch_and_add_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:66: undefined >>>>> reference to `__sync_fetch_and_sub_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:66: undefined >>>>> reference to `__sync_fetch_and_or_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:66: undefined >>>>> reference to `__sync_fetch_and_and_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:66: undefined >>>>> reference to `__sync_fetch_and_xor_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:67: undefined >>>>> reference to `__sync_fetch_and_add_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:67: undefined >>>>> reference to `__sync_fetch_and_sub_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:67: undefined >>>>> reference to `__sync_fetch_and_or_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:67: undefined >>>>> reference to `__sync_fetch_and_and_8' >>>>> /usr/src/redhat/BUILD/openvswitch-2.3.90.37691/conftest.c:67: undefined >>>>> reference to `__sync_fetch_and_xor_8' >>>>> collect2: ld returned 1 exit status >>>>> >>>>>> On Jul 18, 2014, at 5:00 PM, Jarno Rajahalme <jrajaha...@nicira.com> >>>>>> wrote: >>>>>> >>>>>> I think I found the reason for the slowness of cmap on XenServer. >>>>>> >>>>>> I was debugging on a XenServer tonight and found out that the configure >>>>>> test for GCC atomics had apparently failed, as I noticed actual lock and >>>>>> unlock operations on gdb disassembly, and then by the fact the changes I >>>>>> made to the ovs-atomic-gcc4+.h did not trigger any recompilations with >>>>>> ?make? (so there was no dependency on it). >>>>>> >>>>>> If the configure check is correct, then there must be something wrong >>>>>> with the XenServer setup? >>>>>> >>>>>> Jarno >>>>>> >>>>>>> On Jun 25, 2014, at 11:09 AM, Ben Pfaff <b...@nicira.com> wrote: >>>>>>> >>>>>>> I am really surprised to hear that it takes over a minute. On my >>>>>>> workstation (which is a few years old), "ovstest test-cmap check 1" >>>>>>> only takes 6.5 seconds. Do you have any idea what's different on >>>>>>> XenServer? It might be the GCC version, which version is it using? >>>>>>> >>>>>>>> On Wed, Jun 25, 2014 at 09:40:12AM -0700, Gurucharan Shetty wrote: >>>>>>>> I had a quick comment. I noticed while running unit tests on Xenserver >>>>>>>> that the cuckoo hash unit test was very slow. It takes a little more >>>>>>>> than one minute to complete. I wonder whether using more cmap would >>>>>>>> hurt Xenserver's performance? >>>>>>>> >>>>>>>>> On Wed, Jun 25, 2014 at 4:19 AM, Jarno Rajahalme >>>>>>>>> <jrajaha...@nicira.com> wrote: >>>>>>>>> Ben, >>>>>>>>> >>>>>>>>> Thanks for the reviews! >>>>>>>>> >>>>>>>>> I have addressed the (minor) comments on the series, will push once >>>>>>>>> the vector v2 is ack???ed. I???ll keep this last patch as the first >>>>>>>>> in the classifier RCU series. >>>>>>>>> >>>>>>>>> Jarno >>>>>>>>> >>>>>>>>>>> On Jun 13, 2014, at 10:37 AM, Ben Pfaff <b...@nicira.com> wrote: >>>>>>>>>>> >>>>>>>>>>> On Mon, Jun 09, 2014 at 11:53:55AM -0700, Jarno Rajahalme wrote: >>>>>>>>>>> Use cmap instead of hmap & hindex in classifier. Performance impact >>>>>>>>>>> with current locking strategy is not yet tested. Later patches will >>>>>>>>>>> introduce RCU into the classifer. >>>>>>>>>> >>>>>>>>>> into the "classifier". >>>>>>>>>> >>>>>>>>>>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> >>>>>>>>>> >>>>>>>>>> As long as this passes the tests (I assume you ran them) it seems OK >>>>>>>>>> to me. The tricky review will be when we start actually making the >>>>>>>>>> classifier RCU. (I don't know if it makes sense to push this patch >>>>>>>>>> before then. It might be better as the first patch of a series that >>>>>>>>>> makes the classifier RCU. But I'll leave that decision to you.) >>>>>>>>>> >>>>>>>>>> Acked-by: Ben Pfaff <b...@nicira.com> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> dev mailing list >>>>>>>>> dev@openvswitch.org >>>>>>>>> http://openvswitch.org/mailman/listinfo/dev >>>>> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
