Device bonding

Message ID 51235356.60901@tvdr.de
State New
Headers

Commit Message

Klaus Schmidinger Feb. 19, 2013, 10:26 a.m. UTC
  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:


Klaus
  

Comments

Juergen Lock Feb. 19, 2013, 8:32 p.m. UTC | #1
In article <51235356.60901@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 (other than that I ignored the diseqc
case), so it should work (testing now.)

 Thanx! :)
	Juergen
  
Klaus Schmidinger Feb. 19, 2013, 9:12 p.m. UTC | #2
On 19.02.2013 21:32, Juergen Lock wrote:
> In article <51235356.60901@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
  
Juergen Lock Feb. 19, 2013, 10:14 p.m. UTC | #3
In article <5123EAD9.7050808@tvdr.de> you write:
>On 19.02.2013 21:32, Juergen Lock wrote:
>> In article <51235356.60901@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
  
Klaus Schmidinger Feb. 19, 2013, 10:55 p.m. UTC | #4
On 19.02.2013 23:14, Juergen Lock wrote:
> In article <5123EAD9.7050808@tvdr.de> you write:
>> On 19.02.2013 21:32, Juergen Lock wrote:
>>> In article <51235356.60901@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...

Yes, those were the only two places where this function was called purely to
check whether this is the master. I've even made GetBondedMaster() private within
cDvbTuner now. It is called from cDvbTuner::SetChannel(), which should be enough.

Klaus
  

Patch

--- 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;
             }