All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: "Morten Brørup" <mb@smartsharesystems.com>
Cc: dev@dpdk.org
Subject: Re: [RFC PATCH v10] pile stack and mempool driver
Date: Mon, 10 Aug 2026 09:15:05 -0700	[thread overview]
Message-ID: <20260810091505.264a248e@phoenix.local> (raw)
In-Reply-To: <20260810151214.1606363-1-mb@smartsharesystems.com>

On Mon, 10 Aug 2026 15:12:14 +0000
Morten Brørup <mb@smartsharesystems.com> wrote:

> Early submission of:
> - A new "pile" stack-like implementation using the Stack API, and
> - an accompanying "pile" mempool driver.
> And:
> - Some mempool optimizations.
> 
> For CI test and community feedback.
> 
> Needless to say, this must be separated into multiple independent
> series of patches.
> And release notes must be added.
> For now, I'm submitting a snapshot of work in progress.
> 
> The "pile" somewhat resembles the lock-free stack, but operates on
> bulks (arrays) of objects, to significantly reduce linked list
> traversal.
> With the pile's default bulk size of 32 objects, a mempool cache
> flush/refill traverses a linked list of only 16 elements, whereas
> the lock-free stack would traverse a linked list of 512 elements.
> 
> Some performance numbers from mempool_perf_autotest_2cores, all
> with cache=1024 cores=2 n_keep=32768:
> 
> start performance test (using ring_mp_mc, with cache)
> n_get_bulk= 64 n_put_bulk= 64 constant_n=0 rate_persec= 753985338
> n_get_bulk=256 n_put_bulk=256 constant_n=0 rate_persec= 755805913
> 
> start performance test for lf_stack (with cache)
> n_get_bulk= 64 n_put_bulk= 64 constant_n=0 rate_persec=  29132352
> n_get_bulk=256 n_put_bulk=256 constant_n=0 rate_persec=  29276708
> 
> start performance test for pile (with cache)
> n_get_bulk= 64 n_put_bulk= 64 constant_n=0 rate_persec= 560159479
> n_get_bulk=256 n_put_bulk=256 constant_n=0 rate_persec= 557910933
> 
> Hat tip to Bruce for bringing attention to the ring not being the
> optimal mempool driver!
> 
> Note: The GitHub "mini" tests don't include the "pile" mempool driver,
> and are expected to fail.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---

AI review with Claude Fable showed:

Applied to main (c1a46b9), clean build with gcc 13.3 and -Dwerror=true.
No remaining references to the removed flushthresh field or the
RTE_MEMPOOL_HEADER_SIZE macro anywhere in the tree. Ran the stack,
stack_lf, stack_pile and mempool autotests plus a standalone harness
exercising the pile push/pop paths directly.

Errors:

1. test_stack.c: the pile fill loop in test_stack_basic() assumes an
   effective capacity of RTE_STACK_PILE_BULK_SIZE * STACK_SIZE, but the
   pile only has ceil(count / 32) free bulk elements, i.e. exactly
   count objects via the bulk path. The first push(STACK_SIZE) consumes
   all 2048 bulk elements; the second iteration fails. Confirmed at
   runtime:

       [test_stack_basic():193] Fill objects push failed

   The drain loop below it has the mirrored assumption. The loop bound
   should reflect the actual capacity model (one full-bulk fill, or
   fill bulk and solo separately).

