[PATCH] Release lchan directly when refcount drops to zero
Hi all, I have written this small patch yesterday. It is removing the "auto release" after timeout from lchan and will directly release the channel ones the ref count drops to zero. This means that any use of the lchan after a put_lchan is illegal (the code using a timeout used to workaround this issue). I have changed three call sites to follow this convention, I have added a BUG_ON into the abis_rsl method. From a quick test (Location Updating Request and a call) the BUG_ON in the abis_rsl.c is hit by: - The lchan release code (okay... a false warning and avoided by increasing use_count and decreasing it...) - CM Service Request Ack and such is sent without anyone holding a reference to it. I think this warning is safe to ignore for now as no one has simply claimed the channel. I plan to fix this in January/February when moving our MSC code over to the On Waves BSC abstraction. The benefit of this change is that we free our channels a lot earlier. I think this change would be beneficial for the 26C3 and I would like to hear opinions and see some testing with it. regards z.
You need to add #include <openbsc/chan_alloc.h> to silent_call.c or it doesn't compile. (at least on my laptop).
Hi zecke, On Wed, Nov 18, 2009 at 10:19:27AM +0100, Holger Freyther wrote:
Hi all,
I have written this small patch yesterday. It is removing the "auto release" after timeout from lchan and will directly release the channel ones the ref count drops to zero.
thanks for investigating this. The reason why we have this long timeout is mainly due to MO SMS. What happens in the MO SMS case is that we get a CM SERVICE REQUEST is that the channel stays active for a very long time (especially if you have a very long sms) before we get the actual CP-DATA. If our timeout is too low, then the channel will simply timeout while the SMS is being transmitted. So what do we do here in order to solve the issue ? Handle CM SERVICE REQUEST as a special case and manually increase it or start some special timer? The only idea I had was that we should be getting and ESTABLISH INDICATION on SAPI=3, but I'm not sure if that happens before the CP-DATA. So whatever change you make, if it survives sending a maximum-sized MO SMS ona SDCCH (plus even accounting for some L2 retransmissions), then I'm happy with it! Regards, Harald -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
On Friday 20 November 2009 14:04:35 Harald Welte wrote:
Hi zecke,
On Wed, Nov 18, 2009 at 10:19:27AM +0100, Holger Freyther wrote:
Hi all,
I have written this small patch yesterday. It is removing the "auto release" after timeout from lchan and will directly release the channel ones the ref count drops to zero.
thanks for investigating this.
The reason why we have this long timeout is mainly due to MO SMS.
What happens in the MO SMS case is that we get a CM SERVICE REQUEST is that the channel stays active for a very long time (especially if you have a very long sms) before we get the actual CP-DATA.
If our timeout is too low, then the channel will simply timeout while the SMS is being transmitted.
So what do we do here in order to solve the issue ? Handle CM SERVICE REQUEST as a special case and manually increase it or start some special timer?
Hi LaF0rge, I'm not really following here. I have removed the lchan "auto release" timer for now and directly release when the refcount drops down to zero. The only timer that is active is the RSL timer we start in CHan ACTivate (or such). For the CM Service Request... In the future this, Locationg Updating Request and Paging Response (some more according to 08.08) would start the transaction and we would keep the channel open as long as the transaction is alive. We will introduce a "channel" leak only in the case where we will leak the transaction. But this is a bug. In any case I will try sending a real long SMS from my phone and try to observe what is happening. regards holger
Hi Holger, On Fri, Nov 20, 2009 at 02:37:40PM +0100, Holger Freyther wrote:
I'm not really following here. I have removed the lchan "auto release" timer for now and directly release when the refcount drops down to zero. The only timer that is active is the RSL timer we start in CHan ACTivate (or such).
For the CM Service Request... In the future this, Locationg Updating Request and Paging Response (some more according to 08.08) would start the transaction and we would keep the channel open as long as the transaction is alive.
yes, that sounds great and it is definitely how it should be. But is this already working with your proposed patch? At least when I was trying last, simply reducing the timeout to 5 seconds already made MO SMS fail. If there meanwhile were some changes to the core code that make it work: Great!
We will introduce a "channel" leak only in the case where we will leak the transaction. But this is a bug.
ok, that's fine with me.
In any case I will try sending a real long SMS from my phone and try to observe what is happening.
ok, thanks. -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
participants (3)
-
Harald Welte -
Holger Freyther -
Sylvain Munaut