Re: fix for hidden corei7 requirement in binary packages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+dev@ceph

On Sun, Aug 18, 2019 at 9:50 AM Alexandre Oliva <oliva@xxxxxxx> wrote:
>
> Hi,
>
> After upgrading some old Phenom servers from Fedora/Freed-ora 29 to
> 30's, to ceph-14.2.2, I was surprised by very early crashes of both
> ceph-mon and ceph-osd.  After ruling out disk and memory corruption, I
> investigated a bit noticed all of them crashed during pre-main() init
> section processing, at an instruction not available on the Phenom X6
> processors that support sse4a, but not e.g. sse4.1.
>
> It turns out that much of librte is built with -march=corei7.  That's a
> little excessive, considering that x86/rte_memcpy.h would be happy
> enough with -msse4.1, but not with earlier sse versions that Fedora is
> supposed to target.
>
> I understand rte_memcpy.h is meant for better performance, inlining
> fixed-size and known-alignment implementations of memcpy into users.
> Alas, that requires setting a baseline target processor, and you'll only
> get as efficient an implementation as what's built in.
>
> I noticed an attempt for dynamic selection, but GCC rightfully won't
> inline across different target flags, so we'd have to give up inlining
> to get better dynamic behavior.  The good news is that glibc already
> offers dynamic selection of memcpy implementations, so hopefully the
> impact of this change won't be much worse than that of enabling dynamic
> selection, without the complications.
>
> If that's not good enough, compiling ceph with flags that enable SSE4.1,
> AVX2 or AVX512, or with a -march flag that implicitly enables them,
> would restore current performance, but without that, you will (with the
> patch below) get a package that runs on a broader range of processors,
> that the base distro (through the compiler's baseline flags) chooses to
> support.  It's not nice when you install a package on a processor that's
> supposed to be supported and suddenly you're no longer sure it is ;-)
>
> Perhaps building a shared librte, so that one could build and install
> builds targeting different ISA versions, without having to rebuild all
> of ceph, would be a reasonable way to address the better tuning of these
> performance-critical bits.
>
>
>
> src/spdk/dpdk:
>
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> index 7b758094d..ce714bf02 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -40,6 +40,16 @@ extern "C" {
>  static __rte_always_inline void *
>  rte_memcpy(void *dst, const void *src, size_t n);
>
> +#ifndef RTE_MACHINE_CPUFLAG_SSE4_1
> +
> +static __rte_always_inline void *
> +rte_memcpy(void *dst, const void *src, size_t n)
> +{
> +  return memcpy(dst, src, n);
> +}
> +
> +#else /* RTE_MACHINE_CPUFLAG_SSE4_1 */
> +
>  #ifdef RTE_MACHINE_CPUFLAG_AVX512F
>
>  #define ALIGNMENT_MASK 0x3F
> @@ -869,6 +879,8 @@ rte_memcpy(void *dst, const void *src, size_t n)
>                 return rte_memcpy_generic(dst, src, n);
>  }
>
> +#endif /* RTE_MACHINE_CPUFLAG_SSE4_1 */
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/mk/machine/default/rte.vars.mk b/mk/machine/default/rte.vars.mk
> index df08d3b03..6bf695849 100644
> --- a/mk/machine/default/rte.vars.mk
> +++ b/mk/machine/default/rte.vars.mk
> @@ -27,4 +27,4 @@
>  # CPU_LDFLAGS =
>  # CPU_ASFLAGS =
>
> -MACHINE_CFLAGS += -march=corei7
> +# MACHINE_CFLAGS += -march=corei7
>
>
> --
> Alexandre Oliva, freedom fighter  he/him   https://FSFLA.org/blogs/lxo
> Be the change, be Free!                 FSF Latin America board member
> GNU Toolchain Engineer                        Free Software Evangelist
> Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara



-- 
Cheers,
Brad



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux