Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbtree v4 #1081

Draft
wants to merge 7 commits into
base: rust-next
Choose a base branch
from

Conversation

matthewtgilbride
Copy link

Responding to Benno's comments here: https://lore.kernel.org/all/[email protected]/

rust/kernel/alloc/box_ext.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v5:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 5,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ]
    }
  }
}
Copy link
Member

@y86-dev y86-dev left a comment

Choose a reason for hiding this comment

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

I went through the patches, I hope I found everything, but when you send it to the list, I might find some more things. let me know if/when I should take another look

rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Outdated Show resolved Hide resolved
rust/kernel/rbtree.rs Outdated Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
@@ -436,21 +455,76 @@ unsafe impl<'a, K: Sync, V: Sync> Sync for Iter<'a, K, V> {}
impl<'a, K, V> Iterator for Iter<'a, K, V> {
type Item = (&'a K, &'a V);

fn next(&mut self) -> Option<Self::Item> {
self.iter_raw.next().map(|(k, v)|
// SAFETY: Due to `&mut self`, we have exclusive access to `k` and `v`, for the lifetime of `'a`.
Copy link
Member

Choose a reason for hiding this comment

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

This seems wrong, since the Iter only borrows the tree immutably.

rust/kernel/rbtree.rs Show resolved Hide resolved
rust/kernel/rbtree.rs Outdated Show resolved Hide resolved
rust/kernel/rbtree.rs Show resolved Hide resolved
Benno Lossin and others added 6 commits June 3, 2024 15:22
Sometimes (see [1]) it is necessary to drop the value inside of a
`Box<T>`, but retain the allocation. For example to reuse the allocation
in the future.
Introduce a new function `drop_contents` that turns a `Box<T>` into
`Box<MaybeUninit<T>>` by dropping the value.

Signed-off-by: Benno Lossin <[email protected]>
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
The rust rbtree exposes a map-like interface over keys and values,
backed by the kernel red-black tree implementation. Values can be
inserted, deleted, and retrieved from a `RBTree` by key.

This base abstraction is used by binder to store key/value
pairs and perform lookups, for example the patch
"[PATCH RFC 03/20] rust_binder: add threading support"
in the binder RFC [1].

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Signed-off-by: Matt Gilbride <[email protected]>
- Add Iterator implementation for `RBTree`, allowing
  iteration over (key, value) pairs in key order.
- Add individual `keys()` and `values()` functions to iterate over keys
  or values alone.
- Update doctests to use iteration instead of explicitly getting items.

Iteration is needed by the binder driver to enumerate all values in a
tree for oneway spam detection [1].

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Signed-off-by: Matt Gilbride <[email protected]>
Add mutable Iterator implementation for `RBTree`,
allowing iteration over (key, value) pairs in key order. Only values are
mutable, as mutating keys implies modifying a node's position in the tree.

Mutable iteration is used by the binder driver during shutdown to
clean up the tree maintained by the "range allocator" [1].

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Signed-off-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Matt Gilbride <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Add a cursor interface to `RBTree`, supporting the following use cases:
- Inspect the current node pointed to by the cursor, inspect/move to
  it's neighbors in sort order (bidirectionally).
- Mutate the tree itself by removing the current node pointed to by the
  cursor, or one of its neighbors.

Add functions to obtain a cursor to the tree by key:
- The node with the smallest key
- The node with the largest key
- The node matching the given key, or the one with the next larger key

The cursor abstraction is needed by the binder driver to efficiently
search for nodes and (conditionally) modify them, as well as their
neighbors [1].

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Co-developed-by: Alice Ryhl <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Signed-off-by: Matt Gilbride <[email protected]>
This mirrors the entry API [1] from the Rust standard library on
`RBTree`. This API can be used to access the entry at a specific key and
make modifications depending on whether the key is vacant or occupied.
This API is useful because it can often be used to avoid traversing the
tree multiple times.

This is used by binder to look up and conditionally access or insert a
value, depending on whether it is there or not [2].

Link: https://doc.rust-lang.org/stable/std/collections/btree_map/enum.Entry.html [1]
Link: https://android-review.googlesource.com/c/kernel/common/+/2849906 [2]
Signed-off-by: Alice Ryhl <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Signed-off-by: Matt Gilbride <[email protected]>
@matthewtgilbride matthewtgilbride force-pushed the b4/b4-rbtree-v4 branch 2 times, most recently from 668ba7d to 7ef6a96 Compare June 5, 2024 19:25
matthewtgilbride added a commit to matthewtgilbride/linux that referenced this pull request Jul 9, 2024
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v6:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Used `Box::write` in `RBTreeNodeReservation::into_node`, removing
  unnecessary `unsafe` blocks.
- Updated `RBTreeCursor::remove_current` to return the removed node.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 6,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ],
      "v5": [
        "[email protected]"
      ]
    }
  }
}
matthewtgilbride added a commit to matthewtgilbride/linux that referenced this pull request Aug 14, 2024
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v9:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v8: https://lore.kernel.org/r/[email protected]

