# Meeting 2023-12-13
<!-- 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 -->
### patchable-function-entry (needed for `ftrace`)
* [MCP](https://github.com/rust-lang/compiler-team/issues/704) (Just the flag)
* [RFC](https://github.com/rust-lang/rfcs/pull/3543) (full support)
* [Candidate Implementation](https://github.com/maurer/rust/tree/patchable-function-entries)

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

### Safety Requirements for `phy::Device`

My message: https://lore.kernel.org/rust-for-linux/[e-mail]/

> +/// An instance of a PHY device.
> +///
> +/// Wraps the kernel's [`struct phy_device`].
> +///
> +/// A [`Device`] instance is created when a callback in [`Driver`] is executed. A PHY driver
> +/// executes [`Driver`]'s methods during the callback.
> +///
> +/// # Invariants
> +///
> +/// Referencing a `phy_device` using this struct asserts that you are in
> +/// a context where all methods defined on this struct are safe to call.

I know that Alice suggested this, but I reading it now, it sounds a
bit weird. When reading this it sounds like a requirement for everyone
using a `Device`. It would be better to phrase it so that it sounds like
something that users of `Device` can rely upon.

Also, I would prefer for this invariant to be a simple one, for example:
"The mutex of `self.0` is held".
The only problem with that are the `resume` and `suspend` methods.
Andrew mentioned that there is some tribal knowledge on this topic, but
I don't see this written down anywhere here. I can't really suggest an
improvement to invariant without knowing the whole picture.

I wanted to write a reply, but Fujita resent the patch series again... So here is the content of my reply, maybe we can get some consensus that we can tell him:
- it makes the safety comments not state anything, if we use the invariant inside of a function we write "by the invariant of Self, we are in a context where this is safe".
- the invariant comment that establishes the invariant needs to justify *all* `unsafe` blocks in functions defined on `Device`.
- since this invariant comment just forwards the safety requirement of `from_raw`, every callsite of `from_raw` needs to justify all `unsafe` blocks.
- if something goes wrong (ie we get a segfault etc) then the `unsafe` block to blame is the caller of `from_raw`, not the `unsafe` block that might conceptually be the problem in `Device`.

For example I could have this function:

fn foo(&self) {
    // SAFETY: by the type invariant of `Self` we are in a context where
    // this is safe to do:
    unsafe { let _: () = *core::ptr::null(); }
}

And now nobody is allowed to call `from_raw`.

current safety comments on invocations of `from_raw`:

    /// # Safety
    ///
    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
    unsafe extern "C" fn config_aneg_callback(
        phydev: *mut bindings::phy_device,
    ) -> core::ffi::c_int {
        from_result(|| {
            // SAFETY: This callback is called only in contexts
            // where we hold `phy_device->lock`, so the accessors on
            // `Device` are okay to call.
            let dev = unsafe { Device::from_raw(phydev) };
            T::config_aneg(dev)?;
            Ok(0)
        })
    }


    /// # Safety
    ///
    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
    unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int {
        from_result(|| {
            // SAFETY: The C core code ensures that the accessors on
            // `Device` are okay to call even though `phy_device->lock`
            // might not be held.
            let dev = unsafe { Device::from_raw(phydev) };
            T::suspend(dev)?;
            Ok(0)
        })
    }

Tribal knowledge mail: https://lore.kernel.org/rust-for-linux/[e-mail]/

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