On Tue, 1 Sep 2026 06:43:31 +0000 Morten Brørup <[email protected]> wrote:
> Added a new "pile" mempool driver, based on the high-performance lock-free > "pile" stack variant. > > Changed other stack drivers to call their specific push/pop functions, > instead of calling the generic stack push/pop API. > > Signed-off-by: Morten Brørup <[email protected]> > --- FYI - pile driver is a good funny name https://en.wikipedia.org/wiki/Pile_driver Did a AI review with Fable 5.1 as experiment Review of [PATCH v4 0/2] stack/mempool: introduce pile Applied on top of d55ccd4 ("pci: remove deprecated catch-all flag"), built with -Dwerror=true, and ran stack_pile_autotest, stack_autotest and mempool_autotest; all pass on x86_64. Patch 1/2: stack: introduce pile Warning: the pile does not honour the capacity given to rte_stack_create(). Bulk and solo elements are separate resources (ceil(count / RTE_STACK_PILE_BULK_SIZE) bulk elements plus count solo elements), so the pile can hold objects in both pools at once. A pile created with count = 32 accepts 32 single-object pushes, then a further push of 32 succeeds, giving 64 objects stored while rte_stack_free_count() reports 0. Because count is rounded up to a whole number of bulk elements, a single push can also exceed capacity: a pile created with count = 40 accepts one push of 64. The RTE_MIN() in __rte_stack_pile_count() hides this rather than preventing it: return RTE_MIN((unsigned int)s->capacity, __rte_stack_lf_elems_count(&s->stack_pile.bulk) * RTE_STACK_PILE_BULK_SIZE + __rte_stack_lf_elems_count(&s->stack_pile.solo)); This is harmless for the mempool driver, which never holds more than mp->size objects, but RTE_STACK_F_PILE is a public flag on an API documented as a bounded stack, and callers that rely on push failing at capacity (as test_stack_basic() does) get different behaviour depending on how the objects were pushed. Either account for capacity explicitly, or document in the RTE_STACK_F_PILE Doxygen and in stack_lib.rst that the pile is only bounded when used through the mempool driver. Warning: the change to RTE_MEMPOOL_MAX_OPS_IDX in lib/mempool/rte_mempool.h is unrelated to the stack library, is not mentioned in the commit message, and is only needed by patch 2. After this series there are 17 RTE_MEMPOOL_REGISTER_OPS() sites in the tree, so without the bump the pile driver's constructor fails with -ENOSPC. Please move it into the mempool patch (or a separate preparatory mempool patch) and say why in the log. Warning: no release note. A new public stack flag and a new mempool driver both need an entry in doc/guides/rel_notes/release_26_11.rst. Info: the deterministic tests only cover bulk sizes 1 and MAX_BULK. The fragmentation path (__rte_stack_pile_pop_frag) and the bulk-pop retry path are only reached through the random-size multi-thread test, which does not check contents. A short sequence such as push 32 then pop 5, pop 27, and push 5 then push 32 then pop 37 would cover both paths with a known expected result. Info: in test_stack_push_pop() the "goto lifo" jumps backwards into the body of the preceding if block. It is legal C, but restructuring so that the LIFO check is a helper called from both branches would be easier to read. Patch 2/2: mempool: introduce pile driver Warning: the changes to rte_mempool_register_ops() and rte_mempool_set_ops_byname() (duplicate-name rejection, the strlen limit changing from >= to >, the new -ENAMETOOLONG return, log level changes) are unrelated to the pile driver and are not mentioned in the commit message. The strlen change also alters behaviour: names of exactly RTE_MEMPOOL_OPS_NAMESIZE - 1 characters were previously rejected and are now accepted. These belong in a separate mempool patch with their own explanation. Review-Result: WARNING
