Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MDEV-23892: Misc fixes to MIPS build and run compatibility #1716

Closed
wants to merge 3 commits into from
Closed

MDEV-23892: Misc fixes to MIPS build and run compatibility #1716

wants to merge 3 commits into from

Conversation

ottok
Copy link
Contributor

@ottok ottok commented Dec 20, 2020

These patches have been in Debian for several years or months already and have thus proven themselves to be the correct solutions for certain libatomic etc issues so that MariaDB builds and runs correctly mainly on MIPS but as a side effect also on other platforms.

When this PR is merged we can drop 4 patches from https://salsa.debian.org/mariadb-team/mariadb-10.5/-/tree/master/debian/patches

@ottok ottok changed the base branch from 10.6 to 10.5 December 20, 2020 19:02
@ottok ottok force-pushed the ok-10.5-mips-fixes branch 2 times, most recently from 6d805ae to 471f77c Compare December 25, 2020 15:37
@ottok
Copy link
Contributor Author

ottok commented Dec 26, 2020

Quality assurance

Salsa-CI

Passes

Travis-CI

Passes

Buildbot

Overview visible at http://buildbot.askmonty.org/buildbot/grid?branch=ok-10.5-mips-fixes&category=main&category=experimental

On http://buildbot.askmonty.org/buildbot/builders/kvm-bintar-trusty-amd64/builds/20761/steps/compile/logs/stdio the configure steps fails on:

-- Check if the system is big endian
-- Searching 16 bit integer
-- Check size of unsigned short
-- Check size of unsigned short - failed
-- Check size of unsigned int
-- Check size of unsigned int - failed
-- Check size of unsigned long
-- Check size of unsigned long - failed
CMake Error at /usr/share/cmake-2.8/Modules/TestBigEndian.cmake:44 (message):
  no suitable type found
Call Stack (most recent call first):
  configure.cmake:486 (TEST_BIG_ENDIAN)
  CMakeLists.txt:370 (INCLUDE)

The job at http://buildbot.askmonty.org/buildbot/builders/kvm-deb-stretch-amd64/builds/17874/steps/minor-upgrade-all/logs/stdio fails on:

+	libatomic.so.1 => /usr/lib/x86_64-linux-gnu/libatomic.so.1 
...
ERROR: something has changed in the dependencies of binaries or libraries. See the diff above

This is however intentional as there is a new dependency added.

@grooverdan grooverdan changed the title Misc fixes to MIPS build and run compatibility MDEV-23892: Misc fixes to MIPS build and run compatibility Jan 10, 2021
@ottok
Copy link
Contributor Author

ottok commented Mar 22, 2021

@cvicentiu Is this OK to merge? It is your original code, I think you should finalize this.

@an3l an3l added this to the 10.5 milestone Mar 25, 2021
@grooverdan
Copy link
Member

drop the _bss_start patch - MDEV-23491 was fixed by removing _bss_start references.

libatomics looks good.

connect engine patch, @an3l please take a look.

@grooverdan grooverdan mentioned this pull request Apr 14, 2021
Closed
@grooverdan grooverdan requested a review from an3l April 15, 2021 00:18
@ottok
Copy link
Contributor Author

ottok commented May 9, 2021

FYI: I had to update this patch downstream where it is still maintained. I wonder why it hasn't been merged upstream, should be pretty clear cut thing.

https://salsa.debian.org/mariadb-team/mariadb-10.5/-/commit/12d5cef22365db16ca22766bcbb1e3475b7f1f09#7867810dd1d691e3c39b7941a80bc35405fae55d_40_39

@ottok
Copy link
Contributor Author

ottok commented May 24, 2021

I rebased this now on latest 10.5. Please merge @cvicentiu

