| Message ID | ord0h3gy6w.fsf@lxoliva.fsfla.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix for hidden corei7 requirement in binary packages | expand |
+dev@ceph On Sun, Aug 18, 2019 at 9:50 AM Alexandre Oliva <oliva@gnu.org> 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
On Mon, Aug 19, 2019 at 9:24 AM Brad Hubbard <bhubbard@redhat.com> wrote: > > +dev@ceph > > On Sun, Aug 18, 2019 at 9:50 AM Alexandre Oliva <oliva@gnu.org> 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 Hi Alexandre, thanks for the bug report and patch. the bug is tracked by https://tracker.ceph.com/issues/41330, and a fix is posted at https://github.com/ceph/ceph/pull/29728 > > > > > > -- > > 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 > _______________________________________________ > Dev mailing list -- dev@ceph.io > To unsubscribe send an email to dev-leave@ceph.io
+ Tone Zhang On Mon, Aug 19, 2019 at 1:04 PM kefu chai <tchaikov@gmail.com> wrote: > > On Mon, Aug 19, 2019 at 9:24 AM Brad Hubbard <bhubbard@redhat.com> wrote: > > > > +dev@ceph > > > > On Sun, Aug 18, 2019 at 9:50 AM Alexandre Oliva <oliva@gnu.org> 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 > > > Hi Alexandre, thanks for the bug report and patch. the bug is tracked > by https://tracker.ceph.com/issues/41330, and a fix is posted at > https://github.com/ceph/ceph/pull/29728 after a second thought, i think, probably, instead of patching SPDK to cater the needs of older machines. a better option is to disable SPDK when building packages for testing and for our official releases. for instance, Phenom II X6 belongs to AMD's K10 family which was launched over a decade ago[0]. while Bobcat family is still being produced [1]. because we don't test SPDK backend in our CI/CD process. and SPDK backend is not being actively maintained. we could just build it in our "make check" run though. what do you think? -- [0] https://en.wikipedia.org/wiki/Phenom_II [1] https://en.wikipedia.org/wiki/Bobcat_(microarchitecture) > > > > > > > > > > -- > > > 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 > > _______________________________________________ > > Dev mailing list -- dev@ceph.io > > To unsubscribe send an email to dev-leave@ceph.io > > > > -- > Regards > Kefu Chai
On Aug 19, 2019, kefu chai <tchaikov@gmail.com> wrote: > after a second thought, i think, probably, instead of patching SPDK to > cater the needs of older machines. a better option is to disable SPDK > when building packages for testing and for our official releases. I have no real clue on what role or impact disabling SPDK plays in the grand scheme of things, so I'm happy to defer to whoever does. I just wish to keep my home ceph cluster running on those machines until I have good reason to replace them. Unfortunately that's the last generation of x86 hardware that can run with a Free Software BIOS, and OpenPower isn't so much of an option for home use. I hope retaining the ability to build ceph so that it runs on such old and wise ;-) machines is not a major problem. Next in my wishlist is to try to fix the issues that I'm told are getting in the way of ceph's running on 32-bit x86. If anyone is familiar with them and could give me a brain dump to get me started, that would certainly be appreciated. I don't really have 32-bit machines running ceph daemons, but I have often recommended ceph to people who'd like to run it on SBCs connected to USB storage, so I was quite surprised and disappointed to find out even x86 wouldn't work any more. Plus, that messed up my uniform selection of packages on x86 and x86-64 machines with a single meta-package with all the dependencies I care for ;-)
On Tue, Aug 20, 2019 at 2:03 PM Alexandre Oliva <oliva@gnu.org> wrote: > > On Aug 19, 2019, kefu chai <tchaikov@gmail.com> wrote: > > > after a second thought, i think, probably, instead of patching SPDK to > > cater the needs of older machines. a better option is to disable SPDK > > when building packages for testing and for our official releases. > > I have no real clue on what role or impact disabling SPDK plays in the > grand scheme of things, so I'm happy to defer to whoever does. I just > wish to keep my home ceph cluster running on those machines until I have > good reason to replace them. Unfortunately that's the last generation > of x86 hardware that can run with a Free Software BIOS, and OpenPower > isn't so much of an option for home use. I hope retaining the ability > to build ceph so that it runs on such old and wise ;-) machines is not a > major problem. not at this moment =) > > Next in my wishlist is to try to fix the issues that I'm told are > getting in the way of ceph's running on 32-bit x86. If anyone is > familiar with them and could give me a brain dump to get me started, > that would certainly be appreciated. I don't really have 32-bit > machines running ceph daemons, but I have often recommended ceph to > people who'd like to run it on SBCs connected to USB storage, so I was > quite surprised and disappointed to find out even x86 wouldn't work any > more. Plus, that messed up my uniform selection of packages on x86 and > x86-64 machines with a single meta-package with all the dependencies I > care for ;-) i'd say it's a little bit off topic. i think we've fixed a bunch of issues[0] related to armhf. and i managed to build ceph in a chroot env for building armhf binaries. so i assume that Ceph does (or did) build on certain 32bit platforms at least. but since we don't test and/or build on 32bit platforms, this could change over the time. if you are able to pinpoint an FTBFS issue or bug while using Ceph on 32 bit platforms. i can take a look at it. --- [0] for instance, https://github.com/ceph/ceph/pull/25729 > > -- > 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
On Aug 20, 2019, kefu chai <tchaikov@gmail.com> wrote: > On Tue, Aug 20, 2019 at 2:03 PM Alexandre Oliva <oliva@gnu.org> wrote: >> to build ceph so that it runs on such old and wise ;-) machines is not a >> major problem. > not at this moment =) I can hardly imagine why it should ever be. Being disk-bound, and considering that even very old CPUs hardly have trouble keeping up with the much slower storage devices, I can't imagine a reason for ceph or anything portable across multiple architectures to tech to *demand* newer CPUs of any of the supported architectures. Hopefully there will always be some portable fallback to resort to. >> Next in my wishlist is to try to fix the issues that I'm told are >> getting in the way of ceph's running on 32-bit x86. If anyone is >> familiar with them and could give me a brain dump to get me started, >> that would certainly be appreciated. > if you are able to pinpoint an FTBFS issue or bug while using Ceph on > 32 bit platforms. i can take a look at it. Oh, thanks, I didn't mean to burden anyone with that, it's just something that I care about and would be happy to undertake myself. I was just surprised that there weren't i686 ceph packages in Fedora 30, and found comments in the spec file suggesting it didn't work, but I didn't look into why yet.
On Aug 20, 2019, Alexandre Oliva <oliva@gnu.org> wrote: >> if you are able to pinpoint an FTBFS issue or bug while using Ceph on >> 32 bit platforms. i can take a look at it. > Oh, thanks, I didn't mean to burden anyone with that, it's just > something that I care about and would be happy to undertake myself. I > was just surprised that there weren't i686 ceph packages in Fedora 30, > and found comments in the spec file suggesting it didn't work, but I > didn't look into why yet. Here's the patch I used to disable spdk in my rebuilds of the Fedora 30 packages. diff --git a/CMakeLists.txt b/CMakeLists.txt index 3b95cc231d2c..ba4fc8506651 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -267,7 +267,7 @@ if(WITH_BLUESTORE) endif() endif() -if(CMAKE_SYSTEM_PROCESSOR MATCHES "i386|i686|amd64|x86_64|AMD64|aarch64") +if(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64") option(WITH_SPDK "Enable SPDK" ON) else() option(WITH_SPDK "Enable SPDK" OFF) With the following additional tweaks, I could even build 32-bit x86 packages. The problems seem to be all related with int types. For example, this one fixes an assumption that size_t and unsigned are not the same type: diff --git a/src/include/buffer.h b/src/include/buffer.h index b8c78210eae0..8e010a150224 100644 --- a/src/include/buffer.h +++ b/src/include/buffer.h @@ -737,7 +737,7 @@ inline namespace v14_2_0 { void advance(int o) = delete; void advance(unsigned o); - void advance(size_t o) { advance(static_cast<unsigned>(o)); } + void advance(unsigned long o) { advance(static_cast<unsigned>(o)); } void seek(unsigned o); char operator*() const; iterator_impl& operator++(); This one was the simplest fix I could find to address problems in other overloads that did not expect unsigned, only int64_t and uint64_t. Though I'm not sure this change gets to external representations, I figured it made sense to settle on a well-defined width instead of letting it vary across platforms, and to go with the width used in the prevalent, working platforms: diff --git a/src/common/config_values.h b/src/common/config_values.h index ab52060e4629..17eb11d0329d 100644 --- a/src/common/config_values.h +++ b/src/common/config_values.h @@ -50,7 +50,7 @@ public: #define OPTION_OPT_U32(name) uint64_t name; #define OPTION_OPT_U64(name) uint64_t name; #define OPTION_OPT_UUID(name) uuid_d name; -#define OPTION_OPT_SIZE(name) size_t name; +#define OPTION_OPT_SIZE(name) uint64_t name; #define OPTION(name, ty) \ public: \ OPTION_##ty(name) This is probably the only fallout from the above: diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 19bd1a3efd5d..0dcf50ca10cb 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -1628,7 +1628,7 @@ void PrimaryLogPG::calc_trim_to() limit != pg_trim_to && pg_log.get_log().approx_size() > target) { size_t num_to_trim = std::min(pg_log.get_log().approx_size() - target, - cct->_conf->osd_pg_log_trim_max); + size_t(cct->_conf->osd_pg_log_trim_max)); if (num_to_trim < cct->_conf->osd_pg_log_trim_min && cct->_conf->osd_pg_log_trim_max >= cct->_conf->osd_pg_log_trim_min) { return; encode/decode don't handle long at all, they deal with u?int{32,64}_t and uint64_t. When int64_t happens to be int or long, this works, but when int64_t is long long, then long ends up uncovered: diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc index 8a5bc65d31da..adcdcf3b7b68 100644 --- a/src/test/bufferlist.cc +++ b/src/test/bufferlist.cc @@ -837,7 +837,7 @@ TEST(BufferListIterator, iterate_with_empties) { EXPECT_EQ(bl.length(), 0u); EXPECT_EQ(bl.get_num_buffers(), 1u); - encode(42l, bl); + encode(int64_t(42l), bl); EXPECT_EQ(bl.get_num_buffers(), 2u); bl.push_back(ceph::buffer::create(0)); @@ -853,11 +853,11 @@ TEST(BufferListIterator, iterate_with_empties) { bl.append(bl_with_empty_ptr); } - encode(24l, bl); + encode(int64_t(24l), bl); EXPECT_EQ(bl.get_num_buffers(), 5u); auto i = bl.cbegin(); - long val; + int64_t val; decode(val, i); EXPECT_EQ(val, 42l); @@ -865,7 +865,7 @@ TEST(BufferListIterator, iterate_with_empties) { EXPECT_EQ(val, 24l); val = 0; - i.seek(sizeof(long)); + i.seek(sizeof(val)); decode(val, i); EXPECT_EQ(val, 24l); EXPECT_TRUE(i == bl.end()); @@ -2665,7 +2665,7 @@ TEST(BufferList, InternalCarriage) { ceph::bufferlist bl; EXPECT_EQ(bl.get_num_buffers(), 0u); - encode(42l, bl); + encode(int64_t(42l), bl); EXPECT_EQ(bl.get_num_buffers(), 1u); { @@ -2678,7 +2678,7 @@ TEST(BufferList, InternalCarriage) { EXPECT_EQ(bl.get_num_buffers(), 2u); } - encode(24l, bl); + encode(int64_t(24l), bl); EXPECT_EQ(bl.get_num_buffers(), 3u); } @@ -2690,7 +2690,7 @@ TEST(BufferList, ContiguousAppender) { { auto ap = bl.get_contiguous_appender(100); - denc(42l, ap); + denc(int64_t(42l), ap); EXPECT_EQ(bl.get_num_buffers(), 1u); // append bufferlist with single ptr inside. This should @@ -2707,11 +2707,11 @@ TEST(BufferList, ContiguousAppender) { EXPECT_EQ(bl.get_num_buffers(), 3u); } - denc(24l, ap); + denc(int64_t(24l), ap); EXPECT_EQ(bl.get_num_buffers(), 3u); - EXPECT_EQ(bl.length(), sizeof(long) + 3u); + EXPECT_EQ(bl.length(), sizeof(int64_t) + 3u); } - EXPECT_EQ(bl.length(), 2u * sizeof(long) + 3u); + EXPECT_EQ(bl.length(), 2u * sizeof(int64_t) + 3u); } TEST(BufferList, TestPtrAppend) { That was all it took to build ceph-14.2.2-1.fc30 on x86_64 and i686. I haven't even had a chance to check that the resulting packages install, and I won't be able to get to them before I'm back home on Thu, but these are simple enough that I figured I'd post them before I got my attention on something else ;-) I hope this helps,
On Sun, Aug 25, 2019 at 4:43 AM Alexandre Oliva <oliva@gnu.org> wrote: > > On Aug 20, 2019, Alexandre Oliva <oliva@gnu.org> wrote: > > >> if you are able to pinpoint an FTBFS issue or bug while using Ceph on > >> 32 bit platforms. i can take a look at it. > > > Oh, thanks, I didn't mean to burden anyone with that, it's just > > something that I care about and would be happy to undertake myself. I > > was just surprised that there weren't i686 ceph packages in Fedora 30, > > and found comments in the spec file suggesting it didn't work, but I > > didn't look into why yet. > > Here's the patch I used to disable spdk in my rebuilds of the Fedora 30 > packages. > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 3b95cc231d2c..ba4fc8506651 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -267,7 +267,7 @@ if(WITH_BLUESTORE) > endif() > endif() > > -if(CMAKE_SYSTEM_PROCESSOR MATCHES "i386|i686|amd64|x86_64|AMD64|aarch64") > +if(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64") > option(WITH_SPDK "Enable SPDK" ON) > else() > option(WITH_SPDK "Enable SPDK" OFF) SPDK is disabled by default now, see https://github.com/ceph/ceph/pull/29728 > > With the following additional tweaks, I could even build 32-bit x86 > packages. The problems seem to be all related with int types. > > For example, this one fixes an assumption that size_t and unsigned are > not the same type: > > diff --git a/src/include/buffer.h b/src/include/buffer.h > index b8c78210eae0..8e010a150224 100644 > --- a/src/include/buffer.h > +++ b/src/include/buffer.h > @@ -737,7 +737,7 @@ inline namespace v14_2_0 { > > void advance(int o) = delete; > void advance(unsigned o); > - void advance(size_t o) { advance(static_cast<unsigned>(o)); } > + void advance(unsigned long o) { advance(static_cast<unsigned>(o)); } this function has been removed in a9f8b1a6092a923cae74221d0f1120d106d3cb8f > void seek(unsigned o); > char operator*() const; > iterator_impl& operator++(); > > This one was the simplest fix I could find to address problems in other > overloads that did not expect unsigned, only int64_t and uint64_t. > Though I'm not sure this change gets to external representations, I > figured it made sense to settle on a well-defined width instead of > letting it vary across platforms, and to go with the width used in the > prevalent, working platforms: > > diff --git a/src/common/config_values.h b/src/common/config_values.h > index ab52060e4629..17eb11d0329d 100644 > --- a/src/common/config_values.h > +++ b/src/common/config_values.h > @@ -50,7 +50,7 @@ public: > #define OPTION_OPT_U32(name) uint64_t name; > #define OPTION_OPT_U64(name) uint64_t name; > #define OPTION_OPT_UUID(name) uuid_d name; > -#define OPTION_OPT_SIZE(name) size_t name; > +#define OPTION_OPT_SIZE(name) uint64_t name; > #define OPTION(name, ty) \ > public: \ > OPTION_##ty(name) > > This is probably the only fallout from the above: the very same patch was applied in 58b0d514a67ce4f5c115f0f5451d1ac939b3702b > > diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc > index 19bd1a3efd5d..0dcf50ca10cb 100644 > --- a/src/osd/PrimaryLogPG.cc > +++ b/src/osd/PrimaryLogPG.cc > @@ -1628,7 +1628,7 @@ void PrimaryLogPG::calc_trim_to() > limit != pg_trim_to && > pg_log.get_log().approx_size() > target) { > size_t num_to_trim = std::min(pg_log.get_log().approx_size() - target, > - cct->_conf->osd_pg_log_trim_max); > + size_t(cct->_conf->osd_pg_log_trim_max)); > if (num_to_trim < cct->_conf->osd_pg_log_trim_min && > cct->_conf->osd_pg_log_trim_max >= cct->_conf->osd_pg_log_trim_min) { > return; i think it's okay now. as OPTION_OPT_U32 is now defined as a uint64_t, and approx_size() returns uint64_t as well. > > encode/decode don't handle long at all, they deal with u?int{32,64}_t > and uint64_t. When int64_t happens to be int or long, this works, but > when int64_t is long long, then long ends up uncovered: > > diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc > index 8a5bc65d31da..adcdcf3b7b68 100644 > --- a/src/test/bufferlist.cc > +++ b/src/test/bufferlist.cc > @@ -837,7 +837,7 @@ TEST(BufferListIterator, iterate_with_empties) { > EXPECT_EQ(bl.length(), 0u); > EXPECT_EQ(bl.get_num_buffers(), 1u); > > - encode(42l, bl); > + encode(int64_t(42l), bl); > EXPECT_EQ(bl.get_num_buffers(), 2u); > > bl.push_back(ceph::buffer::create(0)); > @@ -853,11 +853,11 @@ TEST(BufferListIterator, iterate_with_empties) { > bl.append(bl_with_empty_ptr); > } > > - encode(24l, bl); > + encode(int64_t(24l), bl); > EXPECT_EQ(bl.get_num_buffers(), 5u); > > auto i = bl.cbegin(); > - long val; > + int64_t val; > decode(val, i); > EXPECT_EQ(val, 42l); > > @@ -865,7 +865,7 @@ TEST(BufferListIterator, iterate_with_empties) { > EXPECT_EQ(val, 24l); > > val = 0; > - i.seek(sizeof(long)); > + i.seek(sizeof(val)); > decode(val, i); > EXPECT_EQ(val, 24l); > EXPECT_TRUE(i == bl.end()); > @@ -2665,7 +2665,7 @@ TEST(BufferList, InternalCarriage) { > ceph::bufferlist bl; > EXPECT_EQ(bl.get_num_buffers(), 0u); > > - encode(42l, bl); > + encode(int64_t(42l), bl); > EXPECT_EQ(bl.get_num_buffers(), 1u); > > { > @@ -2678,7 +2678,7 @@ TEST(BufferList, InternalCarriage) { > EXPECT_EQ(bl.get_num_buffers(), 2u); > } > > - encode(24l, bl); > + encode(int64_t(24l), bl); > EXPECT_EQ(bl.get_num_buffers(), 3u); > } > > @@ -2690,7 +2690,7 @@ TEST(BufferList, ContiguousAppender) { > { > auto ap = bl.get_contiguous_appender(100); > > - denc(42l, ap); > + denc(int64_t(42l), ap); > EXPECT_EQ(bl.get_num_buffers(), 1u); > > // append bufferlist with single ptr inside. This should > @@ -2707,11 +2707,11 @@ TEST(BufferList, ContiguousAppender) { > EXPECT_EQ(bl.get_num_buffers(), 3u); > } > > - denc(24l, ap); > + denc(int64_t(24l), ap); > EXPECT_EQ(bl.get_num_buffers(), 3u); > - EXPECT_EQ(bl.length(), sizeof(long) + 3u); > + EXPECT_EQ(bl.length(), sizeof(int64_t) + 3u); > } > - EXPECT_EQ(bl.length(), 2u * sizeof(long) + 3u); > + EXPECT_EQ(bl.length(), 2u * sizeof(int64_t) + 3u); > } > > TEST(BufferList, TestPtrAppend) { > thanks. i posted your patch as https://github.com/ceph/ceph/pull/29881 . > > That was all it took to build ceph-14.2.2-1.fc30 on x86_64 and i686. I > haven't even had a chance to check that the resulting packages install, > and I won't be able to get to them before I'm back home on Thu, but > these are simple enough that I figured I'd post them before I got my > attention on something else ;-) > > I hope this helps, it does! > > -- > 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
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