# Meeting 2023-06-28
<!-- Leave your topic starting with ### in the relevant sections below -->

## Critical
<!-- bugs, soundness issues, urgent patches/reviews etc. -->

## Status Reports
<!-- You want to report on something you are working on/request reviews.
Or you want to know the status of something someone else is doing -->

### Kangrejos

Miguel is looking at still organizing it around September 14-17.

## Discussion Questions
<!-- Anything that requires lengthy discussion/more general questions also fit here -->

### u32 overflows

In [this review](https://lore.kernel.org/rust-for-linux/udHI3v-OLUqHQt3fwnH71QuRJjzGxexw2rkIYEfnsChCmrLoJTIL_GL1wLCARf-UotY51jkPT6tC8nVDvjf8LkY2zvddpgeRQ5owysZwJos=@proton.me/) I noted at the end that "What if `data.len() > u32::MAX`?". What should be done in this case? Do we want to error? Or write `u32::MAX` bytes? Or call the C side repeatedly?

Consensus: Ask C maintainers what to do.

### `pin-init` chain functions

Lina [suggested](https://rust-for-linux.zulipchat.com/#narrow/stream/367382-Asahi-Meeting/topic/pin_init.20rebase.20and.20test/near/359664612) to have `chain` and `pin_chain` functions:

pub trait PinInit<T, E> {
    // ...
    
    fn pin_chain(self, f: impl FnOnce(Pin<&mut T>)
        -> Result<(), E>) -> impl PinInit<T, E>;
}

It first initializes the value and then calls the closure. I think it would be a good idea, since [others](https://lore.kernel.org/rust-for-linux/[e-mail]/) seem to require it as well. But for the function above to compile we need [`feature(return_position_impl_trait_in_trait)`](https://github.com/rust-lang/rust/issues/91611), do we want to add that? Otherwise it could be standalone functions, but they do not look as nice. Boqun [said](https://rust-for-linux.zulipchat.com/#narrow/stream/367382-Asahi-Meeting/topic/pin_init.20rebase.20and.20test/near/366369034) that it could get stabilized soon-ish.

Consensus: use concrete `PinChain` type.

### Make `Opaque` `!Unpin`?

In [this](https://lore.kernel.org/rust-for-linux/[e-mail]/) patch I saw someone forgetting to write

    #[pin]
    _pin: PhantomPinned,

But they had a

    #[pin]
    opaque: Opaque<bindings::..>,

So should we make `Opaque` `!Unpin`?

Consensus: Yes!

### [XArray](https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/xarray.20bindings)

#### Locking in `get`

Lina's [original patch](https://lore.kernel.org/rust-for-linux/[e-mail]/) takes xarray's internal `xa_lock`, and "users are expected to use an inner type that can be efficiently cloned (such as Arc<T>), and eagerly clone and drop the guard to unblock other users after a lookup."

- Should we instead have `pub fn get(&self) -> Option<&T> where T: Clone { ... }`? Whereby we would take the lock, clone the value, then release the lock and return the cloned value? This means that the caller does not have to worry about the `Guard`, and there is no risk of deadlock when you call `get` twice.
- Consensus: this one makes sense
- There could be another variant of `get` that does not require `Clone` and gives you access to the value for the duration of a closure, with `xa_lock` held during that closure.
- Consensus: this one makes sense
- There could be yet another variant `pub fn get_mut(&mut self) -> Option<&mut T>`, that would take and release the internal lock (due to how xarray is designed), but would not need the the clone, since the mutable reference should already be exclusive.
- Consensus: it's possible we should have a separate abstraction for users that want mutable access

#### Use in Binder (vs. RBTree)

We're interested in `XArray` because [RBTree is deprecated](https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Red.2Fblack.20tree.20deprecation), but there isn't a natural replacement for one proposed usage of `RBTree` [here](https://android-review.googlesource.com/c/kernel/common/+/2567935/14/drivers/android/range_alloc.rs#12).

We need to look up the smallest `T` equal to or greater than a given `size` (where `size` is a field in `T`). The problem is there could be multiple `T`'s of the same size, so we used a tuple `(size, offset)` as the key in an `RBTree`, since each `T` has a unique field `offset` (which, incidentally, is used as the key in another "map"). `RBTree` supported custom keys. Since `XArray` only supports integer keys, we need to do something more complicated, e.g. have it point at just one of the elements of that size, and have `prev_same_size` and `next_same_size` fields in the values. This gets rather complicated.

...thoughts?

- Consensus: we (matt g) will draft an email to the mailing list asking what to do if we can't use `RBTree`.

## Miscellaneous
<!-- stuff that does not fit into other categories -->

### rust: bindgen: upgrade to 0.65.1

@Miguel, do you plan to take it for v6.6?

Miguel: yeah (and the Rust 1.70.0 upgrade too).