| Message ID | 20241016-page-align-v2-1-e0afe85fc4b4@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] rust: page: add Rust version of PAGE_ALIGN | expand |
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
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,
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
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
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,
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
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
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,
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
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,
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
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,
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
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,
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
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(+) --- base-commit: 8d8f785ceb21b9a0de11e05b811cc52d6fa79318 change-id: 20241015-page-align-7e5fa4c751be Best regards,