enhance SubmissionQueue to support generate SQE in place #119

jiangliu · 2021-12-01T10:10:34Z

Enhance io_uring with new interfaces to support prepare SQE in place. These new interfaces pass argument by reference instead of by value, which then could be used to support ringbahn.

introduce trait PrepareSQE for opcode
change opcode to implement the PrepareSQE trait
add SubmissionQueue::push_command()/push_commands() to generate SQE in place.

I have add some unit test cases/one benchmark case. According to my test result, there's no regression for existing SubmissionQueue::push(). The new SubmissionQueue::push_command() is slightly slower than push() by acceptable.

quininer · 2021-12-01T10:47:17Z

Can you describe what problem it solves?

jiangliu · 2021-12-02T07:15:51Z

Can you describe what problem it solves?

Sorry, I has misused an old version as work base, and found a bug in the old version and it's fixed in the latest code base.
BTW, I have tried to follow the way as you described, but feels it's a little complex to support push_multiple_opcode(). So I have slightly changed the design and post it here as RFC.
There are still more changes needed for opcode.rs, I will handle it once we have agreed on the design:)

quininer · 2021-12-02T08:11:43Z

The RefCell<()> added in this PR has nothing to do with our previous discussion and destroys the meaning of borrow_shared.

The borrow_shared is an unsafe api that requires the caller to ensure safety by themselves. It is used to implement cross-threaded apis such as owned split.

jiangliu · 2021-12-02T08:16:30Z

The RefCell<()> added in this PR has nothing to do with our previous discussion and destroys the meaning of borrow_shared.

The borrow_shared is an unsafe api that requires the caller to ensure safety by themselves. It is used to implement cross-threaded apis such as owned split.

Sure, I started working by reading all the code and then found something interesting:)
I will split that patch into dedicated thread.
BTW, ownedsplit is great future and definitely helps enabling rust async io with io uring.
I'm working on combing ringbahn and io-uring crates togetther:)

jiangliu · 2021-12-02T08:19:52Z

BTW, I failed to run "cargo run --package io-uring-test" with the latest code with an error:

  = note: /uring/target/debug/deps/io_uring_test-af1e753fdc660fe4.3x7eimgxzg68dtpw.rcgu.o: In function `io_uring_test::tests::fs::test_statx':
          /uring/io-uring-test/src/tests/fs.rs:316: undefined reference to `statx'
          collect2: error: ld returned 1 exit status

And the toolchain used is "nightly-x86_64-unknown-linux-gnu".
Anything special needed to run the test crate?

quininer · 2021-12-02T08:21:43Z

You may be using a distribution that does not support statx, see here.

jiangliu · 2021-12-02T08:34:28Z

The RefCell<()> added in this PR has nothing to do with our previous discussion and destroys the meaning of borrow_shared.

The borrow_shared is an unsafe api that requires the caller to ensure safety by themselves. It is used to implement cross-threaded apis such as owned split.

Aha, the RefCell actually helps ownedsplit. Say:

{
       let uring = IoUring::new();
       let (submit, sq, cq) = uring.split();
       let sq2 = sq.clone();
       let q1 = sq. submission();
       // With the RefCell changes, following statement should panic, which should be the expected behavior:)
       let q2 = sq2.submission();
}

jiangliu · 2021-12-02T08:37:32Z

The RefCell<()> added in this PR has nothing to do with our previous discussion and destroys the meaning of borrow_shared.
The borrow_shared is an unsafe api that requires the caller to ensure safety by themselves. It is used to implement cross-threaded apis such as owned split.

Aha, the RefCell actually helps ownedsplit. Say:
{
       let uring = IoUring::new();
       let (submit, sq, cq) = uring.split();
       let sq2 = sq.clone();
       let q1 = sq. submission();
       // With the RefCell changes, following statement should panic, which should be the expected behavior:)
       let q2 = sq2.submission();
}

My fault, SubmissionUring does not support Clone yet, so it should be safe.

jiangliu · 2021-12-17T08:37:26Z

Hi @quininer , I have updated the MR and feel it's ready for review:)

quininer · 2021-12-17T12:32:45Z

I missed it and I will look at it on the weekend.

quininer

I am a little bit dissatisfied with the api proposed by this PR. although it can indeed avoid build io_uring_sqe, but it cannot avoid need to build Opcode type once.

benchmarks show that it does not have many performance advantages compared to normal push. I am not sure if this change is worthwhile.

normal                  time:   [6.0094 us 6.0865 us 6.1785 us]
                        change: [-0.1695% +1.3807% +3.1320%] (p = 0.11 > 0.05)
                        No change in performance detected.

