Re: Device bonding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19.02.2013 23:14, Juergen Lock wrote:
In article <5123EAD9.7050808@xxxxxxx> you write:
On 19.02.2013 21:32, Juergen Lock wrote:
In article <51235356.60901@xxxxxxx> 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

_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


[Index of Archives]     [Linux Media]     [Asterisk]     [DCCP]     [Netdev]     [Xorg]     [Util Linux NG]     [Xfree86]     [Big List of Linux Books]     [Fedora Users]     [Fedora Women]     [ALSA Devel]     [Linux USB]

  Powered by Linux