Message-ID: <9116f069-63a7-4cc9-b197-1f39ebfd0a57@gmail.com>
Date: Tue, 10 Sep 2024 13:31:56 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Yunsheng Lin <linyunsheng@...wei.com>,
 Mina Almasry <almasrymina@...gle.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-alpha@...r.kernel.org,
 linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
 sparclinux@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-arch@...r.kernel.org, bpf@...r.kernel.org,
 linux-kselftest@...r.kernel.org, linux-media@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, Donald Hunter <donald.hunter@...il.com>,
 Jakub Kicinski <kuba@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Jonathan Corbet <corbet@....net>,
 Richard Henderson <richard.henderson@...aro.org>,
 Ivan Kokshaysky <ink@...assic.park.msu.ru>, Matt Turner
 <mattst88@...il.com>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 "James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
 Helge Deller <deller@....de>, Andreas Larsson <andreas@...sler.com>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 Ilias Apalodimas <ilias.apalodimas@...aro.org>,
 Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Arnd Bergmann <arnd@...db.de>,
 Steffen Klassert <steffen.klassert@...unet.com>,
 Herbert Xu <herbert@...dor.apana.org.au>, David Ahern <dsahern@...nel.org>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Björn Töpel <bjorn@...nel.org>,
 Magnus Karlsson <magnus.karlsson@...el.com>,
 Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
 Jonathan Lemon <jonathan.lemon@...il.com>, Shuah Khan <shuah@...nel.org>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Sumit Semwal <sumit.semwal@...aro.org>,
 Christian König <christian.koenig@....com>,
 David Wei <dw@...idwei.uk>, Jason Gunthorpe <jgg@...pe.ca>,
 Shailend Chand <shailend@...gle.com>,
 Harshitha Ramamurthy <hramamurthy@...gle.com>,
 Shakeel Butt <shakeel.butt@...ux.dev>, Jeroen de Borst
 <jeroendb@...gle.com>, Praveen Kaligineedi <pkaligineedi@...gle.com>,
 Bagas Sanjaya <bagasdotme@...il.com>, Christoph Hellwig <hch@...radead.org>,
 Nikolay Aleksandrov <razor@...ckwall.org>, Taehee Yoo <ap420073@...il.com>
Subject: Re: [PATCH net-next v25 00/13] Device Memory TCP

On 9/10/24 11:44, Yunsheng Lin wrote:
> On 2024/9/10 0:54, Mina Almasry wrote:
>> On Mon, Sep 9, 2024 at 4:21 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
>>>
>>> On 2024/9/9 13:43, Mina Almasry wrote:
>>>
>>>>
>>>> Perf - page-pool benchmark:
>>>> ---------------------------
>>>>
>>>> bench_page_pool_simple.ko tests with and without these changes:
>>>> https://pastebin.com/raw/ncHDwAbn
>>>>
>>>> AFAIK the number that really matters in the perf tests is the
>>>> 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8
>>>> cycles without the changes but there is some 1 cycle noise in some
>>>> results.
>>>>
>>>> With the patches this regresses to 9 cycles with the changes but there
>>>> is 1 cycle noise occasionally running this test repeatedly.
>>>>
>>>> Lastly I tried disable the static_branch_unlikely() in
>>>> netmem_is_net_iov() check. To my surprise disabling the
>>>> static_branch_unlikely() check reduces the fast path back to 8 cycles,
>>>> but the 1 cycle noise remains.
>>>
>>> Sorry for the late report, as I was adding a testing page_pool ko basing
>>> on [1] to avoid introducing performance regression when fixing the bug in
>>> [2].
>>> I used it to test the performance impact of devmem patchset for page_pool
>>> too, it seems there might be some noticable performance impact quite stably
>>> for the below testcases, about 5%~16% performance degradation as below in
>>> the arm64 system:
>>>
>>
>> Correct me if I'm wrong here, but on the surface here it seems that
>> you're re-reporting a known issue. Consensus seems to be that it's a
>> non-issue.
>>
>> In v6 I reported that the bench_page_pool_simple.ko test reports a 1
>> cycle regression with these patches, from 8->9 cycles. That is roughly
>> consistent with the 5-15% you're reporting.
> 
>  From the description above in the cover letter, I thought the performance
> data using the out of tree testing ko is not stable enough to justify the
> performance impact.
> 
>>
>> I root caused the reason for the regression to be the
>> netmem_is_net_iov() check in the fast path. I removed this regression
>> in v7 (see the change log) by conditionally compiling the check in
>> that function.
>>
>> In v8, Pavel/Jens/David pushed back on the ifdef check. See this
>> entire thread, but in particular this response from Jens:
> 
> It seemed the main objection is about how to enable this feature
> for the io_uring?

The pushback was that config checks as optimisation don't work in real
life, they inevitably get enabled everywhere but some niche cases.
io_uring could do another config for memory providers, but even if it's
not enabled by default (which is not a great option), distributions will
eventually turn it on.

So, if you have that "niche use case" that fully controls the kernel and
wants to shed this overhead, we can do a config structure, but if it's
about overhead for everyone in general, configs hardly help anything,
even without any io_uring in the picture.

> And it seemed that you had added the CONFIG_NET_DEVMEM for this
> devmem thing, why not use it for that?

-- 
Pavel Begunkov

Powered by blists - more mailing lists