prepare                 time:   [6.3320 us 6.4818 us 6.6643 us]
                        change: [-8.5613% -5.6659% -2.6755%] (p = 0.00 < 0.05)
                        Performance has improved.

src/squeue.rs

quininer · 2021-12-18T13:17:09Z

src/squeue.rs

@@ -285,6 +354,44 @@ impl Entry {
    }
 }

+impl OptionValues {


The name should be more descriptive.

How about "SqeCommonOptions"?

src/squeue.rs

quininer · 2021-12-18T13:26:56Z

src/opcode.rs

+/// Trait to prepare an SQE from an opcode object.
+pub trait PrepareSQE {
+    /// Prepare an SQE from an opcode object.
+    fn prepare(&self, sqe: &mut sys::io_uring_sqe, options: Option<&OptionValues>);


It should not expose internal types, maybe this trait can not be exposed。

Both push_command() and push_commands() uses PrepareSQE as generic types.

jiangliu · 2022-01-03T10:03:13Z

@quininer I have implemented a version to prepare SQE in place. And take Readv as an example, the expanded code are as below:

    /// Vectored read, equivalent to `preadv2(2)`.
    pub struct Readv {
        fd: sealed::Target,
        iovec: *const libc::iovec,
        len: u32,
        ioprio: u16,
        offset: libc::off_t,
        rw_flags: types::RwFlags,
    }

    impl Readv {
        /// The opcode of the operation. This can be passed to
        /// [`Probe::is_supported`](crate::Probe::is_supported) to check if this operation is
        /// supported with the current kernel.
        pub const CODE: u8 = sys::IORING_OP_READV as _;
        #[inline]
        pub fn new(fd: impl sealed::UseFixed, iovec: *const libc::iovec, len: u32) -> Self {
            Readv {
                fd: fd.into(),
                iovec: iovec.into(),
                len: len.into(),
                ioprio: 0u16,
                offset: 0i64,
                rw_flags: 0i32,
            }
        }
        #[inline]
        pub const fn ioprio(mut self, ioprio: u16) -> Self {
            self.ioprio = ioprio;
            self
        }
        #[inline]
        pub const fn offset(mut self, offset: libc::off_t) -> Self {
            self.offset = offset;
            self
        }
        /// specified for read operations, contains a bitwise OR of per-I/O flags,
        /// as described in the `preadv2(2)` man page.
        #[inline]
        pub const fn rw_flags(mut self, rw_flags: types::RwFlags) -> Self {
            self.rw_flags = rw_flags;
            self
        }
        #[inline]
        pub fn build(self) -> Entry {
            let mut sqe = sqe_zeroed();
            self.prepare(&mut sqe);
            Entry(sqe)
        }
    }

    impl PrepareSQE for Readv {
        #[inline]
        fn prepare(&self, sqe: &mut sys::io_uring_sqe) {
            sqe.opcode = Readv::CODE;
            sqe.fd = self.fd.as_sqe_value() as _;
            sqe.__bindgen_anon_2.addr = self.iovec.as_sqe_value() as _;
            sqe.len = self.len.as_sqe_value() as _;
            sqe.ioprio = self.ioprio.as_sqe_value() as _;
            sqe.__bindgen_anon_1.off = self.offset.as_sqe_value() as _;
            sqe.__bindgen_anon_3.rw_flags = self.rw_flags.as_sqe_value() as _;
            {
                if match self.fd {
                    sealed::Target::Fixed(_) => true,
                    _ => false,
                } {
                    sqe.flags |= crate::squeue::Flags::FIXED_FILE.bits();
                };
            }
        }
    }

    /// Vectored read, equivalent to `preadv2(2)`.
    #[repr(transparent)]
    pub struct ReadvSqe {
        sqe: sys::io_uring_sqe,
    }

    impl ReadvSqe {
        /// The opcode of the operation. This can be passed to
        /// [`Probe::is_supported`](crate::Probe::is_supported) to check if this operation is
        /// supported with the current kernel.
        pub const CODE: u8 = sys::IORING_OP_READV as _;
        #[inline]
        pub fn prepare(&mut self, fd: sealed::Target, iovec: *const libc::iovec, len: u32) {
            self.sqe.opcode = Self::CODE;
            self.sqe.fd = fd.as_sqe_value() as _;
            self.sqe.__bindgen_anon_2.addr = iovec.as_sqe_value() as _;
            self.sqe.len = len.as_sqe_value() as _;
            self.sqe.ioprio = 0u16.as_sqe_value() as _;
            self.sqe.__bindgen_anon_1.off = 0i64.as_sqe_value() as _;
            self.sqe.__bindgen_anon_3.rw_flags = 0i32.as_sqe_value() as _;
            {
                if match fd {
                    sealed::Target::Fixed(_) => true,
                    _ => false,
                } {
                    self.sqe.flags |= crate::squeue::Flags::FIXED_FILE.bits();
                };
            }
        }
        #[inline]
        pub fn get_mut_sqe(&mut self) -> &mut sys::io_uring_sqe {
            &mut self.sqe
        }
        #[inline]
        pub fn ioprio(mut self, ioprio: u16) -> Self {
            self.sqe.ioprio = ioprio.as_sqe_value() as _;
            self
        }
        #[inline]
        pub fn offset(mut self, offset: libc::off_t) -> Self {
            self.sqe.__bindgen_anon_1.off = offset.as_sqe_value() as _;
            self
        }
        /// specified for read operations, contains a bitwise OR of per-I/O flags,
        /// as described in the `preadv2(2)` man page.
        #[inline]
        pub fn rw_flags(mut self, rw_flags: types::RwFlags) -> Self {
            self.sqe.__bindgen_anon_3.rw_flags = rw_flags.as_sqe_value() as _;
            self
        }
    }

    impl<'a> From<&'a mut sys::io_uring_sqe> for &'a mut ReadvSqe {
        #[inline]
        fn from(sqe: &'a mut sys::io_uring_sqe) -> &'a mut ReadvSqe {
            unsafe { mem::transmute(sqe) }
        }
    }

Introduce helper next_sqe() and move_forward() for SQ, it will be reused in following patches. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Introduce trait PrepareSQE to support prepare SQE in place by calling opcode.prepare(&mut sqe). Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add SubmissionQueue::push_command()/push_commands() to generate SQE in place. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add unit test cases and benchmark for push_command(). Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Currently the opcode module provides a data structure for each io-uring operation code. And it works in following way to submit an SQE: - generate an opcode object, such Nop, Readv etc. - convert the opcode object into an SQE object - copy the generated SQE object onto the next available SQE in the submission queue One previous patch in this series implements PrepareSQE to optimize the process to submit a request as: - generate an opcode object, such Nop, Readv etc. - convert the opcode object to an SQE in the submission queue in place The process to submit a request could be further optimized as: - get the next available SQE from the submission queue - prepare the SQE for io uring operation - commit the SQE This patch enhance the opcode module to prepare SQE in place, without involving the intermedia opcode structures. The way to achieve the goal is to introduce a structure for each opcode to support in-place preparatio. #[repr(transparent)] pub struct ReadvSqe { sqe: sys::io_uring_sqe, } impl ReadvSqe { #[inline] pub fn prepare(&mut self, fd: sealed::Target, iovec: *const libc::iovec, len: u32) { } #[inline] pub fn ioprio(mut self, ioprio: u16) -> Self { self.sqe.ioprio = ioprio.as_sqe_value() as _; self } } impl<'a> From<&'a mut sys::io_uring_sqe> for &'a mut ReadvSqe { #[inline] fn from(sqe: &'a mut sys::io_uring_sqe) -> &'a mut ReadvSqe { unsafe { mem::transmute(sqe) } } } Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Export SubmissionQueue::get_available_sqe() to prepare available SQE for in-place preparation. And export SubmissionQueue::move_forward() to commit prepared SQEs. Sample code to use the new interface: pub fn prepare_sqe(mut sq: SubmissionQueue<'_>) { unsafe { match sq.get_available_sqe(0) { Ok(sqe) => { let nop_sqe: &mut crate::opcode::NopSqe = sqe.into(); nop_sqe.prepare(); sq.move_forward(1); } Err(_) => return, } } } Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add benchmark for preparing SQE in place. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

jiangliu · 2022-01-03T12:22:53Z

I have also conduct a round of benchmark on one platform. For the Nop tests, the results are as below(in us):

version	normal	prepare	prepare_sqe
original	13.782
	13.834
	13.815
first version	13.786	13.776
	13.817	13.829
	13.789	14.143
second version	13.779	13.818	13.966
	13.704	13.754	13.939
	13.734	13.825	13.931

For detailed analysis of the performance result, please refer to to #116

There are slight performance degradations for Nop, but I think the performance behave may change for actual workloads.

And the new interface enables new usages of the io uring crate:)

quininer · 2022-01-04T07:01:55Z

This is the same as I expected. opcode is just an entry builder by design. It should be constructed simply and pushed into sq immediately, so it should always be easy to optimize.

If this PR does not provide a visible performance improvement, I tend not to accept it.
Especially I don't like the API it introduces.

jiangliu · 2022-01-04T07:47:25Z

Thanks for your patience, let's close it because it doesn't bring much values:)

