Add `SafePage::copy_to_page` to copy data from one page to another at a
given offset. Because `SafePage` cannot be mapped to user space or shared
with devices, there are no data races and the copy can be performed using
the existing `with_pointer_into_page` and `write_raw` methods.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/page.rs | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index af6d2ad408ed7..c780d10bcf909 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -23,6 +23,7 @@
marker::PhantomData,
mem::ManuallyDrop,
ops::{Deref, DerefMut},
+ pin::Pin,
ptr::{
self,
NonNull, //
@@ -407,6 +408,38 @@ pub fn alloc_page(flags: Flags) -> Result<Owned<Self>, AllocError> {
// Since `Page` and `SafePage` is transparent, we can cast the pointer directly.
Ok(unsafe { Owned::from_raw(page.cast()) })
}
+
+ /// Copies data from this page to another page at the specified offset.
+ ///
+ /// # Arguments
+ ///
+ /// - `dst` - The destination page to copy data to.
+ /// - `offset` - The byte offset within both pages where copying starts.
+ /// - `len` - The number of bytes to copy.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// # use kernel::page::SafePage;
+ /// # use kernel::alloc::flags::GFP_KERNEL;
+ /// let mut src_page = SafePage::alloc_page(GFP_KERNEL)?;
+ /// let mut dst_page = SafePage::alloc_page(GFP_KERNEL)?;
+ /// src_page.copy_to_page(dst_page.get_pin_mut(), 0, 1024)?;
+ /// # Ok::<(), kernel::error::Error>(())
+ /// ```
+ pub fn copy_to_page(&self, dst: Pin<&mut Self>, offset: usize, len: usize) -> Result {
+ // INVARIANT: The following code makes sure to not cause data races.
+ self.with_pointer_into_page(offset, len, |src| {
+ // SAFETY:
+ // - If `with_pointer_into_page` calls into this closure, then it has performed a
+ // bounds check and guarantees that `src` is valid for `len` bytes.
+ // - By type invariant and existence of shared reference, there are no other writes to
+ // `src` during this call.
+ // - By exclusive ownership of `dst`, there are no other writes to `dst` during this
+ // call.
+ unsafe { dst.write_raw(src, offset, len) }
+ })
+ }
}
// SAFETY: `Owned<SafePage>` objects returned by SafePage::alloc_page() follow the requirements of
--
2.51.2
On Sun, Feb 15, 2026 at 9:04 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: > > + /// Copies data from this page to another page at the specified offset. > + /// > + /// # Arguments > + /// > + /// - `dst` - The destination page to copy data to. > + /// - `offset` - The byte offset within both pages where copying starts. > + /// - `len` - The number of bytes to copy. We generally try to avoid this kind of argument-by-argument docs unless they are really needed. For instance, would this suffice? /// Copies `len` bytes from this page to another one at the specified byte offset. /// /// Copying starts within both pages at the same offset. > + /// ``` > + /// # use kernel::page::SafePage; > + /// # use kernel::alloc::flags::GFP_KERNEL; > + /// let mut src_page = SafePage::alloc_page(GFP_KERNEL)?; > + /// let mut dst_page = SafePage::alloc_page(GFP_KERNEL)?; > + /// src_page.copy_to_page(dst_page.get_pin_mut(), 0, 1024)?; > + /// # Ok::<(), kernel::error::Error>(()) > + /// ``` Could we show some error cases? In addition, why could the test fail here? i.e. if you use `?` in the "main line", then it means this could fail for reasons outside the test. If it cannot, please assert it instead. Also, couldn't you assert that some bytes were copied as expected? Could you show an error case with e.g. an out of bounds case? > + // - By type invariant and existence of shared reference, there are no other writes to > + // `src` during this call. If you use the type invariant here that you promise not to break above, isn't it circular logic? Why someone couldn't have another `&self` and call this? Cheers, Miguel
"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes: > On Sun, Feb 15, 2026 at 9:04 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: >> >> + /// Copies data from this page to another page at the specified offset. >> + /// >> + /// # Arguments >> + /// >> + /// - `dst` - The destination page to copy data to. >> + /// - `offset` - The byte offset within both pages where copying starts. >> + /// - `len` - The number of bytes to copy. > > We generally try to avoid this kind of argument-by-argument docs > unless they are really needed. Why? > > For instance, would this suffice? > > /// Copies `len` bytes from this page to another one at the > specified byte offset. > /// > /// Copying starts within both pages at the same offset. > >> + /// ``` >> + /// # use kernel::page::SafePage; >> + /// # use kernel::alloc::flags::GFP_KERNEL; >> + /// let mut src_page = SafePage::alloc_page(GFP_KERNEL)?; >> + /// let mut dst_page = SafePage::alloc_page(GFP_KERNEL)?; >> + /// src_page.copy_to_page(dst_page.get_pin_mut(), 0, 1024)?; >> + /// # Ok::<(), kernel::error::Error>(()) >> + /// ``` > > Could we show some error cases? > > In addition, why could the test fail here? i.e. if you use `?` in the > "main line", then it means this could fail for reasons outside the > test. If it cannot, please assert it instead. > > Also, couldn't you assert that some bytes were copied as expected? > Could you show an error case with e.g. an out of bounds case? I can demo an out of bounds error, sure. > >> + // - By type invariant and existence of shared reference, there are no other writes to >> + // `src` during this call. > > If you use the type invariant here that you promise not to break > above, isn't it circular logic? Why someone couldn't have another > `&self` and call this? Writes require a mutable reference. There cannot be a mutable reference while we have a shared reference. Best regards, Andreas Hindborg
On Mon, Feb 16, 2026 at 12:42 AM Andreas Hindborg <a.hindborg@kernel.org> wrote: > > Why? If you mean why we don't do it everywhere, then it is because for many functions it wouldn't add much value, but it would add substantial verbosity, which has a cost for both readers and writers. Originally, we picked the standard library style, because it seemed like a good balance that both had shown good results (especially for this language, where we have rich, strong types in signatures which help reduce the need) and that would get others to write docs easily. Sometimes it may be needed, e.g. there are many parameters with details to explain that wouldn't read well otherwise, or there are primitive integers parameters with constraints on them (instead of a newtype that enforces them) and so on. i.e. why do you think you need it here? When a reader sees the list, they will need to pause to read it, thinking there is something important/subtle there -- is there? (I say this as someone that generally likes structured, "exhaustive" documentation such as, say, the classic Win32 docs...) > Writes require a mutable reference. There cannot be a mutable reference > while we have a shared reference. Ok, but I am trying to map what you wrote with what the callee requires. In the second bullet point, you justify there are no races for the read side, and the third one for the write side. But you refer to the type invariant in the second one, for some reason, and that type invariant already promises no data races for `SafePage`, and all we have here are `SafePage`s on both sides, no? So to me it sounds like either you could justify everything just by invoking the type invariant (that is why I mentioned circular reasoning, because the type invariant doesn't seem justified itself in `// INVARIANT:`) or the type invariant is actually a different, weaker one (which would explain why you need extra explanations in `// SAFETY:` on top of the type invariant). (By the way, if we use bullet points, then I think we should map each to the callee's one, i.e. #2 and #3 would be together since #2 is the one in the callee about data races). Cheers, Miguel
"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes: > On Mon, Feb 16, 2026 at 12:42 AM Andreas Hindborg <a.hindborg@kernel.org> wrote: >> >> Why? > > If you mean why we don't do it everywhere, then it is because for many > functions it wouldn't add much value, but it would add substantial > verbosity, which has a cost for both readers and writers. > > Originally, we picked the standard library style, because it seemed > like a good balance that both had shown good results (especially for > this language, where we have rich, strong types in signatures which > help reduce the need) and that would get others to write docs easily. > > Sometimes it may be needed, e.g. there are many parameters with > details to explain that wouldn't read well otherwise, or there are > primitive integers parameters with constraints on them (instead of a > newtype that enforces them) and so on. > > i.e. why do you think you need it here? When a reader sees the list, > they will need to pause to read it, thinking there is something > important/subtle there -- is there? > > (I say this as someone that generally likes structured, "exhaustive" > documentation such as, say, the classic Win32 docs...) I would rather not get into an argument about things that are subjective, but if we picked a style, I should for sure follow that. If we picked a style for documenting argument lists, perhaps we should add it to Documentation/rust/coding-guidelines.rst? > >> Writes require a mutable reference. There cannot be a mutable reference >> while we have a shared reference. > > Ok, but I am trying to map what you wrote with what the callee > requires. In the second bullet point, you justify there are no races > for the read side, and the third one for the write side. But you refer > to the type invariant in the second one, for some reason, and that > type invariant already promises no data races for `SafePage`, and all > we have here are `SafePage`s on both sides, no? > > So to me it sounds like either you could justify everything just by > invoking the type invariant (that is why I mentioned circular > reasoning, because the type invariant doesn't seem justified itself in > `// INVARIANT:`) or the type invariant is actually a different, weaker > one (which would explain why you need extra explanations in `// > SAFETY:` on top of the type invariant). > > (By the way, if we use bullet points, then I think we should map each > to the callee's one, i.e. #2 and #3 would be together since #2 is the > one in the callee about data races). Others called out that the type invariant on `SafePage` is mushy. I will try to tighten that up. I want it to convey the information that the data of this page follows standard Rust aliasing rules. If you have a shared reference to a `SafePage`, there can be no writes to the data. If you have an exclusive reference, you are the only writer, and there are no other readers. I disagree that the bullets should be swapped. The second bullet at the call site: // - By type invariant and existence of shared reference, there are no other writes to // `src` during this call. Maps to the second bullet in the callee safety requirements: /// * Callers must ensure that there are no concurrent writes to the source memory region. The third bullet at the call site: // - By exclusive ownership of `dst`, there are no other writes to `dst` during this // call. Maps to the third bullet of the callee safety requirements: /// * Callers must ensure that this call does not race with a read or write to the same page /// that overlaps with this write. I think it checks out? `self` becomes `src` at the call site. But now that I look, I think it should say: // - By type invariant and existence of shared reference, there are no writes to // `src` during this call. That is, the word "other" is misleading. There are no writers, not us - not others. We are doing a read of `src`. Best regards, Andreas Hindborg
On Wed, Feb 18, 2026 at 10:37 AM Andreas Hindborg <a.hindborg@kernel.org> wrote: > > If we picked a style for documenting argument lists, perhaps we should > add it to Documentation/rust/coding-guidelines.rst? Sure, I will. (In general, I recommend following the overall style, i.e. only 3 places in the kernel use such an argument list, vs. all the others we have.) > I disagree that the bullets should be swapped. The second bullet at the > call site: > > // - By type invariant and existence of shared reference, there are no other writes to > // `src` during this call. > > Maps to the second bullet in the callee safety requirements: > > /// * Callers must ensure that there are no concurrent writes to the source memory region. Ah, I see what happens now -- that second bullet is not in mainline, but I see you are referring to this one you introduce here: https://lore.kernel.org/rust-for-linux/20260213-page-volatile-io-v3-1-d60487b04d40@kernel.org/ Is that new bullet needed though? Don't we already exclude data races when we state "valid for reads"? https://doc.rust-lang.org/std/ptr/index.html#safety I asked Benno about this -- let me put him in the To: so that he is in the loop. If "extra clarifications" are needed, then we could put them in the safety standard (Benno already has a table there with the shorthands etc.), rather than introducing redundancy in `# Safety` sections which can be quite confusing. Cheers, Miguel
© 2016 - 2026 Red Hat, Inc.