Greg Ames wrote: > Brian Pane wrote: > > Greg Ames wrote: > > > >> apr_atomic_dec? That does return something. > > > The problem is that, since we have to use apr_atomic_cas for the > > increment (due to the lack of a return value on apr_atomic_inc), > > we can't use apr_atomic_dec on the same variable. apr_atomic_cas > > works on apr_uint32_t, while apr_atomic_dec works on apr_atomic_t. > > If we could change the apr_atomic_inc/dec functions to use apr_uint32_t, > > this part of the fdqueue code could become a lot simpler. > > I am certainly in favor of changing apr_atomic_inc/dec so they can be useful. > I'm wondering if it's ok to use an ordinary apr type, like apr_uint32_t? or do > we need special atomic types marked as volatile or memory-resident or whatever > so that gcc won't assign them to registers or optimize them out or ??? I don't > know the answer, but have seen kernel code do such things (OK Jeff, I've been > infected by the GPL...no hope for me).
As far as I know, we can make it work with apr_uint32_t on most platforms, as long as we declare any inline assembly blocks as volatile (thanks to Sascha Schumann for fixing this recently in apr_atomic_cas on Linux). The one platform I'm not sure of is OS/390, due to the special data types and API involved: http://marc.theaimsgroup.com/?l=apr-dev&m=104129861312811 > > > I still have one more change I'm hoping to make in the fdqueue > > synchronization logic: move the queue manipulation code in > > ap_queue_push/pop outside the mutex protected region by maintaining > > a linked list of queue nodes with apr_atomic_casptr. > > Sounds good, as long as the pops are single threaded somehow, or if you have > some other way of getting around the A-B-A cas pop window. I'm planning on spinning around the CAS; i.e., retry if the CAS fails due to a collision with another thread. The queue_info synchronization, which uses that same technique, seems to be working well in practice. Brian