patch 22: lchan_use

[Holger Freyther]

On Tuesday 02 June 2009 17:04:33 Andreas.Eversberg wrote:
> lchan_free() is called when RSL released the logical channel. all
> ressources that 'uses' the channel, must be released. i aggree with you,
> that it makes no sense to consider everything that 'uses' the logical
> channel. to fix this, i have an idea and need some response before
> implementing.
>
> problems:
>  - what instance uses the logical channel?
>  - how do we release them, if the channel is closed?
>  - how do we change the channel (handover/SDCCH->TCH ...)? (is this
> required?)
>

>
> what do you think?

Let me tell you what I intended for the lchan and what I see is missing.

Normal operation:
  Basic:
	- lchan is a logical channel
	- we use refcount, when refcount drops zero we will release it
	  (arbitrary timeout to delay it)

 Logical Operations:
         - On top of this we have a logical operation/transaction...
         - E.g. Location Updating Request....
         - It has a specified timeout of 20 seconds T3210 (okay that
	   is cheating as this timer is from the mobile station)
         - During this operation we send a couple of commands, want
           the IMSI, IMEISV, etc. and ultimately want to end in a
           CM SERVICE REQUEST (oops... another bug in our code)
         - But we might not get every command answered, so manipulating
           the refcount for IMSI, IMEISV, etc. might result in leaks..
         - This is where the logical operation comes into place to "hold" the
           ref throughout the operation and put_lchan once it is done (either
           by timeout, failure or success)

         On the specific bug you see, I think removing use_lchan/put_lchan for
         the separate IMSI/IMEISV should fix your leak, and should be safe as
         the creation of the logical operation is getting the lock.


Failure Case:
     - We get a RSL_MT_RF_CHAN_REL_ACK with refcount != 0
     - so the channel got closed under our back...
     - The question is how to do error recovery in general in OpenBSC

     - Your proposal is to have a list of users in each lchan to run 
       "destructors".


     - I think something as easy as our callback system can be used. Just
       broadcast an UNEXPECTED LCHAN RELEASE event and let the client
       code figure out what resources to release.



So I think with "users" we already have them in the sense of logical operation 
that can be embedded into the lchan. So far I would opt for pointers in the 
lchan structure, we can make that a llist once we have too many of them.

But I wouldn't want to mix that with the "error" case, normally the 
"destructors" of an user are ran when the operation is completed and 
afterwards the reference gets dropped. The only case this does not happen is 
the unexpected release of a channel.

what do you think?