# Meeting 2023-05-03
<!-- 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 -->

### `klint` is updated to Rust 1.68.2

### `subclass` Meeting

Benno: There was a meeting on subclasses scheduled: <https://hackmd.io/W_kJ1kRsS3mX8vpk5Cpbzg> what were the results? I read the doc and I agree with Gary in that `FenceObject<T>` is a smart pointer to a `T`. Alternatively we could do this:

pub struct FenceObject<'a, T>(&'a FenceObjInner<T>);
struct FenceObjInner<T> {
    dma_fence: Opaque<bindings::dma_fence>,
    inner: T,
}
impl<'a, T> Deref<Target = T> for FenceObject<'a, T> { .. }
impl<'a, T> Receiver for FenceObject<'a, T> { .. }
impl<'a, T> FenceObject<'a, T> {
    // normal fence ops here
}

since then `FenceObject` truly is a smart pointer. But I do not see the point in that.

Boqun: we didn't decide anything in that meeting, it was just exploring different options. However, one thing I don't really like is using `Deref`, which ends up with `impl Receiver`. In other words, I don't see *much* improvement that we replace `.inner()` with `.deref()`, maybe we can rename `.inner()` with `.ext()` or something.

Boqun: read it, the difference is `myfence.foo()` vs `MyFence::foo(myfence)` where `myfence: FenceObject<MyFence>`, but same argument here.

Benno: For the is-a relationship we can use this pattern. For the has-a relation we could use a macro similar to <https://docs.rs/delegate/latest/delegate/>.

Gary: For two C objects that have a is-a relationship we cannot use this pattern.

Boqun: With timer we want to drop the timer object first which we cannot do with this pattern.

struct Foo {
    a: Box<i32>,
    timer: Timer, // timer_list

}

//
unsafe trait Timeout {
    
}

impl Timeout for Foo {
    fn timeout(ptr: *const timer_list) {
        let f: &Foo = container_of!(...);
        
        println!("{}", f.a); //
    }
}

{
    let f: Foo = ..;
    f.timer.schedule(); // on a global list
    
    drop(f); // delist from the global list first
    // drop Foo
    //   timeout() called here. -- interrupt
    //   drop Timer
}

Boqun:
https://doc.rust-lang.org/beta/core/marker/trait.Send.html vs interrupt handlers

Gary: Interrupt handlers in kernel are similar to signals in userspace threads. It's safe (but maybe too conservative) to treat interrupt handlers as a separate thread.

### Kangrejos update?

Miguel: The date is not yet fixed. I would like to announce it as soon as possible.

### RPITIT!
(return position impl trait in trait)

<https://github.com/rust-lang/rfcs/pull/3425> is getting merged!

All: we are all excited.

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

#[cfg(attendee = "Wedson")]
### Lock guard type split for IRQ save

Gary has a prototype here: https://github.com/nbdd0121/linux/commit/01ae567f6c192bdef2e6e5997769ee2314ff78cd

Postponed, waiting for Wedson.

### `pin-init` fixes and review

