Skip to content

Conversation

@George-Miao
Copy link
Member

This PR refactors compio-buf with following changes:

  • Remove platform-dependent IoSlice{,Mut}. Use IoBuffer{,Mut} instead
  • IoBuf only requires single unsafe fn buffer(&self) -> IoBuffer and IoBufMut only requires fn uninit_len(&self) -> usize.
  • Platform-dependent IoSlice{,Mut} is being moved to driver, renamed to SysSlice and can be converted from IoBuffer{,Mut}.
  • Tidy up IoVectoredBuf{,Mut}:
    • IoVectoredBuf now only requires unsafe fn iter_buffer(&self) -> impl Iterator<Item = IoBuffer>;
    • IoVectoredBufMut now only requires fn uninit_len_of(&self, idx: usize) -> usize; which returns uninit_len of given index corresponding to index in iter_buffer.

@George-Miao George-Miao mentioned this pull request Dec 3, 2025
4 tasks
@George-Miao
Copy link
Member Author

George-Miao commented Dec 3, 2025

I'm thinking of removing the unsafe requirement for constructing IoBuffer{,Mut}, i.e., IoBuf::buffer. We treat it as something similar to as_ptr, where constructing the pointer is harmless (or even better, let it return a &[u8] directly). It's only unsafe at the consumption site, which is IoBuffer::slice (if we stick with IoBuffer) and all the OpCode within compio-driver.

What do you think @Berrysoft

@George-Miao George-Miao marked this pull request as draft December 3, 2025 07:10
@Berrysoft
Copy link
Member

It's OK. Actually I'm considering making the trait OpCode unsafe, so that we can leverage the unsafety to the driver.

@Berrysoft
Copy link
Member

Berrysoft commented Dec 3, 2025

Another reason we previously make it unsafe is that IoBuf{,Mut} is not guaranteed to be pinned, so the address might change after moving. But as you've said, as_ptr also don't ensure the stability of the pointer.

@George-Miao
Copy link
Member Author

George-Miao commented Dec 3, 2025

It's OK. Actually I'm considering making the trait OpCode unsafe, so that we can leverage the unsafety to the driver.

Sounds good to me. Then I'll try to implement slice-based buffer tomorrow.

@George-Miao
Copy link
Member Author

George-Miao commented Dec 3, 2025

Another reason we previously make it unsafe is that IoBuf{,Mut} is not guaranteed to be pinned, so the address might change after moving. But as you've said, as_ptr also don't ensure the stability of the pointer.

In fact we don't need to: let's suppose the new api looks like common fn as_slice<'a>(&'a self) -> &'a [u8] (I'm making 'a explicit here). What we are doing is basically calling as_slice with 'a being the duration of whole opcode. Then the validity of the pointer is automatically guaranteed by rust's ownership: the reference is valid forces the pointer of the data chunk to stay intact. Same applies to as_mut. And the 'a = OpCode part is guaranteed by driver, which is what I mean by "unsafe at the consumption site": driver cannot and will not touch or change the buffer itself; it takes the slice, construct some opcode based on it, hold on to it, and when the opcode is finished or cancelled, release the buffer.

@George-Miao George-Miao force-pushed the refactor/buf branch 9 times, most recently from 8beabb6 to 7430913 Compare December 4, 2025 10:23
@George-Miao George-Miao marked this pull request as ready for review December 4, 2025 10:44
@George-Miao George-Miao requested a review from Berrysoft December 4, 2025 10:44
@George-Miao George-Miao self-assigned this Dec 4, 2025
@George-Miao George-Miao added package: buf Related to compio-buf package: driver Related to compio-driver refactor Refactoring existing code labels Dec 4, 2025
@George-Miao George-Miao requested a review from Berrysoft December 4, 2025 21:09
Copy link
Member

@Berrysoft Berrysoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@George-Miao George-Miao merged commit 65917da into compio-rs:master Dec 5, 2025
53 checks passed
@George-Miao George-Miao deleted the refactor/buf branch December 5, 2025 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: buf Related to compio-buf package: driver Related to compio-driver refactor Refactoring existing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants