On Wed, Sep 01, 2021 at 03:56:20PM +0200, Fabio M. De Francesco wrote:
Anyway I tried to reason about it. I perfectly know what is required to protect critical sections of code, but I don't know how drivers work; I mean I don't know whether or not different threads that run concurrently could really interfere in that specific section. This is because I simply reason in terms of general rules of protection of critical section but I really don't know how Greybus works or (more in general) how drivers work.
From a quick look, it is indeed possible to get rid of the mutex.
If this were a high-performance path which needed to have many threads simultaneously looking up the gb_tty, or it were core kernel code, I would say that you should use kfree_rcu() to free gb_tty and then you could replace the mutex_lock() on lookup with a rcu_read_lock().
Since this is low-performance and driver code, I think you're better off simply doing this:
xa_lock((&tty_minors); gb_tty = xa_load(&tty_minors, minor); ... establish a refcount ... xa_unlock(&tty_minors);
EXCEPT ...
establishing a refcount currently involves taking a mutex. So you can't do that. First, you have to convert the gb_tty mutex to a spinlock (which looks fine to me), and then you can do this. But this is where you need to work with the driver authors to make sure it's OK.