2. test_stack.c: the failure above is masked -- the test still reports
   "Test OK". In all four new branches ("All objects push failed",
   "Fill objects push failed", "All objects pop failed", "Drain objects
   pop failed"), ret holds the return value of the failed push/pop,
   which is 0, and "goto fail_test" then does "return ret;", returning
   0 = success to __test_stack(). Each of these branches needs
   "ret = -1;" before the goto. Confirmed at runtime: the failure
   message prints, then the suite reports Test OK. This is presumably
   why CI never surfaced error 1.

Warnings:

3. Pile capacity contract: a pile created with count N accepts up to
   2*N objects (N via bulk elements plus N via solo elements). Verified
   empirically: after a full-bulk fill of 65536, another 65536 solo
   pushes were accepted, while rte_stack_count() clamps at s->capacity
   and under-reports the real content. Either size the solo free list
   so total capacity is N, or document the 2x behavior; the current
   state makes count/free_count inconsistent for raw stack API users.
   (Not reachable through mempool usage, where the object total is
   conserved.)

4. Cache sizes 1..31 now fail pool creation outright: rounding down to
   a multiple of 32 yields 0 and EINVAL. Verified:
   rte_mempool_create(cache=4) and rte_mempool_cache_create(4) both
   fail; 250 is silently rounded to 224 with only a DEBUG log. This
   breaks existing applications at runtime (the in-tree tap driver
   needed fixing from 4 to 32; out-of-tree apps get no warning).
   Consider rounding up for small sizes instead of failing, and this
   needs a prominent release notes entry either way. The rte_mempool.h
   doc says cache_size "must be divisible by 32" while the code accepts
   and rounds -- doc and behavior should agree.

5. struct rte_mempool now embeds local_cache[RTE_MAX_LCORE]
   unconditionally, so cache-less (cache_size=0) mempools grow by
   RTE_MAX_LCORE * sizeof(struct rte_mempool_cache) -- roughly 1 MB of
   memzone per pool at the default RTE_MAX_LCORE=128 with the enlarged
   objs[1024] array plus cache guard. Applications creating many small
   cache-less pools pay this in hugepage memory. Deliberate trade-off
   for removing the hot-path branch, but it should be called out in the
   release notes.

6. tap: raising TAP_GSO_MBUF_CACHE_SIZE from 4 to 32 also multiplies
   TAP_GSO_MBUFS_NUM (defined as PER_CORE * CACHE_SIZE) from 512 to
   4096 mbufs, an 8x larger GSO pool. If the change is only to satisfy
   the divisibility constraint, decouple MBUFS_NUM from CACHE_SIZE.

7. The two FIXME test-only settings (RTE_MBUF_DEFAULT_MEMPOOL_OPS
   "pile", RTE_USE_C11_MEM_MODEL in config/x86/meson.build) must be
   reverted before merge, as the FIXMEs already note.

Info:

8. RTE_MEMPOOL_DRIVER_REPRESENTOR_MZ_PREFIX: "representor" collides
   with the port-representor concept; "representative" or "worst-case"
   would read better.

9. rte_mempool_register_ops(): the old name-too-long path set rte_errno
   before returning; the new -ENAMETOOLONG and -EEXIST paths return the
   negative errno without setting rte_errno. Consistent with the other
   error returns in this function, but a small change in error
   reporting.

10. test_stack_push_pop() skips content verification entirely when
    bulk_sz >= RTE_STACK_PILE_BULK_SIZE but not a multiple of it.
    Latent (only 1 and MAX_BULK=512 are used today), but worth a
    comment or coverage.

Runtime verification that passed: the fragmentation pop path (push 64,
pop 40 as 1 bulk + 8 via a fragmented bulk element, then pop 24)
conserves all objects with no duplication or loss; the retry path with
odd sizes (push 33+31+32, pop 96) likewise; bulk-level LIFO ordering
matches the memcmp formula in the test; the pile mempool handler passes
populate/get/put/audit and a 1000-iteration uneven get/put churn
through the per-lcore cache; mempool_autotest (which now defaults to
pile) and the std/lf stack autotests all pass. The rounded-up
rte_memcpy in rte_mempool_do_generic_put stays within objs[] bounds
given the divisible-by-32 cache size enforced at creation.

  reply	other threads:[~2026-08-10 16:15 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-01  7:15 [RFC PATCH] NEW: pile stack and mempool driver Morten Brørup
2026-08-01 10:09 ` [RFC PATCH v2] " Morten Brørup
2026-08-01 10:17 ` [RFC PATCH v3] " Morten Brørup
2026-08-01 11:13 ` [RFC PATCH v4] " Morten Brørup
2026-08-02  7:24 ` [RFC PATCH v5] " Morten Brørup
2026-08-02  9:59 ` [RFC PATCH v6] " Morten Brørup
2026-08-02 15:22   ` Stephen Hemminger
2026-08-02 16:45     ` Morten Brørup
2026-08-03  8:14 ` [RFC PATCH v7] " Morten Brørup
2026-08-03 23:02   ` Stephen Hemminger
2026-08-10  9:02 ` [RFC PATCH v8] " Morten Brørup
2026-08-10 13:58 ` [RFC PATCH v9] " Morten Brørup
2026-08-10 15:12 ` [RFC PATCH v10] " Morten Brørup
2026-08-10 16:15   ` Stephen Hemminger [this message]
2026-08-10 18:36   ` Morten Brørup
2026-08-11 12:35 ` [RFC PATCH v11] " Morten Brørup
2026-08-11 15:25   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260810091505.264a248e@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=mb@smartsharesystems.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.