Re: [PATCH RFC net-next v2] page_pool: import Jesper's page_pool benchmark

[Jesper Dangaard Brouer <hawk@kernel.org>]

On 28/05/2025 21.46, Mina Almasry wrote:
> On Wed, May 28, 2025 at 2:28 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Mina Almasry <almasrymina@google.com> writes:
>>
>>> On Mon, May 26, 2025 at 5:51 AM Toke Høiland-Jørgensen <toke@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 <hawk@kernel.org>
>>>>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>>>> Cc: Jakub Kicinski <kuba@kernel.org>
>>>>> Cc: Toke Høiland-Jørgensen <toke@toke.dk>
>>>>>
>>>>> Signed-off-by: Mina Almasry <almasrymina@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.

Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>

  [1] 
https://lore.kernel.org/all/CAC_iWjLmO4XZ_+PBaCNxpVCTmGKNBsLGyeeKS2ptRrepn1u0SQ@mail.gmail.com/

Thanks Mina for pushing this forward,
--Jesper