Just pinging the reviewers, the patch for fixing `Self` in `pin_init!` constructors is [in the list](https://lore.kernel.org/rust-for-linux/[e-mail]/#r), but could use some more reviews.

@ojeda Lina has given me some feedback and I would like to improve some things:
- Make the guards in `[try_][pin_]init!` hygienic.
- Add a derive macro for `Zeroable`.
- Add `..Zeroable::zeroed()` syntax at the end of the struct initializer to make all fields not mentioned zero.

Can these things be considered 'fixes' and still put into 6.4 or how is the timing on that?

Miguel: I have thought about this. Are these really fixes?

Benno: No not really they are improvements.

Consensus: Put them in `next` normally (unless Lina really needs it for a reason).

### Field projections

There was some feedback on the RFC, see [here](https://github.com/rust-lang/rfcs/pull/3318#issuecomment-1529146661). I think we should go ahead and do some more experimentation with solely macro based projections. @nbdd0121 already has an implementation with `syn`. The question is, do we feel that such a proc macro could be placed in the kernel? We could then move forward and experiment with `Rcu`, `Seqlock` and `Pin` projections. I have convinced myself that the hash approach is fine.

Very unstable:

struct Field<const NAME: &'static str> {}

instead of that Gary's solution does

struct Field<const NAME: usize> {}

Benno: Jules had a declarative macro approach: <https://github.com/rust-lang/rfcs/pull/3318#issuecomment-1529144319>

Benno: Doing experimentation is going to be good to make RFC process.

Consensus: Start experimenting and if it is useful, then n n

### Cache line padding for locks
https://lore.kernel.org/rust-for-linux/[e-mail]

The kernel `struct spinlock` is 4 bytes on x86 when lockdep is not enabled. The
structure is not padded to fit a cache line. The effect of this for `SpinLock`
is that the lock variable and the value protected by the lock will share a cache
line, depending on the alignment requirements of the protected value. Aligning
the lock variable and the protected value to a cache line yields a 20%
performance increase for the Rust null block driver for sequential reads to
memory backed devices at 6 concurrent readers.

Andreas: All threads try to get exculsive access to the cacheline of the lock. We need to pad the lock to move the value of a lock to a different cacheline.

Alice: Cant we just have the alignment on the value?

Andreas: No that is not sufficient, since Rust might reorder the fields.

Alice: This problem also exists in userspace rust.

Gary: If the value has alignment 64 it works.

Alice: The solution I suggested on the list should work, it also is nice, since it allows you to opt-in.

Andreas: I need to try it.

### Rust Binder

Alice has started submitting patches with various dependencies of binder.

**Question:** How do we approach the filesystem bindings? We will need to coordinate who will upstream which pieces.

Binder still needs these files:

* `io_buffer.rs`
* `user_ptr.rs`
* `cred.rs`
* `iov_iter.rs`
* `pages.rs` (included in the null block driver patchset)
* `mm.rs`
* `file.rs`
* `device.rs`
* `miscdev.rs`
* `rbtree.rs`
* `linked_list.rs`
* `workqueue.rs`
* `task_work.rs`

These have non-trivial dependencies on each other, see [here](https://github.com/Rust-for-Linux/linux/issues/1004).

Alice: I need Wedson, since I am blocked on some of his patches.

Andreas: I sent some of Wedsons patches with my last series (of course crediting him).

Miguel: It is fine for an RFC, since that is not intended to be merged.

Alice: Main topic: binder has a lot of big dependencies, I would like to have coordination on how to upstream these.

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

### Should we start to annotate klint for every code?

Miguel: Should we introduce this to the kernel?

Boqun: I would like `klint` to run like `clippy`.

Gary: You have to add `klint` as a tool.

Miguel: Do we want to have `KLINT=1` similar to `CLIPPY=1`?

Consensus: Yes

Boqun: I want it to work ASAP.

Gary: If you go to this [PR](https://github.com/Rust-for-Linux/linux/pull/958/files) then you see how to register it.

Boqun: If I compile it without klint, it errors.

Miguel: I can try to integrate it.

Benno: Do we want to upstream it?

Miguel: That was my original question.

Gary: The blog post is out, it is in the `Rust-for-Linux` repo, so I think we can do it. I have concerns about unstable features.

Miguel: That is not an issue. Even if it is the last one we can think about it.

Benno: We could also `cfg` them away.

Miguel: Gary, if you want to send it, then you can do it. Or if you want me to do the integration then you can introduce it first?

Gary: Yeah, or I can send the patch with only the attributes patch and Miguel then can send the KLINT=1 command line patch

Gary: You also do not have to add attributes, as all ffi function are annotated. You can still do assertions though.

Benno: Gary, did you find a solution for `try_lock`.

Gary: I did not really have the time for that. I have some ideas, but still need to work on that.

### Adding Linux to Rust CI / crater

(Discussion)

### Null block driver upstreaming

Discussion about whether the no-duplicate-driver policy would prevent Rust driver upstreaming or requiring the C driver to be removed.

Consensus: Rust is currently experimental so probably that's okay.