| Message ID | 20241015-page-align-v1-1-68fbd8b6d10c@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust: page: add Rust version of PAGE_ALIGN | expand |
On Tue, Oct 15, 2024 at 02:28:28PM +0000, Alice Ryhl wrote: > This is a useful for helper for working with indices into buffers that > consist of several pages. I forgot to include it when I added PAGE_SIZE > and PAGE_MASK for the same purpose in commit fc6e66f4696b ("rust: add > abstraction for `struct page`"). > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > rust/kernel/page.rs | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs > index 208a006d587c..90846e3fe829 100644 > --- a/rust/kernel/page.rs > +++ b/rust/kernel/page.rs > @@ -20,6 +20,11 @@ > /// A bitmask that gives the page containing a given address. > pub const PAGE_MASK: usize = !(PAGE_SIZE - 1); > > +/// Round up the given number to a multiple of `PAGE_SIZE`. > +pub fn page_align(addr: usize) -> usize { Make it a `const` function? For example, there is a VMBUS_RING_SIZE() macro in C that calculates a const number for a vmbus ringbuffer size, so it will be useful. > + (addr + (PAGE_SIZE - 1)) & PAGE_MASK I guess overflows are unexpected, i.e. the users should not pass a `addr` that `> usize::MAX - PAGE_SIZE + 1`? Regards, Boqun > +} > + > /// A pointer to a page that owns the page allocation. > /// > /// # Invariants > > --- > base-commit: 8d8f785ceb21b9a0de11e05b811cc52d6fa79318 > change-id: 20241015-page-align-7e5fa4c751be > > Best regards, > -- > Alice Ryhl <aliceryhl@google.com> >
On Tue, Oct 15, 2024 at 08:44:22PM +0200, Alice Ryhl wrote: > On 10/15/24 8:33 PM, Boqun Feng wrote: > > On Tue, Oct 15, 2024 at 02:28:28PM +0000, Alice Ryhl wrote: > > > This is a useful for helper for working with indices into buffers that > > > consist of several pages. I forgot to include it when I added PAGE_SIZE > > > and PAGE_MASK for the same purpose in commit fc6e66f4696b ("rust: add > > > abstraction for `struct page`"). > > > > > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > > > --- > > > rust/kernel/page.rs | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs > > > index 208a006d587c..90846e3fe829 100644 > > > --- a/rust/kernel/page.rs > > > +++ b/rust/kernel/page.rs > > > @@ -20,6 +20,11 @@ > > > /// A bitmask that gives the page containing a given address. > > > pub const PAGE_MASK: usize = !(PAGE_SIZE - 1); > > > +/// Round up the given number to a multiple of `PAGE_SIZE`. > > > +pub fn page_align(addr: usize) -> usize { > > > > Make it a `const` function? For example, there is a VMBUS_RING_SIZE() > > macro in C that calculates a const number for a vmbus ringbuffer size, > > so it will be useful. > > Good idea. > > > > + (addr + (PAGE_SIZE - 1)) & PAGE_MASK > > > > I guess overflows are unexpected, i.e. the users should not pass a > > `addr` that `> usize::MAX - PAGE_SIZE + 1`? > Correct. If this wraps around to zero, that's incorrect. Note that the minus > one is in brackets to only trigger overflow detection in the right cases. > Make senses, with the const change and some docs on the overflow case, feel free to add: Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Regards, Boqun > I can mention this in the docs. > > Alice
On 10/15/24 8:33 PM, Boqun Feng wrote: > On Tue, Oct 15, 2024 at 02:28:28PM +0000, Alice Ryhl wrote: >> This is a useful for helper for working with indices into buffers that >> consist of several pages. I forgot to include it when I added PAGE_SIZE >> and PAGE_MASK for the same purpose in commit fc6e66f4696b ("rust: add >> abstraction for `struct page`"). >> >> Signed-off-by: Alice Ryhl <aliceryhl@google.com> >> --- >> rust/kernel/page.rs | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs >> index 208a006d587c..90846e3fe829 100644 >> --- a/rust/kernel/page.rs >> +++ b/rust/kernel/page.rs >> @@ -20,6 +20,11 @@ >> /// A bitmask that gives the page containing a given address. >> pub const PAGE_MASK: usize = !(PAGE_SIZE - 1); >> >> +/// Round up the given number to a multiple of `PAGE_SIZE`. >> +pub fn page_align(addr: usize) -> usize { > > Make it a `const` function? For example, there is a VMBUS_RING_SIZE() > macro in C that calculates a const number for a vmbus ringbuffer size, > so it will be useful. Good idea. >> + (addr + (PAGE_SIZE - 1)) & PAGE_MASK > > I guess overflows are unexpected, i.e. the users should not pass a > `addr` that `> usize::MAX - PAGE_SIZE + 1`? Correct. If this wraps around to zero, that's incorrect. Note that the minus one is in brackets to only trigger overflow detection in the right cases. I can mention this in the docs. Alice
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs index 208a006d587c..90846e3fe829 100644 --- a/rust/kernel/page.rs +++ b/rust/kernel/page.rs @@ -20,6 +20,11 @@ /// A bitmask that gives the page containing a given address. pub const PAGE_MASK: usize = !(PAGE_SIZE - 1); +/// Round up the given number to a multiple of `PAGE_SIZE`. +pub fn page_align(addr: usize) -> usize { + (addr + (PAGE_SIZE - 1)) & PAGE_MASK +} + /// A pointer to a page that owns the page allocation. /// /// # Invariants
This is a useful for helper for working with indices into buffers that consist of several pages. I forgot to include it when I added PAGE_SIZE and PAGE_MASK for the same purpose in commit fc6e66f4696b ("rust: add abstraction for `struct page`"). Signed-off-by: Alice Ryhl <aliceryhl@google.com> --- rust/kernel/page.rs | 5 +++++ 1 file changed, 5 insertions(+) --- base-commit: 8d8f785ceb21b9a0de11e05b811cc52d6fa79318 change-id: 20241015-page-align-7e5fa4c751be Best regards,