[vdr] Device bonding

Juergen Lock vdr-l at jelal.kn-bremen.de
Tue Feb 19 23:14:16 CET 2013


In article <5123EAD9.7050808 at tvdr.de> you write:
>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; }
>
Hmm were these the only two places that were just these checks or
were there more?  If there were other places too then in those cases
the master may not be determined yet...

 Just thinking, :)
	Juergen



More information about the vdr mailing list