On 03/30/2011 07:52 PM, Zane Bitter wrote: > On 2011/03/29, at 17:18, Steven Dake wrote: > >> When a message is received via totem, it triggers: >> static int mcast_cq_recv_event_fn (hdb_handle_t poll_handle, int >> events, int suck, void *context) >> >> This causes the recv buffer to be posted back to the hardware recv >> buffer list: >> mcast_recv_buf_post in the same funtion >> >> In your patches, when a message is received via totem, it is delivered, >> and then freed via totemsrp.c:messages_free() (which adds it to a list >> of free messages for the send queue) but never posts it back to the recv >> buffer. >> >> Regards >> -steve > > OK, so it sounds like the real issue is that we still have an extra memcpy > going on, in message_handler_mcast() at totemsrp.c:3861. In the iba case we > can avoid this copy by simply keeping the receive buffer around. In the > udp/udpa case we still need to allocate a new buffer with > totemsrp_buffer_alloc() and copy the data into it, as is happening now. > > Possible solution 1: a new totemsrp_buffer_copy() function, implemented as > buffer_alloc()+memcpy() for udp/udpa, but just snagging a reference to the > existing buffer and returning it for iba. > Possible solution 2: modify the udp/udpa drivers to allocate a buffer with > buffer_alloc() and receive data directly into it, then pass responsibility > for it to totemsrp when we deliver. This saves a copy, but may be tricky to > implement without a mutex when multiple threads are involved; I need to think > more about it. > > But in order to effect this, as you say, we need to be able to transmit or > receive and transmit from the same buffer in totemiba, and we need to make > sure that when we call buffer_release() it gets reposted to the receive queue > if that is where it originated. Or, better, we just need to make sure that > *some* buffer gets reposted to the receive queue after we receive a packet so > that we still are able to receive new packets, even if we are holding a large > number of buffers in totemsrp.c. > > Is this an accurate summary of the problem? > > The totemiba_mcast_flush_send() implementation in my patch is wrong and still > needs to be fixed, since the buffer it is passed is allocated on the stack, > not by buffer_alloc(). This can continue to allocate a send buffer internally > and memcpy() the data into it as was happening before and as we're continuing > to do with totemiba_token_send(). > > Assuming this is substantially correct, I think I now understand the > implementation well enough to start thinking about the threading, which is > the _really_ tricky part ;) >
Right on target! I believe solution two can work by merging the send and receive buffers into one data structure and having a marker indicate if it is a send buffer or receive buffer. This would allow the buffer_release api to then post to the recv buffer in the case of a recv buffer release, or add to the send list, if it is a send buffer. In regards to threading, you will probably need one list and a mutex. tls would be interesting, but I don't think it would work after more consideration. All recv buffers and freeing of those receive buffers occur on one thread (the main program's thread of execution). Most totempg_mcast calls are executed from within ipc which are all separate threads. This triggers an allocation from the ipc thread. If a buffer is freed, it is freed from the main program's thread (via messages_free which is triggered via he poll event loop). Assuming you used tls to make separate lists, All allocs would be freed into one list that was never accessible from the allocing ipc threads. > thanks, > Zane. > _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
