cmake: fix WITH_SYSTEM_LIBURING and make uring available outside of src/blk#50821
Merged
cmake: fix WITH_SYSTEM_LIBURING and make uring available outside of src/blk#50821
Conversation
Contributor
Author
|
debian builds will continue to build from submodule, because liburing-dev is available for jammy but not focal |
9ba9e38 to
5a6ebe3
Compare
Contributor
Author
|
i tried enabling WITH_SYSTEM_LIBURING for centos builds, but that seems to have broken rocksdb builds, so i reverted that |
Contributor
|
A good example of a speedup we could get from dropping Ubuntu Focal |
Contributor
Author
|
jenkins test make check |
idryomov
reviewed
Apr 26, 2023
|
|
||
| const size_t OBJECT_SIZE = 4 * 1024 * 1024; | ||
| const uint64_t DATA_OFFSET = MockCryptoInterface::DATA_OFFSET; | ||
| const uint64_t DATA_OFFSET = MockCryptoInterface::MOCK_CRYPTO_INTERFACE_DATA_OFFSET; |
Contributor
There was a problem hiding this comment.
I'm not sure I understand the rationale:
there's a conflicting BLOCK_SIZE macro defined inside
the liburing headers.
Is there an expectation that a random librbd unit test would end up including liburing.h now? Is it because it would be forced by one of the Boost headers?
Contributor
Author
There was a problem hiding this comment.
the BOOST_ASIO_HAS_IO_URING define causes boost/asio.hpp to include this header:
In file included from /usr/include/liburing/io_uring.h:11,
from /usr/include/liburing.h:25,
from /home/cbodley/ceph/build/boost/include/boost/asio/detail/io_uring_operation.hpp:22,
from /home/cbodley/ceph/build/boost/include/boost/asio/detail/io_uring_descriptor_read_at_op.hpp:28,
from /home/cbodley/ceph/build/boost/include/boost/asio/detail/io_uring_descriptor_service.hpp:28,
from /home/cbodley/ceph/build/boost/include/boost/asio/detail/io_uring_file_service.hpp:26,
from /home/cbodley/ceph/build/boost/include/boost/asio/basic_file.hpp:39,
from /home/cbodley/ceph/build/boost/include/boost/asio.hpp:28,
from /home/cbodley/ceph/src/include/neorados/RADOS.hpp:27,
from /home/cbodley/ceph/src/test/librbd/mock/MockImageCtx.cc:4:
/home/cbodley/ceph/src/test/librbd/mock/crypto/MockCryptoInterface.h:16:25: error: expected unqualified-id before numeric constant
16 | static const uint64_t BLOCK_SIZE = 4096;
| ^~~~~~~~~~
cc @adamemerson we might consider changing neorados/RADOS.hpp to use more granular includes of asio headers to avoid this dependency on liburing (and to reduce compilation times in general)
Contributor
Author
There was a problem hiding this comment.
we might consider changing neorados/RADOS.hpp to use more granular includes of asio headers
i added some more commits for this, along with a SQUASHME commit that reverts the changes to this librbd test
49e78a0 to
65945b9
Compare
65945b9 to
d6b233c
Compare
cbodley
commented
Aug 7, 2023
d6b233c to
9c6b979
Compare
Contributor
Author
|
shaman builds failing with: |
Contributor
Author
|
more stuff to work through in the neorados tests: |
809afa1 to
21d4d85
Compare
Contributor
Author
|
jenkins test make check |
Contributor
Author
|
builds are green in https://shaman.ceph.com/builds/ceph/wip-cmake-uring/3d7427c267fdd40b6d00c956b103b3d9a8b6caf1/, tagging for qa |
Contributor
|
Hrm, |
Contributor
|
jenkins test make check |
Contributor
Nothing to go on here, filed https://tracker.ceph.com/issues/63222 just for tracking purposes. |
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
liburing-devel package provides `/usr/lib64/liburing.so`. without this
fix, `find_package(uring)` failed with this `CMAKE_FIND_DEBUG_MODE`
output:
```
find_library considered the following locations:
/home/cbodley/ceph/build/virtualenv/bin//(lib)liburing.a(\.so|\.a)
/home/cbodley/ceph/build/virtualenv/bin/(lib)liburing.a(\.so|\.a)
/usr/local/bin//(lib)liburing.a(\.so|\.a)
/usr/local/bin/(lib)liburing.a(\.so|\.a)
/usr/local/sbin//(lib)liburing.a(\.so|\.a)
/usr/local/sbin/(lib)liburing.a(\.so|\.a)
/usr/bin//(lib)liburing.a(\.so|\.a)
/usr/bin/(lib)liburing.a(\.so|\.a)
/usr/sbin//(lib)liburing.a(\.so|\.a)
/usr/sbin/(lib)liburing.a(\.so|\.a)
/usr/local/lib64//(lib)liburing.a(\.so|\.a)
/usr/local/lib64/(lib)liburing.a(\.so|\.a)
/usr/local/lib//(lib)liburing.a(\.so|\.a)
/usr/local/lib/(lib)liburing.a(\.so|\.a)
/usr/local//(lib)liburing.a(\.so|\.a)
/usr/local/(lib)liburing.a(\.so|\.a)
/usr/lib64//(lib)liburing.a(\.so|\.a)
/usr/lib64/(lib)liburing.a(\.so|\.a)
/usr/lib//(lib)liburing.a(\.so|\.a)
/usr/lib/(lib)liburing.a(\.so|\.a)
/usr//(lib)liburing.a(\.so|\.a)
/usr/(lib)liburing.a(\.so|\.a)
/lib64//(lib)liburing.a(\.so|\.a)
/lib64/(lib)liburing.a(\.so|\.a)
/lib//(lib)liburing.a(\.so|\.a)
/lib/(lib)liburing.a(\.so|\.a)
/opt//(lib)liburing.a(\.so|\.a)
/opt/(lib)liburing.a(\.so|\.a)
/usr/lib64/X11//(lib)liburing.a(\.so|\.a)
/usr/lib64/X11/(lib)liburing.a(\.so|\.a)
```
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
this added a dependency on uring headers where it wasn't necessary. pick_address.cc was relying on that for the IFF_UP define Signed-off-by: Casey Bodley <cbodley@redhat.com>
the use of boost::asio::posix::stream_descriptor requires the concrete header instead of posix/basic_stream_descriptor.hpp Signed-off-by: Casey Bodley <cbodley@redhat.com>
avoid the boost/asio.hpp convenience header, as it pulls in a dependency on liburing Signed-off-by: Casey Bodley <cbodley@redhat.com>
and use the updated class name io_context Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
3d7427c to
fea2618
Compare
Contributor
Author
|
thanks @yuriw @ljflores @NitzanMordhai |
Contributor
|
@cbodley This has caused a regression in librbd in the place where it uses ASIO to connect and fetch an image via HTTP: Here is the job in question on main (+ unrelated RBD PR), ran 10 times: And here is the same job with 05c341b and f479bbc from this PR reverted: I think the issue is that header file reshuffling in librados lead to 29ee772 (Jason's workaround for what is arguably a bug in ASIO, see chriskohlhoff/asio#642) to break. Some of the static initialization no longer occurs in librados and therefore is no longer covered by that |
14 tasks
tchaikov
added a commit
to tchaikov/ceph
that referenced
this pull request
Mar 5, 2026
BOOST_ASIO_HAS_IO_URING is a user-defined macro that enables io_uring
support in Boost.Asio for specific operations that explicitly opt in,
such as file I/O via basic_file (BOOST_ASIO_HAS_FILE) and buffer
registration. Unlike BOOST_ASIO_HAS_IO_URING_AS_DEFAULT, it does NOT
replace epoll as the default event loop. Currently, Ceph does not use
Boost.Asio to perform file I/O, so this macro only affects compilation,
not runtime behavior.
When BOOST_ASIO_HAS_IO_URING is defined, the umbrella header
boost/asio.hpp pulls in <liburing.h> via:
boost/asio.hpp
-> boost/asio/basic_file.hpp (guarded by BOOST_ASIO_HAS_FILE)
-> boost/asio/detail/io_uring_file_service.hpp
-> boost/asio/detail/io_uring_descriptor_service.hpp
-> boost/asio/detail/io_uring_service.hpp
-> <liburing.h>
PR ceph#50821 introduced BOOST_ASIO_HAS_IO_URING (commit 05c341b) as
preparation for uring-based file I/O in rgw/posix. To avoid pulling
liburing headers (which define macros like BLOCK_SIZE and DATA_OFFSET
that conflict with Ceph code) into unrelated translation units, that PR
also converted many files to use granular Boost.Asio includes instead of
the umbrella boost/asio.hpp.
However, BOOST_ASIO_HAS_IO_URING was added as a global compile
definition, and individual targets linked uring::uring directly. This
has two problems:
1. The uring::uring CMake target only exists when WITH_LIBURING=ON, so
targets that link it unconditionally break when WITH_LIBURING=OFF.
2. The global definition causes all translation units to see
BOOST_ASIO_HAS_IO_URING, even those that don't need io_uring.
Since neither CMake's FindBoost module nor Boost's CMake config file
defines a Boost::asio target, introduce one as an INTERFACE IMPORTED
library. When WITH_LIBURING=ON, the Boost::asio target carries the
BOOST_ASIO_HAS_IO_URING definition and uring::uring dependency; when
WITH_LIBURING=OFF, it is an empty interface. Targets that don't need
io_uring should use Boost::headers or Boost::boost instead.
This also improves code organization:
- Scope the BOOST_ASIO_HAS_IO_URING compile definition to Boost::asio
instead of adding it globally, so only targets that actually use
Boost.Asio with io_uring see the definition.
- Place the uring-in-Boost::asio configuration next to where we
build/detect Boost libraries for better readability.
- Set the Boost::asio properties in a single place for better
maintainability.
See also
- 05c341b
- f479bbc
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this is preparation for the use of boost::asio's uring-based file abstractions in #50635
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows