# Meeting 2023-09-20 <!-- 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 --> ## Discussion Questions <!-- Anything that requires lengthy discussion/more general questions also fit here --> ### Generalized Track Caller Gary: what would the idea language feature for generalised track caller look like? Or maybe a macro for each callsite isn't that bad? We might be able to have a single macro that takes care of all lockdep callsite tracking, e.g: ```rust // Current let mutex = Box::pin_init(new_mutex!(42))?; // Option 1: struct Callsite { key: &'static LockClassKey, name: &'static CStr, loc: &'static Location<'static>, } let mutex = Box::pin_init(Mutex::new(42, callsite!())); // Option 2: trait PinInitWithCallsite; // with_callsite!(impl PinInitWithCallsite): impl PinInit let mutex = Box::pin_init(with_callsite!(Mutex::new(42))); ``` Concensus: better than the current macros. Option 1 seems better and allows re-use lock class key. Generalized track caller: ```rust pub trait Callsite { const fn new(at: &'static Location<'static>) -> Self; } pub struct CustomCallsite { loc: &'static Location<'static>, } impl Callsite for CustomCallsite {} #[track_caller = callsite] fn tracked(callsite: &CustomCallsite) {} // calls to tracked would be desugared to static tracked_callsite: CustomCallsite = ... ``` Alice: I am pessimistic about upstreaming this kind of feature. `callsite!()` is good enough for me. Boqun: LockClassKey is just a key, so maybe there doesn't need to be a static reference if we can modify C side API. Gary: What do we use as the unique identifier? A hash of Location? Boqun: TypeId Gary: You can't get TypeId from the callee. Boqun: Would something like the below work? ```rust= impl<T> Mutex<T> { pub fn new(data: T) { let key = core::any::TypeId::of::<T>(); ... } } ``` Gary: That's for a given type, not a given callsite. If the `Mutex` contains some common types, then they'll be sharing a key. Boqun: Yes, but my point is more that lockclass key should really be type-based, the current location-based design is because C's limitation. Gary: If someone is using `Mutex` just for synchronising they might be using `Mutex<()>` (aka `RawMutex`), so everyone will be using the same key. I think location-based makes more sense. Boqun: But actually, they can use dummy type to separate the different things they protect, for example `Mutex<INode>`, `Mutex<Dentry>` where `INode` and `Dentry` are just empty struct. Gary: Yes, but it sounds quite error-prone to require custom new types to use Mutex. Maybe require an extra tag type `Mutex<T, TagTypeSolelyForLockDep>`? Benno: that will be an ergonomic nightmare if I want multiple instances of a type that containse a mutex (or worse multiple ones) with different lock classes Boqun: Probably I could play around this idea a little bit more, so that we can evalute it with concrete cases, either way `callsite!()` looks good to me. ### Minimum Patch Size? Benno: Does the kernel have a minimum patch size? Because I think that patches like [this](https://lore.kernel.org/rust-for-linux/20230906204857.85619-1-manmshuk@gmail.com/) should maybe fix more markdown issues (if there are more). Wedson: I don't think there's a minimum size for a patch. One-liners should be ok. Benno: IIRC we also use these small issues as an opportunity for people to have their first contribution. Miguel: yes, we have some tagged with the "good first issue" label, one can see the list at https://github.com/Rust-for-Linux/linux/contribute. ## Miscellaneous <!-- stuff that does not fit into other categories -->