Thanks Jesper, On Wed, 4 Jun 2025 at 11:39, Jesper Dangaard Brouer <h...@kernel.org> wrote: > > > > On 28/05/2025 21.46, Mina Almasry wrote: > > On Wed, May 28, 2025 at 2:28 AM Toke Høiland-Jørgensen <t...@redhat.com> > > wrote: > >> > >> Mina Almasry <almasrym...@google.com> writes: > >> > >>> On Mon, May 26, 2025 at 5:51 AM Toke Høiland-Jørgensen <t...@redhat.com> > >>> wrote: > >>>>> Fast path results: > >>>>> no-softirq-page_pool01 Per elem: 11 cycles(tsc) 4.368 ns > >>>>> > >>>>> ptr_ring results: > >>>>> no-softirq-page_pool02 Per elem: 527 cycles(tsc) 195.187 ns > >>>>> > >>>>> slow path results: > >>>>> no-softirq-page_pool03 Per elem: 549 cycles(tsc) 203.466 ns > >>>>> ``` > >>>>> > >>>>> Cc: Jesper Dangaard Brouer <h...@kernel.org> > >>>>> Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org> > >>>>> Cc: Jakub Kicinski <k...@kernel.org> > >>>>> Cc: Toke Høiland-Jørgensen <t...@toke.dk> > >>>>> > >>>>> Signed-off-by: Mina Almasry <almasrym...@google.com> > >>>> > >>>> Back when you posted the first RFC, Jesper and I chatted about ways to > >>>> avoid the ugly "load module and read the output from dmesg" interface to > >>>> the test. > >>>> > >>> > >>> I agree the existing interface is ugly. > >>> > >>>> One idea we came up with was to make the module include only the "inner" > >>>> functions for the benchmark, and expose those to BPF as kfuncs. Then the > >>>> test runner can be a BPF program that runs the tests, collects the data > >>>> and passes it to userspace via maps or a ringbuffer or something. That's > >>>> a nicer and more customisable interface than the printk output. And if > >>>> they're small enough, maybe we could even include the functions into the > >>>> page_pool code itself, instead of in a separate benchmark module? > >>>> > >>>> WDYT of that idea? :) > >>> > >>> ...but this sounds like an enormous amount of effort, for something > >>> that is a bit ugly but isn't THAT bad. Especially for me, I'm not that > >>> much of an expert that I know how to implement what you're referring > >>> to off the top of my head. I normally am open to spending time but > >>> this is not that high on my todolist and I have limited bandwidth to > >>> resolve this :( > >>> > >>> I also feel that this is something that could be improved post merge. > >>> I think it's very beneficial to have this merged in some form that can > >>> be improved later. Byungchul is making a lot of changes to these mm > >>> things and it would be nice to have an easy way to run the benchmark > >>> in tree and maybe even get automated results from nipa. If we could > >>> agree on mvp that is appropriate to merge without too much scope creep > >>> that would be ideal from my side at least. > >> > >> Right, fair. I guess we can merge it as-is, and then investigate whether > >> we can move it to BPF-based (or maybe 'perf bench' - Cc acme) later :) > > > > Thanks for the pliability. Reviewed-bys and comments welcome. > > > > Additionally Signed-off-by from Jesper is needed I think. Since most > > of this code is his, I retained his authorship. Jesper, whenever this > > looks good to me, a signed-off-by would be good and I would carry it > > to future versions. Changing authorship to me is also fine by me but I > > would think you want to retain the credit. > > Okay, I think Ilias'es comment[1] and ACK convinced me, let us merge > this as-is. We have been asking people to run it over several years > before accepting patches. We shouldn't be pointing people to use > out-of-tree tests for accepting patches. > > It is not perfect, but it have served us well for benchmarking in the > last approx 10 years (5 years for page_pool test). It is isolated as a > selftest under (tools/testing/selftests/net/bench/page_pool/). > > Realistically we are all too busy inventing a new "perfect" benchmark > for page_pool. That said, I do encourage others with free cycles to > integrated a better benchmark test into `perf bench`. Then we can just > remove this module again.
I'll spend some time looking at acme comments. They seem to be moving towards the right direction Thanks /Ilias > > Signed-off-by: Jesper Dangaard Brouer <h...@kernel.org> > > [1] > https://lore.kernel.org/all/cac_iwjlmo4xz_+pbacnxpvctmgknbslgyeeks2ptrrepn1u...@mail.gmail.com/ > > Thanks Mina for pushing this forward, > --Jesper
