Hi Stanislav, Yes, binary api messages are handled by main thread. If the issue is purely binary api related, then yes, checking for pool (vectors or bitmap) realloc with single writer should work.
Note that the pool’s free_indices vector can also expand (not only the bitmap), so that should be checked as well. Regards, Florin > On Nov 4, 2021, at 1:58 PM, Stanislav Zaikin <zsta...@gmail.com> wrote: > > Hi Damjan, > Hi Florin, > > I may have given the wrong example, even though the IP API's marked as > mt-safe (VL_API_IP_ROUTE_ADD_DEL/VL_API_IP_ROUTE_ADD_DEL_V2), they'd be > called only from the main thread, right? So, only one pool_put will be called > and it looks like the patch actually fixes the problem (I still don't like > it). > > I liked the idea with a vec of freed elements, but we still need to put a > worker barrier to perform such an operation. I thought about something more > lightweight, like CAS for a newly allocated bitmap/vector. But I find myself > extremely incompetent to write any lock-free solutions. > > > On Thu, 4 Nov 2021 at 20:01, Florin Coras <fcoras.li...@gmail.com > <mailto:fcoras.li...@gmail.com>> wrote: > Agreed! Since pools are not thread safe, the whole load balance destroy > scheme should probably be refactored. > > Maybe add, with locks, the indices to be freed to a vector and once enough > accumulate, or if a new element is needed, grab the worker barrier and > actually free them all. > > Regards, > Florin > >> On Nov 4, 2021, at 11:50 AM, Damjan Marion via lists.fd.io >> <http://lists.fd.io/> <dmarion=me....@lists.fd.io >> <mailto:dmarion=me....@lists.fd.io>> wrote: >> >> >> Dear Stanislav, >> >> It doesn’t look to me as a thread safe solution. >> >> i.e imagine 2 threads call pool_put_will expand on the same time, and there >> is just one free slot. Both will get negative answer, but 2nd put operation >> will actually expand. >> >> — >> Damjan >> >>> On 04.11.2021., at 18:24, Stanislav Zaikin <zsta...@gmail.com >>> <mailto:zsta...@gmail.com>> wrote: >>> >>> >>> Hello folks, >>> >>> In a multi-threaded environment (in my case I have 2 workers) I observed a >>> crash, and thanks to Neale, it turned out that free_bitmap may expand while >>> doing pool_put. >>> Let's say one thread is doing pool_put, while another thread is calling >>> "pool_elt_at_index". I observed different addresses before and after >>> checking "ASSERT (! pool_is_free (p, _e))" in that macro. >>> >>> I prepared a patch [0], but it's kind of ugly. We don't have asserts in >>> release mode, so why should we care about it? >>> >>> On the other hand, 2 different threads can do 2 pool_puts simultaneously >>> and we can lose one free element in the pool (and also additionally >>> allocated bitmap). >>> >>> For me, it's a place where it would be nice to have an mt-safe vec. What do >>> you think? >>> >>> [0] - https://gerrit.fd.io/r/c/vpp/+/34332 >>> <https://gerrit.fd.io/r/c/vpp/+/34332> >>> -- >>> Best regards >>> Stanislav Zaikin >>> >>> >>> >> >> > > > > -- > Best regards > Stanislav Zaikin
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#20429): https://lists.fd.io/g/vpp-dev/message/20429 Mute This Topic: https://lists.fd.io/mt/86821639/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
