On 12/04/17 13:22, Dmitry Eremin-Solenikov wrote: > On 12.04.2017 14:05, Joe Savage wrote: >> On 12/04/17 11:32, Maxim Uvarov wrote: >>> On 12.04.2017 13:15, Joe Savage wrote: >>>>>>> The problem is that when we discussed this patch on ODP call people very >>>>>>> worry about having 128bit instructions in ODP examples. At least Petri >>>>>>> and Barry asked if it would be possible to rewrite that with 64 bit >>>>>>> instructions? Some compilers might not support 128 bits and we need to >>>>>>> test it more. >>>>>> >>>>>> On 32-bit platforms, it already does use 64-bit atomics. In general, >>>>>> though, >>>>>> the example hinges around having atomics that are twice the pointer size. >>>>>> We've actually discussed this on the list already in the thread "32-bit >>>>>> support in examples". Even if lock-free implementations can't be used, >>>>>> compilers can (and frequently do?) provide a lock-based compare exchange >>>>>> operation. >>>>> >>>>> Any progress on this? >>>> >>>> This is getting mildly ridiculous now — it's nearing three months since I >>>> initially submitted this simple example patch, and there's still no end in >>>> sight! Maxim: any status updates? >>>> >>> >>> Dmitry wanted to write some big review for that patch. But I do not see >>> anything here. People commented on 128 bit instructions in examples and >>> nobody set their review-by. I will rise question about your patch one >>> more time on arch call. I can not include things where we did not get >>> common agreement. I do not see anything bad with this patch but we need >>> account all existence odp platforms. > > I'm sorry for the delay. Was quite under a pile of IPsec stuff...
No worries. > I'm quite not happy about intermixture of arch-dependent and > arch-specific stuff in headers. Would it be possible to split > arch-specific implementation details to per-arch headers (one for > AArch64, one for older ARM and one generic) and include proper header > based on the $(ARCH_DIR)? It would be especially nice to have such > header provide ATOMIC_STRONG_CAS_DBLPTR (or maybe just > atomic_strong_cas_dblptr() ). Sure, I can split apart the architecture dependent stuff if you'd prefer it that way. If I'm understanding you correctly, the suggestion here is to move the architecture-specific CAS implementations into separate files — each defining its own version of atomic_strong_cas_dblptr() — and for these files to then be conditionally included in odp_ipfragreass_helpers.h. Is that right? I'm not sure I quite follow your comments regarding $(ARCH_DIR) though. Are there some ODP-defined macros that I can use instead of "__ARM_ARCH" and friends or something? > Forgot to mention. Your implementation seems build fine for MIPS64, so > there should be no more issues from my side. Cool, good stuff.