quininer · 2022-01-04T08:04:48Z

Anyway, thank you for trying, thank you. :D

jiangliu force-pushed the refine branch from 3f1628c to 86ee27b Compare December 1, 2021 10:14

jiangliu force-pushed the refine branch from 86ee27b to db22b95 Compare December 2, 2021 07:17

jiangliu changed the title ~~[WIP] Enhance io-uring for safety~~ [RFC] enhance submission queue to generate SQE in place Dec 2, 2021

jiangliu changed the title ~~[RFC] enhance submission queue to generate SQE in place~~ [RFC] enhance SubmissionQueue to support generate SQE in place Dec 2, 2021

jiangliu force-pushed the refine branch from db22b95 to c354912 Compare December 2, 2021 08:00

jiangliu force-pushed the refine branch from c354912 to f7758da Compare December 8, 2021 06:46

jiangliu changed the title ~~[RFC] enhance SubmissionQueue to support generate SQE in place~~ enhance SubmissionQueue to support generate SQE in place Dec 8, 2021

jiangliu force-pushed the refine branch from f7758da to 6d789cb Compare December 8, 2021 06:56

quininer reviewed Dec 18, 2021

View reviewed changes

jiangliu force-pushed the refine branch from 6d789cb to 36bbda6 Compare January 3, 2022 08:58