Changes in v8:
- Fix small style nit (use ? operator)
- Fix doc comment pointing at a private item
- Link to v7: https://lore.kernel.org/r/[email protected]

Changes in v7:
- make `RawVacantEntry.rbtree` a raw pointer like
  `RawVacantEntry.child_field_of_parent`, since the latter can
  technically point at a field of the former. We prefer that the
  implementation be explicit about the safety guarantees of both because
  of the relationship between them.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Minimize usage of `*mut bindings::rb_node`, replacing with
  `NonNull<bindings::rb_node>`. Specifically, changing
  `RBTreeCursor.current` to be `NonNull<bindings::rb_node>` and updating
  the corresponding functions.
- Update `RBTreeCursor:to_key_value` helpers to have their own lifetime
  (they are not instance methods, using a different lifetime than that
  of the `impl` block they are in makes things more clear.
- Fix misplaced semicolon in `cursor_lower_bound`.
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Used `Box::write` in `RBTreeNodeReservation::into_node`, removing
  unnecessary `unsafe` blocks.
- Updated `RBTreeCursor::remove_current` to return the removed node.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 9,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ],
      "v5": [
        "[email protected]"
      ],
      "v6": [
        "[email protected]"
      ],
      "v7": [
        "[email protected]"
      ],
      "v8": [
        "[email protected]"
      ]
    }
  }
}
matthewtgilbride added a commit to matthewtgilbride/linux that referenced this pull request Aug 18, 2024
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v10:
- Rebase on top of the following:
  - Linux 6.11-rc3
  - helpers.c refactor
  - BoxExt::drop_contents
- Link to v9: https://lore.kernel.org/r/[email protected]

Changes in v9:
- Rebase on top of Linux 6.11-rc2
- Address feedback from v8
- Link to v8: https://lore.kernel.org/r/[email protected]

Changes in v8:
- Fix small style nit (use ? operator)
- Fix doc comment pointing at a private item
- Link to v7: https://lore.kernel.org/r/[email protected]

Changes in v7:
- make `RawVacantEntry.rbtree` a raw pointer like
  `RawVacantEntry.child_field_of_parent`, since the latter can
  technically point at a field of the former. We prefer that the
  implementation be explicit about the safety guarantees of both because
  of the relationship between them.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Minimize usage of `*mut bindings::rb_node`, replacing with
  `NonNull<bindings::rb_node>`. Specifically, changing
  `RBTreeCursor.current` to be `NonNull<bindings::rb_node>` and updating
  the corresponding functions.
- Update `RBTreeCursor:to_key_value` helpers to have their own lifetime
  (they are not instance methods, using a different lifetime than that
  of the `impl` block they are in makes things more clear.
- Fix misplaced semicolon in `cursor_lower_bound`.
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Used `Box::write` in `RBTreeNodeReservation::into_node`, removing
  unnecessary `unsafe` blocks.
- Updated `RBTreeCursor::remove_current` to return the removed node.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 10,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ],
      "v5": [
        "[email protected]"
      ],
      "v6": [
        "[email protected]"
      ],
      "v7": [
        "[email protected]"
      ],
      "v8": [
        "[email protected]"
      ],
      "v9": [
        "[email protected]"
      ]
    }
  }
}
matthewtgilbride added a commit to matthewtgilbride/linux that referenced this pull request Aug 18, 2024
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v10:
- Rebase on top of the following:
  - Linux 6.11-rc3
  - https://lore.kernel.org/rust-for-linux/[email protected]/
  - https://lore.kernel.org/all/CAH5fLgj2urZ6OD2ki6E=6EuPqW3Z8BGg8jd6Bgo4OOrNiptnDw@mail.gmail.com/
- Link to v9: https://lore.kernel.org/r/[email protected]

Changes in v9:
- Rebase on top of Linux 6.11-rc2
- Address feedback from v8
- Link to v8: https://lore.kernel.org/r/[email protected]

