From eaacce4753c33342a6512039fe4153b15b476fb3 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Tue, 2 Jul 2019 13:07:37 -0700 Subject: svm: fix multi-chunk fifo alloc and add more tests Type: fix - make sure that chunks and the rbtree are initialized if fifo segment allocates multiple chunks for the fifo. - ensure head/tail chunks are updated on all enqueue/dequeue events, including when dropping data. - more unit tests Also fixes dequeue drop updates of head chunk. Change-Id: I77f3550bc4e8b4e077f80ea87fe82b83ed013aeb Signed-off-by: Florin Coras --- src/svm/fifo_segment.c | 18 +++++++-- src/svm/svm_fifo.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++--- src/svm/svm_fifo.h | 16 +++++++- 3 files changed, 127 insertions(+), 10 deletions(-) (limited to 'src/svm') diff --git a/src/svm/fifo_segment.c b/src/svm/fifo_segment.c index d47c8534d14..eeb274636c1 100644 --- a/src/svm/fifo_segment.c +++ b/src/svm/fifo_segment.c @@ -394,6 +394,14 @@ fifo_segment_alloc_fifo (fifo_segment_t * fs, u32 data_bytes, /* (re)initialize the fifo, as in svm_fifo_create */ svm_fifo_init (f, data_bytes); + /* Initialize chunks and rbtree for multi-chunk fifos */ + if (f->start_chunk->next != f->start_chunk) + { + void *oldheap = ssvm_push_heap (fs->ssvm.sh); + svm_fifo_init_chunks (f); + ssvm_pop_heap (oldheap); + } + /* If rx fifo type add to active fifos list. When cleaning up segment, * we need a list of active sessions that should be disconnected. Since * both rx and tx fifos keep pointers to the session, it's enough to track @@ -613,12 +621,16 @@ fifo_segment_preallocate_fifo_pairs (fifo_segment_t * fs, /* Calculate space requirements */ pair_size = 2 * hdrs + rx_rounded_data_size + tx_rounded_data_size; space_available = fs_free_space (fs); - pairs_to_alloc = clib_min (space_available / pair_size, *n_fifo_pairs); + pairs_to_alloc = space_available / pair_size; + pairs_to_alloc = clib_min (pairs_to_alloc, *n_fifo_pairs); + + if (!pairs_to_alloc) + return; if (fs_try_alloc_fifo_batch (fs, rx_fl_index, pairs_to_alloc)) - clib_warning ("rx prealloc failed"); + clib_warning ("rx prealloc failed: pairs %u", pairs_to_alloc); if (fs_try_alloc_fifo_batch (fs, tx_fl_index, pairs_to_alloc)) - clib_warning ("tx prealloc failed"); + clib_warning ("tx prealloc failed: pairs %u", pairs_to_alloc); /* Account for the pairs allocated */ *n_fifo_pairs -= pairs_to_alloc; diff --git a/src/svm/svm_fifo.c b/src/svm/svm_fifo.c index 3d538293c70..56f53a3d339 100644 --- a/src/svm/svm_fifo.c +++ b/src/svm/svm_fifo.c @@ -404,6 +404,31 @@ svm_fifo_init (svm_fifo_t * f, u32 size) f->head_chunk = f->tail_chunk = f->ooo_enq = f->ooo_deq = f->start_chunk; } +void +svm_fifo_init_chunks (svm_fifo_t * f) +{ + svm_fifo_chunk_t *c, *prev; + + if (f->start_chunk->next == f->start_chunk) + return; + + f->flags |= SVM_FIFO_F_MULTI_CHUNK; + rb_tree_init (&f->chunk_lookup); + rb_tree_add2 (&f->chunk_lookup, 0, pointer_to_uword (f->start_chunk)); + + f->start_chunk->start_byte = 0; + prev = f->start_chunk; + c = prev->next; + + while (c != f->start_chunk) + { + c->start_byte = prev->start_byte + prev->length; + rb_tree_add2 (&f->chunk_lookup, c->start_byte, pointer_to_uword (c)); + prev = c; + c = c->next; + } +} + /** * Creates a fifo in the current heap. Fails vs blow up the process */ @@ -559,6 +584,7 @@ svm_fifo_add_chunk (svm_fifo_t * f, svm_fifo_chunk_t * c) * that this is called with the heap where the rbtree's pool is pushed. */ if (!(f->flags & SVM_FIFO_F_MULTI_CHUNK)) { + ASSERT (f->start_chunk->next == f->start_chunk); rb_tree_init (&f->chunk_lookup); rb_tree_add2 (&f->chunk_lookup, 0, pointer_to_uword (f->start_chunk)); f->flags |= SVM_FIFO_F_MULTI_CHUNK; @@ -626,7 +652,7 @@ svm_fifo_collect_chunks (svm_fifo_t * f) void svm_fifo_try_shrink (svm_fifo_t * f, u32 head, u32 tail) { - u32 len_to_shrink = 0, tail_pos, len; + u32 len_to_shrink = 0, tail_pos, len, last_pos; svm_fifo_chunk_t *cur, *prev, *next, *start; tail_pos = tail; @@ -649,13 +675,24 @@ svm_fifo_try_shrink (svm_fifo_t * f, u32 head, u32 tail) * - not wrapped * - last used byte less than start of last chunk */ - if (tail_pos >= head && tail_pos <= f->end_chunk->start_byte) + if (tail_pos >= head && tail_pos < f->end_chunk->start_byte) { /* Lookup the last position not to be removed. Since size still needs - * to be nitems + 1, nitems must fall within the usable space */ - tail_pos = tail_pos > 0 ? tail_pos - 1 : tail_pos; - prev = svm_fifo_find_chunk (f, clib_max (f->nitems, tail_pos)); + * to be nitems + 1, nitems must fall within the usable space. Also, + * first segment is not removable, so tail_pos can be 0. */ + last_pos = tail_pos > 0 ? tail_pos - 1 : tail_pos; + prev = svm_fifo_find_chunk (f, clib_max (f->nitems, last_pos)); next = prev->next; + /* If tail_pos is first position in next, skip the chunk, otherwise, + * we must update the tail and, if fifo size is 0, even the head. + * We should not invalidate the tail for the caller and must not change + * consumer owned variables from code that's typically called by the + * producer */ + if (next->start_byte == tail_pos) + { + prev = next; + next = next->next; + } while (next != f->start_chunk) { cur = next; @@ -790,7 +827,11 @@ svm_fifo_enqueue (svm_fifo_t * f, u32 len, const u8 * src) /* collect out-of-order segments */ if (PREDICT_FALSE (f->ooos_list_head != OOO_SEGMENT_INVALID_INDEX)) - len += ooo_segment_try_collect (f, len, &tail); + { + len += ooo_segment_try_collect (f, len, &tail); + if (!svm_fifo_chunk_includes_pos (f->tail_chunk, tail)) + f->tail_chunk = svm_fifo_find_chunk (f, tail); + } /* store-rel: producer owned index (paired with load-acq in consumer) */ clib_atomic_store_rel_n (&f->tail, tail); @@ -847,6 +888,10 @@ svm_fifo_enqueue_nocopy (svm_fifo_t * f, u32 len) /* load-relaxed: producer owned index */ tail = f->tail; tail = (tail + len) % f->size; + + if (!svm_fifo_chunk_includes_pos (f->tail_chunk, tail)) + f->tail_chunk = svm_fifo_find_chunk (f, tail); + /* store-rel: producer owned index (paired with load-acq in consumer) */ clib_atomic_store_rel_n (&f->tail, tail); } @@ -919,6 +964,9 @@ svm_fifo_dequeue_drop (svm_fifo_t * f, u32 len) /* move head */ head = (head + total_drop_bytes) % f->size; + if (!svm_fifo_chunk_includes_pos (f->head_chunk, head)) + f->head_chunk = svm_fifo_find_chunk (f, head); + /* store-rel: consumer owned index (paired with load-acq in producer) */ clib_atomic_store_rel_n (&f->head, head); @@ -930,6 +978,10 @@ svm_fifo_dequeue_drop_all (svm_fifo_t * f) { /* consumer foreign index */ u32 tail = clib_atomic_load_acq_n (&f->tail); + + if (!svm_fifo_chunk_includes_pos (f->head_chunk, tail)) + f->head_chunk = svm_fifo_find_chunk (f, tail); + /* store-rel: consumer owned index (paired with load-acq in producer) */ clib_atomic_store_rel_n (&f->head, tail); } @@ -1055,6 +1107,45 @@ svm_fifo_del_subscriber (svm_fifo_t * f, u8 subscriber) } } +u8 +svm_fifo_is_sane (svm_fifo_t * f) +{ + if (f->size - 1 != f->nitems && !(f->flags & SVM_FIFO_F_SHRINK)) + return 0; + if (!svm_fifo_chunk_includes_pos (f->head_chunk, f->head)) + return 0; + if (!svm_fifo_chunk_includes_pos (f->tail_chunk, f->tail)) + return 0; + + if (f->start_chunk->next != f->start_chunk) + { + svm_fifo_chunk_t *c, *prev = 0, *tmp; + u32 size = 0; + + if (!(f->flags & SVM_FIFO_F_MULTI_CHUNK)) + return 0; + + c = f->start_chunk; + do + { + tmp = svm_fifo_find_chunk (f, c->start_byte); + if (tmp != c) + return 0; + if (prev && (prev->start_byte + prev->length != c->start_byte)) + return 0; + size += c->length; + prev = c; + c = c->next; + } + while (c != f->start_chunk); + + if (size != f->size) + return 0; + } + + return 1; +} + u8 * format_ooo_segment (u8 * s, va_list * args) { diff --git a/src/svm/svm_fifo.h b/src/svm/svm_fifo.h index 9b23d7e8e6f..d5dfc9c4aa3 100644 --- a/src/svm/svm_fifo.h +++ b/src/svm/svm_fifo.h @@ -252,9 +252,16 @@ svm_fifo_t *svm_fifo_create (u32 size); /** * Initialize fifo * + * @param f fifo * @param size size for fifo */ void svm_fifo_init (svm_fifo_t * f, u32 size); +/** + * Initialize fifo chunks and rbtree + * + * @param f fifo + */ +void svm_fifo_init_chunks (svm_fifo_t * f); /** * Allocate a fifo chunk on heap * @@ -456,13 +463,20 @@ void svm_fifo_del_subscriber (svm_fifo_t * f, u8 subscriber); * @return number of out of order segments */ u32 svm_fifo_n_ooo_segments (svm_fifo_t * f); -/* +/** * First out-of-order segment for fifo * * @param f fifo * @return first out-of-order segment for fifo */ ooo_segment_t *svm_fifo_first_ooo_segment (svm_fifo_t * f); +/** + * Check if fifo is sane. Debug only. + * + * @param f fifo + * @return 1 if sane, 0 otherwise + */ +u8 svm_fifo_is_sane (svm_fifo_t * f); format_function_t format_svm_fifo; /** -- cgit 1.2.3-korg