Devin Heitmueller wrote:
> On Fri, Jan 22, 2010 at 3:22 PM, Manu Abraham <abraham.m...@gmail.com> wrote:
>> On Fri, Jan 22, 2010 at 11:40 PM, Devin Heitmueller
>> <dheitmuel...@kernellabs.com> wrote:
>>> Also, the dvb_frontend.c makes calls to i2c_gate_ctrl() at various
>>> points, so you would need to ensure that none of those occur before
>>> calling into your driver as there could potentially be a deadlock
>>> there too.
>> Ok, thanks for the pointer. The gate control is never called
>> externally in reality. I will wait a little while for this patch to be
>> applied.  It removes the exported function and thereby an unnecessary
>> dereference.
>>
>> http://jusst.de/hg/stv090x/rev/b3d28f5b2b53
> 
> If it never needs to be called externally, then removing it from the
> dvb_frontend_ops does eliminate the risk entirely.  The case I
> frequently see it called from dvb_frontend.c is for powering down the
> tuner when the dvb frontend thread shuts down.

The removal of the external call to the gate function removes the risk that
an external call, at dvb core, to keep it locked. Yet, the code there is 
completely
symetric nowadays.

However, the proper documentation of the lock is needed to avoid the risk
of some patch to keep the mutex locked.

As, even the initial lock changeset has this problem (later fixed by
http://jusst.de/hg/stv090x/rev/3a8f35abc0f2), it shows that a good documentation
is required.

As you've talked about a FSM, the lock itself is a FSM with 2 states. All I'm
asking is that you document that the lock FSM has changed its state in the name
of the function that alters the state of the lock.

Cheers,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to