Changes in v8:
- Fix small style nit (use ? operator)
- Fix doc comment pointing at a private item
- Link to v7: https://lore.kernel.org/r/[email protected]

Changes in v7:
- make `RawVacantEntry.rbtree` a raw pointer like
  `RawVacantEntry.child_field_of_parent`, since the latter can
  technically point at a field of the former. We prefer that the
  implementation be explicit about the safety guarantees of both because
  of the relationship between them.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Minimize usage of `*mut bindings::rb_node`, replacing with
  `NonNull<bindings::rb_node>`. Specifically, changing
  `RBTreeCursor.current` to be `NonNull<bindings::rb_node>` and updating
  the corresponding functions.
- Update `RBTreeCursor:to_key_value` helpers to have their own lifetime
  (they are not instance methods, using a different lifetime than that
  of the `impl` block they are in makes things more clear.
- Fix misplaced semicolon in `cursor_lower_bound`.
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Used `Box::write` in `RBTreeNodeReservation::into_node`, removing
  unnecessary `unsafe` blocks.
- Updated `RBTreeCursor::remove_current` to return the removed node.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 10,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ],
      "v5": [
        "[email protected]"
      ],
      "v6": [
        "[email protected]"
      ],
      "v7": [
        "[email protected]"
      ],
      "v8": [
        "[email protected]"
      ],
      "v9": [
        "[email protected]"
      ]
    }
  }
}
matthewtgilbride added a commit to matthewtgilbride/linux that referenced this pull request Aug 19, 2024
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v10:
- Rebase on top of rust-for-linux/rust-next (e26fa54)
  - Adopt to changes from "rust: kbuild: split up helpers.c"
    (8763465)
- Rebase on top of "rust: kernel: add `drop_contents` to `BoxExt`" instead of including it in the patch series
  - https://lore.kernel.org/all/CAH5fLgj2urZ6OD2ki6E=6EuPqW3Z8BGg8jd6Bgo4OOrNiptnDw@mail.gmail.com/
- Link to v9: https://lore.kernel.org/r/[email protected]

Changes in v9:
- Rebase on top of Linux 6.11-rc2
- Address feedback from v8
- Link to v8: https://lore.kernel.org/r/[email protected]

Changes in v8:
- Fix small style nit (use ? operator)
- Fix doc comment pointing at a private item
- Link to v7: https://lore.kernel.org/r/[email protected]

Changes in v7:
- make `RawVacantEntry.rbtree` a raw pointer like
  `RawVacantEntry.child_field_of_parent`, since the latter can
  technically point at a field of the former. We prefer that the
  implementation be explicit about the safety guarantees of both because
  of the relationship between them.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Minimize usage of `*mut bindings::rb_node`, replacing with
  `NonNull<bindings::rb_node>`. Specifically, changing
  `RBTreeCursor.current` to be `NonNull<bindings::rb_node>` and updating
  the corresponding functions.
- Update `RBTreeCursor:to_key_value` helpers to have their own lifetime
  (they are not instance methods, using a different lifetime than that
  of the `impl` block they are in makes things more clear.
- Fix misplaced semicolon in `cursor_lower_bound`.
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Used `Box::write` in `RBTreeNodeReservation::into_node`, removing
  unnecessary `unsafe` blocks.
- Updated `RBTreeCursor::remove_current` to return the removed node.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 10,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ],
      "v5": [
        "[email protected]"
      ],
      "v6": [
        "[email protected]"
      ],
      "v7": [
        "[email protected]"
      ],
      "v8": [
        "[email protected]"
      ],
      "v9": [
        "[email protected]"
      ]
    }
  }
}
matthewtgilbride added a commit to matthewtgilbride/linux that referenced this pull request Aug 19, 2024
This patchset contains the red-black tree abstractions needed by the Rust
implementation of the Binder driver.

Binder driver benefits from O(log n) search/insertion/deletion of
key/value mappings in various places, including `process.rs` and
`range_alloc.rs`.  In `range_alloc.rs`, the ability to store and
search by a generic key type is also useful.

Please see the Rust Binder RFC for usage examples [1]. Note that
the `container_of` macro is currently used only by `rbtree` itself.

Users of "rust: rbtree: add red-black tree implementation backed by the C version"
    [PATCH RFC 03/20] rust_binder: add threading support
    [PATCH RFC 05/20] rust_binder: add nodes and context managers
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add iterator"
    [PATCH RFC 17/20] rust_binder: add oneway spam detection

Users of "rust: rbtree: add mutable iterator"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add `RBTreeCursor`"
    [PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: rbtree: add RBTree::entry"
    Not used in the original RFC, but introduced after further
    code review.  See: https://r.android.com/2849906

The Rust Binder RFC addresses the upstream deprecation of red-black
tree. Quoted here for convenience:

"This RFC uses the kernel's red-black tree for key/value mappings, but we
are aware that the red-black tree is deprecated. We did this to make the
performance comparison more fair, since C binder also uses rbtree for
this. We intend to replace these with XArrays instead. That said, we
don't think that XArray is a good fit for the range allocator, and we
propose to continue using the red-black tree for the range allocator."

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
To: Miguel Ojeda <[email protected]>
To: Alex Gaynor <[email protected]>
To: Wedson Almeida Filho <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Andreas Hindborg <[email protected]>
To: Alice Ryhl <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arve Hjønnevåg <[email protected]>
To: Todd Kjos <[email protected]>
To: Martijn Coenen <[email protected]>
To: Joel Fernandes <[email protected]>
To: Christian Brauner <[email protected]>
To: Carlos Llamas <[email protected]>
To: Suren Baghdasaryan <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Michel Lespinasse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Matt Gilbride <[email protected]>

---
Changes in v10:
- Rebase on top of rust-for-linux/rust-next (e26fa54)
  - Adapt to changes from "rust: kbuild: split up helpers.c"
    (8763465)
- Rebase on top of "rust: kernel: add `drop_contents` to `BoxExt`" instead of including it in the patch series
  - https://lore.kernel.org/all/CAH5fLgj2urZ6OD2ki6E=6EuPqW3Z8BGg8jd6Bgo4OOrNiptnDw@mail.gmail.com/
- Link to v9: https://lore.kernel.org/r/[email protected]

Changes in v9:
- Rebase on top of Linux 6.11-rc2
- Address feedback from v8
- Link to v8: https://lore.kernel.org/r/[email protected]

Changes in v8:
- Fix small style nit (use ? operator)
- Fix doc comment pointing at a private item
- Link to v7: https://lore.kernel.org/r/[email protected]

Changes in v7:
- make `RawVacantEntry.rbtree` a raw pointer like
  `RawVacantEntry.child_field_of_parent`, since the latter can
  technically point at a field of the former. We prefer that the
  implementation be explicit about the safety guarantees of both because
  of the relationship between them.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Minimize usage of `*mut bindings::rb_node`, replacing with
  `NonNull<bindings::rb_node>`. Specifically, changing
  `RBTreeCursor.current` to be `NonNull<bindings::rb_node>` and updating
  the corresponding functions.
- Update `RBTreeCursor:to_key_value` helpers to have their own lifetime
  (they are not instance methods, using a different lifetime than that
  of the `impl` block they are in makes things more clear.
- Fix misplaced semicolon in `cursor_lower_bound`.
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Used `Box::write` in `RBTreeNodeReservation::into_node`, removing
  unnecessary `unsafe` blocks.
- Updated `RBTreeCursor::remove_current` to return the removed node.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- rebased onto the tip of rust-for-linux/rust-next (97ab3e8)
- addressed comments from draft PR on GitHub: Rust-for-Linux#1081
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Address various feedback re: SAFETY and INVARIANT comments from v2.
- Update variable naming and add detailed comments for the `RBTree::insert` (later moved to
  `RBTree::raw_entry`) implementation.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Update documentation link to the C header file
- Use `core::convert::Infallible` in try_reserve_node
- Link to v1: https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 10,
    "change-id": "20231205-b4-rbtree-abb1a016f0a0",
    "prefixes": [],
    "history": {
      "v1": [
        "[email protected]"
      ],
      "v2": [
        "[email protected]"
      ],
      "v3": [
        "[email protected]"
      ],
      "v4": [
        "[email protected]"
      ],
      "v5": [
        "[email protected]"
      ],
      "v6": [
        "[email protected]"
      ],
      "v7": [
        "[email protected]"
      ],
      "v8": [
        "[email protected]"
      ],
      "v9": [
        "[email protected]"
      ]
    }
  }
}
@ojeda ojeda force-pushed the rust-next branch 3 times, most recently from c9b5ce6 to ce1c54f Compare October 9, 2024 22:37
@ojeda ojeda force-pushed the rust-next branch 3 times, most recently from 9ee7197 to 6ce162a Compare October 15, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants