[asterisk-dev] [Code Review]: Astobj2 locking enhancement

[rmudgett]

> On Oct. 31, 2011, 6:02 p.m., David Vossel wrote:
> > /trunk/include/asterisk/astobj2.h, lines 729-743
> > <https://reviewboard.asterisk.org/r/1554/diff/1/?file=21542#file21542line729>
> >
> >     I kind of hate this option... Which is okay for me to say because I made it.
> >     
> >     It was made to optimize how I was doing the format interface lookup to use read write locks external to the astobj2 container.  Now that rw locks are a part of the container itself, lets just get rid of this option all together.  It adds some really weird complications to the whole thing now.  This flag is used on app_queue as well, for a completely different reason that I am not fond of.  That usage should just go away.
> >     
> >

You were just completing the set since the corresponding iterator flag has existed for a very long time.

This flag and the corresponding flag for iterators are needed for RWLOCKs as stated in the note describing the flag above.  For an example of how it is used properly see format.c:ast_format_attr_reg_interface().

The flag must be used with RWLOCKs to tell the ao2 code that the thread already holds a lock and so it can detect what lock strength is held.

The use of the OBJ_LOCK flag by app_queue.c is within the rules defined here.  I don't see much gain with its use there since I don't think that acquiring a MUTEX lock a second time is that expensive.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1554/#review4619
-----------------------------------------------------------


On Oct. 31, 2011, 3:01 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1554/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2011, 3:01 p.m.)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> Add the ability to specify what kind of locking an ao2 object has when it is allocated.
> The locking could be one of: MUTEX, RWLOCK, or none.
> 
> New API:
> 
> ao2_t_alloc_options()
> ao2_alloc_options()
> ao2_t_container_alloc_options()
> ao2_container_alloc_options()
> 
> ao2_rdlock()
> ao2_wrlock()
> ao2_tryrdlock()
> ao2_trywrlock()
> 
> The OBJ_NOLOCK and AO2_ITERATOR_DONTLOCK flags have a slight meaning 
> change.  They no longer mean that the object is protected by an external 
> mechanism.  They mean the lock associated with the object has already been 
> manually obtained by one of the ao2_lock calls.  This change is necessary 
> for RWLOCK support since they are not reentrant.  Also an operation on an 
> ao2 container may require promoting a read lock to a write lock by 
> releasing the already held read lock to re-acquire as a write lock.  
> 
> 
> The changes to format.c and format_cap.c are taking advantange of the new 
> ao2 locking options to simplify the use of the format capabilities 
> containers.  
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/lock.h 342867 
>   /trunk/main/astobj2.c 342867 
>   /trunk/main/format.c 342867 
>   /trunk/main/format_cap.c 342867 
>   /trunk/include/asterisk/astobj2.h 342867 
> 
> Diff: https://reviewboard.asterisk.org/r/1554/diff
> 
> 
> Testing
> -------
> 
> Ran the ao2 unit tests.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111101/494f6fc3/attachment.htm>