[PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Alice Ryhl]

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`").

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Make the function const.
- Address integer overflow in docs.
- Link to v1: https://lore.kernel.org/r/20241015-page-align-v1-1-68fbd8b6d10c@google.com
---
 rust/kernel/page.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 208a006d587c..9ef01929e7d0 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -20,6 +20,15 @@
 /// A bitmask that gives the page containing a given address.
 pub const PAGE_MASK: usize = !(PAGE_SIZE - 1);
 
+/// Round up the given number to the next multiple of `PAGE_SIZE`.
+///
+/// It is incorrect to pass an address where the next multiple of `PAGE_SIZE` doesn't fit in a
+/// `usize`.
+pub const fn page_align(addr: usize) -> usize {
+    // Brackets around PAGE_SIZE-1 to avoid triggering overflow sanitizers in the wrong cases.
+    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
+}
+
 /// 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>

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Miguel Ojeda]

On Wed, Oct 16, 2024 at 1:34 PM Alice Ryhl <aliceryhl@google.com> 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`").
>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Applied to `rust-next` -- thanks everyone!

    [ Added intra-doc links, formatted comment. - Miguel ]

Cheers,
Miguel

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[John Hubbard]

On 10/16/24 4:34 AM, 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`").
> 
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Changes in v2:
> - Make the function const.
> - Address integer overflow in docs.
> - Link to v1: https://lore.kernel.org/r/20241015-page-align-v1-1-68fbd8b6d10c@google.com
> ---
>   rust/kernel/page.rs | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index 208a006d587c..9ef01929e7d0 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -20,6 +20,15 @@
>   /// A bitmask that gives the page containing a given address.
>   pub const PAGE_MASK: usize = !(PAGE_SIZE - 1);
>   
> +/// Round up the given number to the next multiple of `PAGE_SIZE`.
> +///
> +/// It is incorrect to pass an address where the next multiple of `PAGE_SIZE` doesn't fit in a
> +/// `usize`.
> +pub const fn page_align(addr: usize) -> usize {
> +    // Brackets around PAGE_SIZE-1 to avoid triggering overflow sanitizers in the wrong cases.

Silly nit, but I did start looking for brackets that aren't there, so:

s/Brackets/parentheses/


> +    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
> +}
> +
>   /// A pointer to a page that owns the page allocation.
>   ///
>   /// # Invariants
> 
> ---
> base-commit: 8d8f785ceb21b9a0de11e05b811cc52d6fa79318
> change-id: 20241015-page-align-7e5fa4c751be
> 
> Best regards,

thanks,
-- 
John Hubbard

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Miguel Ojeda]

On Mon, Oct 21, 2024 at 8:20 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> Silly nit, but I did start looking for brackets that aren't there, so:
>
> s/Brackets/parentheses/

I noticed that too, but I checked it and brackets is the BE spelling.
So I left it as it is...

Cheers,
Miguel

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[John Hubbard]

On 10/21/24 11:29 AM, Miguel Ojeda wrote:
> On Mon, Oct 21, 2024 at 8:20 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> Silly nit, but I did start looking for brackets that aren't there, so:
>>
>> s/Brackets/parentheses/
> 
> I noticed that too, but I checked it and brackets is the BE spelling.
> So I left it as it is...
> 

Is this another case of C and Rust using different words for things??
Wow. OK...

thanks,
-- 
John Hubbard

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Miguel Ojeda]

On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> Is this another case of C and Rust using different words for things??
> Wow. OK...

I am not sure what you mean -- by BE I meant British English.

See my other reply as well -- I just changed it anyway because Rust
apparently uses "parentheses".

Cheers,
Miguel

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[John Hubbard]

On 10/21/24 11:37 AM, Miguel Ojeda wrote:
> On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> Is this another case of C and Rust using different words for things??
>> Wow. OK...
> 
> I am not sure what you mean -- by BE I meant British English.
> 
> See my other reply as well -- I just changed it anyway because Rust
> apparently uses "parentheses".
> 

Right. For spoken languages, that's simply preference, and I would not
try to impose anything on anyone there.

But in this case, at least for C (and, from reading my Rust book(s), I
thought for Rust also), "parentheses" is a technical specification, and
we should prefer to be accurate:

     parentheses: ()
     brackets:    []

Yes?


thanks,
-- 
John Hubbard

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Alice Ryhl]

On 10/21/24 8:41 PM, John Hubbard wrote:
> On 10/21/24 11:37 AM, Miguel Ojeda wrote:
>> On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>>
>>> Is this another case of C and Rust using different words for things??
>>> Wow. OK...
>>
>> I am not sure what you mean -- by BE I meant British English.
>>
>> See my other reply as well -- I just changed it anyway because Rust
>> apparently uses "parentheses".
>>
> 
> Right. For spoken languages, that's simply preference, and I would not
> try to impose anything on anyone there.
> 
> But in this case, at least for C (and, from reading my Rust book(s), I
> thought for Rust also), "parentheses" is a technical specification, and
> we should prefer to be accurate:
> 
>      parentheses: ()
>      brackets:    []
> 
> Yes?
What word would you use to collectively talk about (), [], {}? In my 
native language they're all a kind of parenthesis.

Alice

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[John Hubbard]

On 10/21/24 11:59 AM, Alice Ryhl wrote:
> On 10/21/24 8:41 PM, John Hubbard wrote:
>> On 10/21/24 11:37 AM, Miguel Ojeda wrote:
>>> On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>>>
>>>> Is this another case of C and Rust using different words for things??
>>>> Wow. OK...
>>>
>>> I am not sure what you mean -- by BE I meant British English.
>>>
>>> See my other reply as well -- I just changed it anyway because Rust
>>> apparently uses "parentheses".
>>>
>>
>> Right. For spoken languages, that's simply preference, and I would not
>> try to impose anything on anyone there.
>>
>> But in this case, at least for C (and, from reading my Rust book(s), I
>> thought for Rust also), "parentheses" is a technical specification, and
>> we should prefer to be accurate:
>>
>>      parentheses: ()
>>      brackets:    []
>>
>> Yes?
> What word would you use to collectively talk about (), [], {}? In my native language they're all a kind of parenthesis.
> 

Good question. I've never attempted that when discussing programming
language details, because it hasn't come up, because it would be a
programming error in C to use one in place of the other. And it is
rare to refer to both cases in C.

Rust so far seems to have the same distinction, although I am standing
by to be corrected as necessary, there! :)

At a higher level of abstraction, though, perhaps "grouping" is a good
word.


thanks,
-- 
John Hubbard

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Alice Ryhl]

On Mon, Oct 21, 2024 at 9:09 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> On 10/21/24 11:59 AM, Alice Ryhl wrote:
> > On 10/21/24 8:41 PM, John Hubbard wrote:
> >> On 10/21/24 11:37 AM, Miguel Ojeda wrote:
> >>> On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> wrote:
> >>>>
> >>>> Is this another case of C and Rust using different words for things??
> >>>> Wow. OK...
> >>>
> >>> I am not sure what you mean -- by BE I meant British English.
> >>>
> >>> See my other reply as well -- I just changed it anyway because Rust
> >>> apparently uses "parentheses".
> >>>
> >>
> >> Right. For spoken languages, that's simply preference, and I would not
> >> try to impose anything on anyone there.
> >>
> >> But in this case, at least for C (and, from reading my Rust book(s), I
> >> thought for Rust also), "parentheses" is a technical specification, and
> >> we should prefer to be accurate:
> >>
> >>      parentheses: ()
> >>      brackets:    []
> >>
> >> Yes?
> > What word would you use to collectively talk about (), [], {}? In my native language they're all a kind of parenthesis.
> >
>
> Good question. I've never attempted that when discussing programming
> language details, because it hasn't come up, because it would be a
> programming error in C to use one in place of the other. And it is
> rare to refer to both cases in C.
>
> Rust so far seems to have the same distinction, although I am standing
> by to be corrected as necessary, there! :)
>
> At a higher level of abstraction, though, perhaps "grouping" is a good
> word.

Rust macros can use different types of brackets. For example, the
`assert!(1 < 2)` macro uses round parenthesises, the `vec![1,2,3]`
macro uses square parenthesises, and the `thread_local! { ... }` macro
uses curly parenthesies. The round and square brackets are used for
expression-like things, and the curlies are used for things that
expand to top-level items such as global variables or functions.

Macros cannot use any other delimiter than those three. So e.g. <>
wouldn't work.

Alice

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[John Hubbard]

On 10/21/24 12:26 PM, Alice Ryhl wrote:
> On Mon, Oct 21, 2024 at 9:09 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> On 10/21/24 11:59 AM, Alice Ryhl wrote:
>>> On 10/21/24 8:41 PM, John Hubbard wrote:
>>>> On 10/21/24 11:37 AM, Miguel Ojeda wrote:
>>>>> On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>>>>>
>>>>>> Is this another case of C and Rust using different words for things??
>>>>>> Wow. OK...
>>>>>
>>>>> I am not sure what you mean -- by BE I meant British English.
>>>>>
>>>>> See my other reply as well -- I just changed it anyway because Rust
>>>>> apparently uses "parentheses".
>>>>>
>>>>
>>>> Right. For spoken languages, that's simply preference, and I would not
>>>> try to impose anything on anyone there.
>>>>
>>>> But in this case, at least for C (and, from reading my Rust book(s), I
>>>> thought for Rust also), "parentheses" is a technical specification, and
>>>> we should prefer to be accurate:
>>>>
>>>>       parentheses: ()
>>>>       brackets:    []
>>>>
>>>> Yes?
>>> What word would you use to collectively talk about (), [], {}? In my native language they're all a kind of parenthesis.
>>>
>>
>> Good question. I've never attempted that when discussing programming
>> language details, because it hasn't come up, because it would be a
>> programming error in C to use one in place of the other. And it is
>> rare to refer to both cases in C.
>>
>> Rust so far seems to have the same distinction, although I am standing
>> by to be corrected as necessary, there! :)
>>
>> At a higher level of abstraction, though, perhaps "grouping" is a good
>> word.
> 
> Rust macros can use different types of brackets. For example, the
> `assert!(1 < 2)` macro uses round parenthesises, the `vec![1,2,3]`
> macro uses square parenthesises, and the `thread_local! { ... }` macro
> uses curly parenthesies. The round and square brackets are used for
> expression-like things, and the curlies are used for things that
> expand to top-level items such as global variables or functions.
> 
> Macros cannot use any other delimiter than those three. So e.g. <>
> wouldn't work.

That answers my implicit "are there any cases in which you would
want to collectively refer to all three types of...bracket?", yes.

For the original point, though, we are not in a Rust macro. Is it
actually allowable to use [] or {} here:

+    // Brackets around PAGE_SIZE-1 to avoid triggering overflow sanitizers in the wrong cases.
+    (addr + (PAGE_SIZE - 1)) & PAGE_MASK

? Is that why you were not seeing a difference between saying "brackets"
vs. "parentheses" there? If so, this would be yet another case of my
Rust newbie-ness being inflicted on you. :)


thanks,
-- 
John Hubbard

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Alice Ryhl]

On 10/21/24 9:34 PM, John Hubbard wrote:
> On 10/21/24 12:26 PM, Alice Ryhl wrote:
>> On Mon, Oct 21, 2024 at 9:09 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>>
>>> On 10/21/24 11:59 AM, Alice Ryhl wrote:
>>>> On 10/21/24 8:41 PM, John Hubbard wrote:
>>>>> On 10/21/24 11:37 AM, Miguel Ojeda wrote:
>>>>>> On Mon, Oct 21, 2024 at 8:35 PM John Hubbard <jhubbard@nvidia.com> 
>>>>>> wrote:
>>>>>>>
>>>>>>> Is this another case of C and Rust using different words for 
>>>>>>> things??
>>>>>>> Wow. OK...
>>>>>>
>>>>>> I am not sure what you mean -- by BE I meant British English.
>>>>>>
>>>>>> See my other reply as well -- I just changed it anyway because Rust
>>>>>> apparently uses "parentheses".
>>>>>>
>>>>>
>>>>> Right. For spoken languages, that's simply preference, and I would not
>>>>> try to impose anything on anyone there.
>>>>>
>>>>> But in this case, at least for C (and, from reading my Rust book(s), I
>>>>> thought for Rust also), "parentheses" is a technical specification, 
>>>>> and
>>>>> we should prefer to be accurate:
>>>>>
>>>>>       parentheses: ()
>>>>>       brackets:    []
>>>>>
>>>>> Yes?
>>>> What word would you use to collectively talk about (), [], {}? In my 
>>>> native language they're all a kind of parenthesis.
>>>>
>>>
>>> Good question. I've never attempted that when discussing programming
>>> language details, because it hasn't come up, because it would be a
>>> programming error in C to use one in place of the other. And it is
>>> rare to refer to both cases in C.
>>>
>>> Rust so far seems to have the same distinction, although I am standing
>>> by to be corrected as necessary, there! :)
>>>
>>> At a higher level of abstraction, though, perhaps "grouping" is a good
>>> word.
>>
>> Rust macros can use different types of brackets. For example, the
>> `assert!(1 < 2)` macro uses round parenthesises, the `vec![1,2,3]`
>> macro uses square parenthesises, and the `thread_local! { ... }` macro
>> uses curly parenthesies. The round and square brackets are used for
>> expression-like things, and the curlies are used for things that
>> expand to top-level items such as global variables or functions.
>>
>> Macros cannot use any other delimiter than those three. So e.g. <>
>> wouldn't work.
> 
> That answers my implicit "are there any cases in which you would
> want to collectively refer to all three types of...bracket?", yes.
> 
> For the original point, though, we are not in a Rust macro. Is it
> actually allowable to use [] or {} here:
> 
> +    // Brackets around PAGE_SIZE-1 to avoid triggering overflow 
> sanitizers in the wrong cases.
> +    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
> 
> ? Is that why you were not seeing a difference between saying "brackets"
> vs. "parentheses" there? If so, this would be yet another case of my
> Rust newbie-ness being inflicted on you. :)
You can use both () and {}, but you can only use brackets if you're 
European. ;)

Using {} to create a block works because a block evaluates to the value 
of the last expression in the block. It would be super weird to define a 
block here, though.

Alice

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[John Hubbard]

On 10/21/24 12:49 PM, Alice Ryhl wrote:
> On 10/21/24 9:34 PM, John Hubbard wrote:
>> On 10/21/24 12:26 PM, Alice Ryhl wrote:
>>> On Mon, Oct 21, 2024 at 9:09 PM John Hubbard <jhubbard@nvidia.com> wrote:
...
>>> Rust macros can use different types of brackets. For example, the
>>> `assert!(1 < 2)` macro uses round parenthesises, the `vec![1,2,3]`
>>> macro uses square parenthesises, and the `thread_local! { ... }` macro
>>> uses curly parenthesies. The round and square brackets are used for
>>> expression-like things, and the curlies are used for things that
>>> expand to top-level items such as global variables or functions.
>>>
>>> Macros cannot use any other delimiter than those three. So e.g. <>
>>> wouldn't work.
>>
>> That answers my implicit "are there any cases in which you would
>> want to collectively refer to all three types of...bracket?", yes.
>>
>> For the original point, though, we are not in a Rust macro. Is it
>> actually allowable to use [] or {} here:
>>
>> +    // Brackets around PAGE_SIZE-1 to avoid triggering overflow sanitizers in the wrong cases.
>> +    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
>>
>> ? Is that why you were not seeing a difference between saying "brackets"
>> vs. "parentheses" there? If so, this would be yet another case of my
>> Rust newbie-ness being inflicted on you. :)
> You can use both () and {}, but you can only use brackets if you're European. ;)

I *knew* I was missing out! hahaha

> 
> Using {} to create a block works because a block evaluates to the value of the last expression in the block. It would be super weird to define a block here, though.
> 

No argument there. :)

thanks,
-- 
John Hubbard

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Alice Ryhl]

On 10/21/24 8:20 PM, John Hubbard wrote:
> On 10/16/24 4:34 AM, 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`").
>>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>> ---
>> Changes in v2:
>> - Make the function const.
>> - Address integer overflow in docs.
>> - Link to v1: https://lore.kernel.org/r/20241015-page-align- 
>> v1-1-68fbd8b6d10c@google.com
>> ---
>>   rust/kernel/page.rs | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
>> index 208a006d587c..9ef01929e7d0 100644
>> --- a/rust/kernel/page.rs
>> +++ b/rust/kernel/page.rs
>> @@ -20,6 +20,15 @@
>>   /// A bitmask that gives the page containing a given address.
>>   pub const PAGE_MASK: usize = !(PAGE_SIZE - 1);
>> +/// Round up the given number to the next multiple of `PAGE_SIZE`.
>> +///
>> +/// It is incorrect to pass an address where the next multiple of 
>> `PAGE_SIZE` doesn't fit in a
>> +/// `usize`.
>> +pub const fn page_align(addr: usize) -> usize {
>> +    // Brackets around PAGE_SIZE-1 to avoid triggering overflow 
>> sanitizers in the wrong cases.
> 
> Silly nit, but I did start looking for brackets that aren't there, so:
> 
> s/Brackets/parentheses/
This isn't a distinction that exists in my vocabulary, shrug.

Miguel, feel free to reword when you pick this.

Alice

Re: [PATCH v2] rust: page: add Rust version of PAGE_ALIGN

[Miguel Ojeda]

On Mon, Oct 21, 2024 at 8:29 PM Alice Ryhl <alice@ryhl.io> wrote:
>
> This isn't a distinction that exists in my vocabulary, shrug.
>
> Miguel, feel free to reword when you pick this.

I just rebased -- the Rust reference uses "parentheses", so I guess it
is fair to use that.

    https://doc.rust-lang.org/reference/expressions/grouped-expr.html

Cheers,
Miguel