jiangliu force-pushed the refine branch from 36bbda6 to 2ca37df Compare January 3, 2022 10:25

jiangliu added 7 commits January 3, 2022 18:44

Introduce helper next_sqe() and move_forward() for SQ

9c161d1

Introduce helper next_sqe() and move_forward() for SQ, it will be reused in following patches. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Introduce trait PrepareSQE for opcode

fef41a8

Introduce trait PrepareSQE to support prepare SQE in place by calling opcode.prepare(&mut sqe). Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add SubmissionQueue::push_command()/push_commands()

e6c7b67

Add SubmissionQueue::push_command()/push_commands() to generate SQE in place. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add unit test cases and benchmark for push_command()

0e6299c

Add unit test cases and benchmark for push_command(). Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add benchmark for preparing SQE in place

3c6f30d

Add benchmark for preparing SQE in place. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

jiangliu force-pushed the refine branch from 2ca37df to 3c6f30d Compare January 3, 2022 10:45

jiangliu closed this Jan 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enhance SubmissionQueue to support generate SQE in place #119

enhance SubmissionQueue to support generate SQE in place #119

jiangliu commented Dec 1, 2021 •

edited

Loading

quininer commented Dec 1, 2021

jiangliu commented Dec 2, 2021

quininer commented Dec 2, 2021

jiangliu commented Dec 2, 2021

jiangliu commented Dec 2, 2021

quininer commented Dec 2, 2021

jiangliu commented Dec 2, 2021

jiangliu commented Dec 2, 2021

jiangliu commented Dec 17, 2021

quininer commented Dec 17, 2021

quininer left a comment •

edited

Loading

quininer Dec 18, 2021

jiangliu Jan 2, 2022

quininer Dec 18, 2021

jiangliu Jan 2, 2022 •

edited

Loading

jiangliu commented Jan 3, 2022 •

edited

Loading

jiangliu commented Jan 3, 2022 •

edited

Loading

quininer commented Jan 4, 2022

jiangliu commented Jan 4, 2022

quininer commented Jan 4, 2022

enhance SubmissionQueue to support generate SQE in place #119

enhance SubmissionQueue to support generate SQE in place #119

Conversation

jiangliu commented Dec 1, 2021 • edited Loading

quininer commented Dec 1, 2021

jiangliu commented Dec 2, 2021

quininer commented Dec 2, 2021

jiangliu commented Dec 2, 2021

jiangliu commented Dec 2, 2021

quininer commented Dec 2, 2021

jiangliu commented Dec 2, 2021

jiangliu commented Dec 2, 2021

jiangliu commented Dec 17, 2021

quininer commented Dec 17, 2021

quininer left a comment • edited Loading

Choose a reason for hiding this comment

quininer Dec 18, 2021

Choose a reason for hiding this comment

jiangliu Jan 2, 2022

Choose a reason for hiding this comment

quininer Dec 18, 2021

Choose a reason for hiding this comment

jiangliu Jan 2, 2022 • edited Loading

Choose a reason for hiding this comment

jiangliu commented Jan 3, 2022 • edited Loading

jiangliu commented Jan 3, 2022 • edited Loading

quininer commented Jan 4, 2022

jiangliu commented Jan 4, 2022

quininer commented Jan 4, 2022

jiangliu commented Dec 1, 2021 •

edited

Loading

quininer left a comment •

edited

Loading

jiangliu Jan 2, 2022 •

edited

Loading

jiangliu commented Jan 3, 2022 •

edited

Loading

jiangliu commented Jan 3, 2022 •

edited

Loading