Re: [PATCH] [media] dvb-frontends/stv0910: prevent consecutive mutex_unlock()'s

[Daniel Scheller <d.scheller.oss@gmail.com>]

Am Sat, 21 Oct 2017 11:28:10 +0200
schrieb Ralph Metzler <rjkm@metzlerbros.de>:

> Daniel Scheller writes:
>  > From: Daniel Scheller <d.scheller@gmx.net>
>  > 
>  > When calling gate_ctrl() with enable=0 if previously the mutex
>  > wasn't locked (ie. on enable=1 failure and subdrivers not handling
>  > this properly, or by otherwise badly behaving drivers), the
>  > i2c_lock could be unlocked  
> 
> I think drivers and subdrivers should rather be fixed so that this
> cannot happen.

As long as stv6111 remains the only chip/driver interfacing with the
stv0910, that's an easy task. However, if other hardware has some other
stv0910+tunerchip combination, things get interesting. In a perfect
world with unicorns and such, every component interacts as intended,
but that's not the case, so I believe this should be handled at the
root.

> But to do this we will first need to define exactly how a failure in
> gate_ctrl() is supposed to be handled, both inside gate_ctrl() and
> by calling drivers.

Well, IMHO (and thats the intention) if gate_ctrl fails due to a
hardware/I2C problem, it isn't opened so there's no need to hold the
lock (since the gate isn't - exclusively - opened). For reasons stated
above this keeps things safe from deadlocking (and we want to avoid
that, even more than double unlocking).

>  > consecutively which isn't allowed. Prevent this by keeping track
>  > of the lock state, and actually call mutex_unlock() only when
>  > certain the lock is held.  
> 
> Why not use mutex_is_locked()?

Good catch (I should try harder finding out what the kernel API has to
offer...). If you prefer that, I'll respin with this and without the
var as v2.

> And there should be a debug message if it (tried double unlocking)
> happens.

Ok. Should IMHO go to dev_dbg then - if drivers don't catch that
situation, this may else lead do kernel log spam.

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst