[vdr] Device bonding

[Klaus Schmidinger]

On 19.02.2013 21:32, Juergen Lock wrote:
> In article <51235356.60901 at tvdr.de> you write:
>> On 19.02.2013 11:19, Klaus Schmidinger wrote:
>>> On 18.02.2013 23:51, Juergen Lock wrote:
>>>> On Sun, Feb 17, 2013 at 04:34:25PM +0100, Juergen Lock wrote:
>>>>> On Sat, Feb 16, 2013 at 04:46:36PM +0100, Juergen Lock wrote:
>>>>>> Hi!
>>>>>>
>>>>>> [...]
>>>>>
>>>>>> 3. Running with these four tuners (dual DVB-T and the bonded two DVB-S2)
>>>>>>      I get two different deadlocks waiting for cDvbTuner::bondMutex
>>>>>>      after live viewing a DVB-T(!) channel for longer (OSD doesn't
>>>>>>      react anymore and attaching gdb reveals two threads waiting for
>>>>>>      bondMutex) - the following two changes make it work but there
>>>>>>      probably is a better fix:  (patch may apply with offsets; one
>>>>>>      of the problems I think is a lock order reversal with cDvbTuner::mutex
>>>>>>      and bondMutex when cDvbTuner::SetChannel calls back into itself
>>>>>>      with bondMutex held.)
>>>>>>
>>>>>> --- dvbdevice.c.orig
>>>>>> +++ dvbdevice.c
>>>>>> @@ -476,8 +476,10 @@ void cDvbTuner::SetChannel(const cChanne
>>>>>>                      t->SetChannel(NULL);
>>>>>>                  }
>>>>>>               }
>>>>>> -        else if (strcmp(GetBondingParams(Channel), BondedMaster->GetBondingParams()) != 0)
>>>>>> +        else if (strcmp(GetBondingParams(Channel), BondedMaster->GetBondingParams()) != 0) {
>>>>>> +           bondMutex.Unlock();
>>>>>>               BondedMaster->SetChannel(Channel);
>>>>>> +           }
>>>>>>            }
>>>>>>         cMutexLock MutexLock(&mutex);
>>>>>>         if (!IsTunedTo(Channel))
>>>>>> @@ -761,7 +773,12 @@ bool cDvbTuner::SetFrontend(void)
>>>>>>               tone = SEC_TONE_ON;
>>>>>>               }
>>>>>>            int volt = (dtp.Polarization() == 'V' || dtp.Polarization() == 'R') ? SEC_VOLTAGE_13 : SEC_VOLTAGE_18;
>>>>>> -        if (GetBondedMaster() != this) {
>>>>>> +#if 1
>>>>>> +        if (bondedTuner && !bondedMaster)
>>>>>> +#else
>>>>>> +        if (GetBondedMaster() != this)
>>>>>> +#endif
>>>>>> +           {
>>>>>>               tone = SEC_TONE_OFF;
>>>>>>               volt = SEC_VOLTAGE_13;
>>>>>>               }
>>>>>>
>>>>>
>>>>> Hmm looks like I posted too soon, the first hunk is actually too much
>>>>> and causes other deadlocks (like when trying to play a DVB-S channel
>>>>> via streamdev while live viewing another), so the patch I'm not testing
>>>>
>>>> .. I'm _now_ testing...
>>>>
>>>>> becomes:
>>>>>
>>>>> --- dvbdevice.c.orig
>>>>> +++ dvbdevice.c
>>>>> @@ -761,7 +773,12 @@ bool cDvbTuner::SetFrontend(void)
>>>>>               tone = SEC_TONE_ON;
>>>>>               }
>>>>>            int volt = (dtp.Polarization() == 'V' || dtp.Polarization() == 'R') ? SEC_VOLTAGE_13 : SEC_VOLTAGE_18;
>>>>> -        if (GetBondedMaster() != this) {
>>>>> +#if 1
>>>>> +        if (bondedTuner && !bondedMaster)
>>>>> +#else
>>>>> +        if (GetBondedMaster() != this)
>>>>> +#endif
>>>>> +           {
>>>>>               tone = SEC_TONE_OFF;
>>>>>               volt = SEC_VOLTAGE_13;
>>>>>               }
>>>>>
>>>
>>> Can you please test whether this one works just as well?
>>>
>>> --- dvbdevice.c 2013/02/17 13:17:33     2.80
>>> +++ dvbdevice.c 2013/02/19 10:18:08
>>> @@ -742,7 +742,7 @@
>>>            if (const cDiseqc *diseqc = Diseqcs.Get(device->CardIndex() + 1, channel.Source(), frequency, dtp.Polarization(), &scr)) {
>>>               frequency -= diseqc->Lof();
>>>               if (diseqc != lastDiseqc || diseqc->IsScr()) {
>>> -              if (GetBondedMaster() == this) {
>>> +              if (bondedMaster) {
>>>                     ExecuteDiseqc(diseqc, &frequency);
>>>                     if (frequency == 0)
>>>                        return false;
>>> @@ -768,7 +768,7 @@
>>>               tone = SEC_TONE_ON;
>>>               }
>>>            int volt = (dtp.Polarization() == 'V' || dtp.Polarization() == 'R') ? SEC_VOLTAGE_13 : SEC_VOLTAGE_18;
>>> -        if (GetBondedMaster() != this) {
>>> +        if (!bondedMaster) {
>>>               tone = SEC_TONE_OFF;
>>>               volt = SEC_VOLTAGE_13;
>>>               }
>>
>> Sorry, that was a mistake.
>> Try this one, please:
>>
>> --- dvbdevice.c 2013/02/17 13:17:33     2.80
>> +++ dvbdevice.c 2013/02/19 10:24:39
>> @@ -742,7 +742,7 @@
>>           if (const cDiseqc *diseqc = Diseqcs.Get(device->CardIndex() + 1, channel.Source(), frequency, dtp.Polarization(), &scr)) {
>>              frequency -= diseqc->Lof();
>>              if (diseqc != lastDiseqc || diseqc->IsScr()) {
>> -              if (GetBondedMaster() == this) {
>> +              if (!bondedTuner || bondedMaster) {
>>                    ExecuteDiseqc(diseqc, &frequency);
>>                    if (frequency == 0)
>>                       return false;
>> @@ -768,7 +768,7 @@
>>              tone = SEC_TONE_ON;
>>              }
>>           int volt = (dtp.Polarization() == 'V' || dtp.Polarization() == 'R') ? SEC_VOLTAGE_13 : SEC_VOLTAGE_18;
>> -        if (GetBondedMaster() != this) {
>> +        if (bondedTuner && !bondedMaster) {
>>              tone = SEC_TONE_OFF;
>>              volt = SEC_VOLTAGE_13;
>>              }
>>
> Yeah that's the same as I had it

Absolutely! I thought it would work in a simpler manner, but I was wrong.
Didn't mean to "steal" your idea ;-).

> (other than that I ignored the diseqc
> case), so it should work (testing now.)

I just systematically replaced all calls to GetBondedMaster() that were just checks with
the appropriate use of bondedTuner and bondedMaster.
Maybe I'll even put this into a function

bool IsBondedMaster(void) const { return !bondedTuner || bondedMaster; }

Klaus