[kloenk]

Create a Mutex and pr_* macros

Draft for now, so I can first use it somewhere.

used it in the example as example. ready from my side.

It's a 3p library, but it may make sense to consider using https://docs.rs/lock_api/0.4.2/lock_api/

It's a 3p library, but it may make sense to consider using https://docs.rs/lock_api/0.4.2/lock_api/

I would like to keep the dependencies for kernel at a bare minimum. So would like to implement by my self, instead of using this crate.

renamed to only Mutex, as i think the example is enough to test, and so can keep the change smaller

Something broke now. Should sleep now, will Fix tomorrow.

Nitpicks: uppercase on first letter, end with period ..

Nitpicks: missing /, typo help -> held, use Markdown syntax, i.e. `CONFIG_DEBUG_LOCK_ALLOW` and `mutex_lock_nested(&mutex, 0)`, end with .. Also "task" instead of "process".

Is there any kernel APIs we can use to make this safe? It'd be a shame if all mutex usage requires unsafe.

I guess we could test if the mutex is already locked and panic in that case (similar to what Rust's std guarantees: a deadlock or a panic but never returning), but that would pretty much be like redoing the kernel mutex debugging infrastructure, so not sure how it will be received. Even if we go that route, we should have both APIs.

^ "panic in that case" -> "deadlock in that case" -- considering we are the kernel, it is way better to deadlock a task (with perhaps a warning or even an oops prior to the deadlock itself) than kill the entire system.

---

Hmm... I wonder, could we do this statically for a subset of the use cases? Say, two types, Mutex and MutexLocked which move themselves into each other on every operation (& forget themselves to avoid calling unnecessary drops which would require a test). Then we could have code like:

let m = m.lock();
...
let m = m.unlock();

Things like trylock could return an enum of either locked/unlocked, there could be a m.take(|...| { ... }), etc.

Of course, this doesn't work if one needs to keep the mutex stored in different states, but in some use cases this could be enough.

What do you think?

Not a fan. With mutexguard you only need a reference of the mutex. Your idea needs a owned type which can not live in a struct or similar.

I must be missing something here. Given that mutexes are not recursive, how is it that lock() can return a second MutexGuard when the mutex is already locked (by whomever)?

Currently it's unsafe, as if you call Ut 2 times from the same function, it calles 2 times mutex_unlock, which results in a kernel panic.

I tested this and it just deadlocked the task, perhaps we have different debugging configurations. In any case, whether it deadlocks or panics, why does it make this unsafe?

Mutex from the std library is not unsafe and behaves similarly:

The exact behavior on locking a mutex in the thread which already holds the lock is left unspecified. However, this function will not return on the second call (it might panic or deadlock, for example).

why does it make this unsafe?

It depends whether the panic is a controlled one or one triggered due to a double free, GPF or similar.

Mutex from the std library is not unsafe and behaves similarly:

Yeah, this is what I mentioned in my first comment above.

We can remove them unsafe again, and Wirte some documentation. I can only do it after my exams. So either feel free to take my work, or wait until I have more time again.

Don't worry about it. Good luck on your exams! :)

struct mutex contains a struct list_head, which is a self-referential data structure. So we need to pin Mutex before we can safely initialise and use it.

You need to call mutex_init before you can safely use it. Where is it being called?

For some mutexes (I think it was in the rtlink code) I don't saw it called anywhere. So I thought it works without calling mutex_unit if you set the head_list

For some mutexes (I think it was in the rtlink code) I don't saw it called anywhere.

In such cases they were probably defined with DEFINE_MUTEX, which allows them to skip mutex_init.

So I thought it works without calling mutex_unit if you set the head_list

Where are you setting the head of the list?

In any case, check out Generic Mutex Subsystem, in particular:
A mutex must only be initialized via the API (see below).

Hi Finn,

I just saw on the rust-for-linux list archive that you are doing this to implement pr_*; I'm not responding there because I just joined the list, so I don't have the thread to reply to.

Anyway, I don't think mutex is what you need for this because it will put the task to sleep when it is locked, which means that the pr_* functions you implement won't be usable for contexts where sleeping is not an option, for example, interrupt contents. You'll likely need a spinlock -- I was working on an implementation before, I'll upload to a branch here so that you can take a look.

But if you only need to set a value once, other sync primitives may be cheaper/better. Could you give more details on what you're trying to do?

Thanks!

Yes, I only want to set the value once. So I guess other sync primitives are better.

This will be superseded by #102 more or less, but the contribution has been very welcome!

@kloenk Perhaps you want to review the other one and/or compare it with this one; e.g. the other one does not have Display, uses a different interface, etc. :-)

Yes, closing this sounds like a good plan. Display I would like for easy (println) debugging :-). Hopefully I have time to implement it this weekend.