<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-trx/+/15524">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">radioInterface: Atomically fetch and change underrun variable<br><br>Otherwise, it could happen that underrun events are lost:<br>TxLower (isUnderrun):      RxLower (pullBuffer):<br>read(underrun)<br>                 read(underrun)<br>                        write(underrun, |val) [maybe underrun becomes TRUE]<br>write(underrun, false)<br><br>Similary, it could happen the other direction if atomic was only applied<br>to isUnderrun:<br>TxLower (isUnderrun):  RxLower (pullBuffer):<br>                 read(underrun) -> true<br>read(underrun)-> true<br>write(underrun, false)<br>                   write(underrun, true|val) where val=false<br><br>So in here isUnderrun would return true twice while it should only<br>return one.<br><br>Change-Id: I684e0a5d2a9583a161d5a6593559b3a9e7cd57e3<br>---<br>M Transceiver52M/radioInterface.cpp<br>M Transceiver52M/radioInterface.h<br>M Transceiver52M/radioInterfaceMulti.cpp<br>M Transceiver52M/radioInterfaceResamp.cpp<br>4 files changed, 10 insertions(+), 10 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/24/15524/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/Transceiver52M/radioInterface.cpp b/Transceiver52M/radioInterface.cpp</span><br><span>index 6e49a75..bfd6099 100644</span><br><span>--- a/Transceiver52M/radioInterface.cpp</span><br><span>+++ b/Transceiver52M/radioInterface.cpp</span><br><span>@@ -288,9 +288,9 @@</span><br><span> </span><br><span> bool RadioInterface::isUnderrun()</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-  bool retVal = underrun;</span><br><span style="color: hsl(0, 100%, 40%);">-  underrun = false;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+  bool retVal;</span><br><span style="color: hsl(120, 100%, 40%);">+  /* atomically get previous value of "underrun" and set the var to false */</span><br><span style="color: hsl(120, 100%, 40%);">+  retVal = __sync_fetch_and_and(&underrun, false);</span><br><span>   return retVal;</span><br><span> }</span><br><span> </span><br><span>@@ -340,7 +340,7 @@</span><br><span>                      segmentLen * 2);</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  underrun |= local_underrun;</span><br><span style="color: hsl(120, 100%, 40%);">+  __sync_or_and_fetch(&underrun, local_underrun);</span><br><span>   readTimestamp += numRecv;</span><br><span>   return 0;</span><br><span> }</span><br><span>@@ -366,7 +366,7 @@</span><br><span>                                  segmentLen,</span><br><span>                                  &local_underrun,</span><br><span>                                  writeTimestamp);</span><br><span style="color: hsl(0, 100%, 40%);">-  underrun |= local_underrun;</span><br><span style="color: hsl(120, 100%, 40%);">+  __sync_or_and_fetch(&underrun, local_underrun);</span><br><span>   writeTimestamp += numSent;</span><br><span> </span><br><span>   return true;</span><br><span>diff --git a/Transceiver52M/radioInterface.h b/Transceiver52M/radioInterface.h</span><br><span>index 83e00b9..6f4deaa 100644</span><br><span>--- a/Transceiver52M/radioInterface.h</span><br><span>+++ b/Transceiver52M/radioInterface.h</span><br><span>@@ -48,7 +48,7 @@</span><br><span>   std::vector<short *> convertRecvBuffer;</span><br><span>   std::vector<short *> convertSendBuffer;</span><br><span>   std::vector<float> powerScaling;</span><br><span style="color: hsl(0, 100%, 40%);">-  bool underrun;                       ///< indicates writes to USRP are too slow</span><br><span style="color: hsl(120, 100%, 40%);">+  int underrun;                                  ///< indicates writes to USRP are too slow</span><br><span>   bool overrun;                                ///< indicates reads from USRP are too slow</span><br><span>   TIMESTAMP writeTimestamp;                   ///< sample timestamp of next packet written to USRP</span><br><span>   TIMESTAMP readTimestamp;                   ///< sample timestamp of next packet read from USRP</span><br><span>diff --git a/Transceiver52M/radioInterfaceMulti.cpp b/Transceiver52M/radioInterfaceMulti.cpp</span><br><span>index 99f6231..8060e73 100644</span><br><span>--- a/Transceiver52M/radioInterfaceMulti.cpp</span><br><span>+++ b/Transceiver52M/radioInterfaceMulti.cpp</span><br><span>@@ -251,7 +251,7 @@</span><br><span>      convert_short_float((float *) outerRecvBuffer->begin(),</span><br><span>                       convertRecvBuffer[0], 2 * outerRecvBuffer->size());</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  underrun |= local_underrun;</span><br><span style="color: hsl(120, 100%, 40%);">+   __sync_or_and_fetch(&underrun, local_underrun);</span><br><span>  readTimestamp += num;</span><br><span> </span><br><span>    channelizer->rotate((float *) outerRecvBuffer->begin(),</span><br><span>@@ -348,7 +348,7 @@</span><br><span>          LOG(ALERT) << "Transmit error " << num;</span><br><span>        }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   underrun |= local_underrun;</span><br><span style="color: hsl(120, 100%, 40%);">+   __sync_or_and_fetch(&underrun, local_underrun);</span><br><span>  writeTimestamp += num;</span><br><span> </span><br><span>   return true;</span><br><span>diff --git a/Transceiver52M/radioInterfaceResamp.cpp b/Transceiver52M/radioInterfaceResamp.cpp</span><br><span>index 864cdee..03551ee 100644</span><br><span>--- a/Transceiver52M/radioInterfaceResamp.cpp</span><br><span>+++ b/Transceiver52M/radioInterfaceResamp.cpp</span><br><span>@@ -184,7 +184,7 @@</span><br><span>  convert_short_float((float *) outerRecvBuffer->begin(),</span><br><span>                       convertRecvBuffer[0], 2 * resamp_outchunk);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     underrun |= local_underrun;</span><br><span style="color: hsl(120, 100%, 40%);">+   __sync_or_and_fetch(&underrun, local_underrun);</span><br><span>  readTimestamp += (TIMESTAMP) resamp_outchunk;</span><br><span> </span><br><span>    /* Write to the end of the inner receive buffer */</span><br><span>@@ -232,7 +232,7 @@</span><br><span>             LOG(ALERT) << "Transmit error " << numSent;</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   underrun |= local_underrun;</span><br><span style="color: hsl(120, 100%, 40%);">+   __sync_or_and_fetch(&underrun, local_underrun);</span><br><span>  writeTimestamp += resamp_outchunk;</span><br><span> </span><br><span>       return true;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-trx/+/15524">change 15524</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-trx/+/15524"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-trx </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I684e0a5d2a9583a161d5a6593559b3a9e7cd57e3 </div>
<div style="display:none"> Gerrit-Change-Number: 15524 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>