I already had to update the patches when importing MariaDB 10.5.10 to Debian (https://salsa.debian.org/mariadb-team/mariadb-10.5/-/commit/12d5cef22365db16ca22766bcbb1e3475b7f1f09#7867810dd1d691e3c39b7941a80bc35405fae55d_40_39). It is waste of my time to maintain these patches in Debian when the it could be permanently solved by simply merging these patches at upstream now.

@grooverdan
Copy link
Member

I did have a clear cut ask for to drop the _bss_start patch - MDEV-23491 as it was fixed by removing _bss_start references.

Note Debian should make use of CMAKE_CROSSCOMPILING_EMULATOR to eliminate some of the cross compilation build time assumptions especially on upstream no-supported architectures less fragile.

@ottok
Copy link
Contributor Author

ottok commented May 28, 2021

@grooverdan These patches were originally written mostly by @cvicentiu, they are in Debian and are proven to fix the MIPS issues. I suggest these would be merged now (unless a bug is found), and further improvements done in a later iteration.

Regarding CMAKE_CROSSCOMPILING_EMULATOR, I read https://manpages.debian.org/stretch/cmake-data/cmake-variables.7.en.html#CMAKE_CROSSCOMPILING_EMULATOR and looked quickly on https://codesearch.debian.net/search?q=CMAKE_CROSSCOMPILING_EMULATOR&literal=1&perpkg=1 how other Debian packages might be using it, but I didn't find a clear purpose and method to use it.

Note that we already have it in MariaDB CMake files. I don't have critisism to the current use nor any ideas on how to use CMAKE_CROSSCOMPILING_EMULATOR in a better way.

ottok added 3 commits May 29, 2021 22:00
@ottok
Link with libatomic to enable C11 atomics support
3c1a3e9 
Some architectures (mips) require libatomic to support proper
atomic operations. Check first if support is available without
linking, otherwise use the library.

Contributors:
James Cowgill <jcowgill@debian.org>
Jessica Clarke <jrtc27@debian.org>
Vicențiu Ciorbaru <vicentiu@mariadb.org>
@ottok
Fix MIPS build failure: Handle unaligned buffers in connect's TYPBLK …
fedf625 
…class

On MIPS platforms (and probably others) unaligned memory access results in a
bus error. In the connect storage engine, block data for some data formats is
stored packed in memory and the TYPBLK class is used to read values from it.
Since TYPBLK does not have special handling for this packed memory, it can
quite easily result in unaligned memory accesses.

The simple way to fix this is to perform all accesses to the main buffer
through memcpy. With GCC and optimizations turned on, this call to memcpy is
completely optimized away on architectures where unaligned accesses are ok
(like x86).

Contributors:
James Cowgill <jcowgill@debian.org>
@ottok
Fix Groonga crash on MIPS: Correctly link to libatomic
b4a6a97 
MIPS (and possibly other) platforms require linking against libatomic to
support 64-bit atomic integers. Groonga was failing to do so and all related
tests were failing with an atomics relocation error on MIPS.

Contributors:
James Cowgill <jcowgill@debian.org>
@ottok
Copy link
Contributor Author

ottok commented Jun 4, 2021

@an3l @cvicentiu Any comments here? Ok to merge?

@an3l
Copy link
Contributor

an3l commented Jun 4, 2021 •
edited

Hi @ottok regarding CONNECT SE I think it is ok, but cmake parts of patch @cvicentiu or @grooverdan should confirm please.

@ottok
Copy link
Contributor Author

ottok commented Oct 12, 2021

@cvicentiu This is your code, please finalize and merge it. Thanks!

@cvicentiu
Copy link
Member

@ottok I have merged the Connect and MIPS libatomic linkage fix. The Mroonga patch doesn't work as expected, it fails in BB. I will close this PR when I have a proper fix for it.

@cvicentiu
Copy link
Member

Merged with:
1388845 (mroonga changes)
a33c108 (connect changes)
f502ccb (libatomic for mysys)

@cvicentiu cvicentiu closed this Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@an3l an3l Awaiting requested review from an3l

Assignees

@cvicentiu cvicentiu

Labels
None yet
Milestone
10.5
4 participants
@ottok @grooverdan @an3l @cvicentiu