> > > +/** > > > + * @internal Reserve ring elements to enqueue several objects on the > > > +ring > > > + * > > > + * @param r > > > + * A pointer to the ring structure. > > > + * @param esize > > > + * The size of ring element, in bytes. It must be a multiple of 4. > > > + * This must be the same value used while creating the ring. Otherwise > > > + * the results are undefined. > > > + * @param n > > > + * The number of elements to reserve in the ring. > > > + * @param behavior > > > + * RTE_RING_QUEUE_FIXED: Reserve a fixed number of elements from a > > ring > > > + * RTE_RING_QUEUE_VARIABLE: Reserve as many elements as possible > > from ring > > > + * @param is_sp > > > + * Indicates whether to use single producer or multi-producer reserve > > > + * @param old_head > > > + * Producer's head index before reservation. > > > + * @param new_head > > > + * Producer's head index after reservation. > > > + * @param free_space > > > + * returns the amount of space after the reserve operation has > > > finished. > > > + * It is not updated if the number of reserved elements is zero. > > > + * @param dst1 > > > + * Pointer to location in the ring to copy the data. > > > + * @param n1 > > > + * Number of elements to copy at dst1 > > > + * @param dst2 > > > + * In case of ring wrap around, this pointer provides the location to > > > + * copy the remaining elements. The number of elements to copy at this > > > + * location is equal to (number of elements reserved - n1) > > > + * @return > > > + * Actual number of elements reserved. > > > + * If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only. > > > + */ > > > +static __rte_always_inline unsigned int > > > +__rte_ring_do_enqueue_elem_reserve(struct rte_ring *r, unsigned int > > > +esize, > > > > > > I do understand the purpose of reserve, then either commit/abort for serial > > sync mode, but what is the purpose of non-serial version of reserve/commit? > In RCU, I have the need for scatter-gather feature. i.e. the data in the ring > element is coming from multiple sources ('token' is generated by > the RCU library and the application provides additional data). If I do not > provide the reserve/commit, I need to introduce an intermediate > memcpy to get these two data contiguously to copy to the ring element. The > sequence is 'reserve(1), memcpy1, mempcy2, commit(1)'. > Hence, you do not see the abort API for the enqueue. > > > In serial MP/MC case, after _reserve_(n) you always have to do > > _commit_(n) - you can't reduce number of elements, or do _abort_. > Agree, the intention here is to provide the scatter/gather feature. > > > Again you cannot avoid memcpy(n) here anyhow. > > So what is the point of these functions for non-serial case? > It avoids an intermediate memcpy when the data is coming from multiple > sources.
Ok, I think I understand what was my confusion: Your intention: 1) reserve/commit for both serial and non-serial mode - to allow user get/set contents of the ring manually and avoid intermediate load/stores. 2) abort only for serial mode. My intention: 1) commit/reserve/abort only for serial case (as that's the only mode where we can commit less then was reserved or do abort). 2) get/set of ring contents are done as part of either reserve(for dequeue) or commit(for enqueue) API calls (no scatter-gather ability). I still think that this new API you suggest creates too big exposure of ring internals, and makes it less 'safe-to-use': - it provides direct access to contents of the ring. - user has to specify head/tail values directly. So in case of some programmatic error in related user code, there are less chances it could be catch-up by API, and we can easily end-up with silent memory corruption and other nasty things that would be hard to catch/reproduce. That makes me wonder how critical is this scatter-gather ability in terms of overall RCU performance? Is the gain provided really that significant, especially if you'll update the ring by one element at a time? > > > > > BTW, I think it would be good to have serial version of _enqueue_ too. > If there is a good use case, they should be provided. I did not come across a > good use case. > > > > > > + unsigned int n, enum rte_ring_queue_behavior behavior, > > > + unsigned int is_sp, unsigned int *old_head, > > > + unsigned int *new_head, unsigned int *free_space, > > > + void **dst1, unsigned int *n1, void **dst2) > > > > I do understand the intention to avoid memcpy(), but proposed API seems > > overcomplicated, error prone, and not very convenient for the user. > The issue is the need to handle the wrap around in ring storage array. i.e. > when the space is reserved for more than 1 ring element, the wrap > around might happen. > > > I don't think that avoiding memcpy() will save us that many cycles here, so > This depends on the amount of data being copied. > > > probably better to keep API model a bit more regular: > > > > n = rte_ring_mp_serial_enqueue_bulk_reserve(ring, num, &free_space); ... > > /* performs actual memcpy(), m<=n */ > > rte_ring_mp_serial_enqueue_bulk_commit(ring, obj, m); > These do not take care of the wrap-around case or I am not able to understand > your comment. I meant that serial_enqueue_commit() will do both: actual copy of elements to the ring and tail update (no Scatter-Gather), see above.