On Sat, Jan 21, 2017 at 9:12 PM, kernel netdev [off-list ref] wrote:
Den 21. jan. 2017 7.10 PM skrev "Tom Herbert" [off-list ref]:
On Thu, Jan 19, 2017 at 11:14 AM, Saeed Mahameed
[off-list ref] wrote:
quoted
On Thu, Jan 19, 2017 at 9:03 AM, Eric Dumazet [off-list ref]
wrote:
quoted
From: Eric Dumazet <edumazet@google.com>
A driver using dev_alloc_page() must not reuse a page allocated from
emergency memory reserve.
Otherwise all packets using this page will be immediately dropped,
unless for very specific sockets having SOCK_MEMALLOC bit set.
This issue might be hard to debug, because only a fraction of received
packets would be dropped.
Hi Eric,
When you say reuse, you mean point to the same page from several SKBs ?
Because in our page cache implementation we don't reuse pages that
already passed to the stack,
we just keep them in the page cache until the ref count drop back to
one, so we recycle them (i,e they will be re-used only when no one
else is using them).
Saeed,
Speaking of the mlx page cache can we remove this or a least make it
optional to use. It is another example of complex functionality being
put into drivers that makes things like backports more complicated and
Re complexity, I am not sure the mlx page cache is that complex,
we just wrap alloc_page/put_page with our own page cache calls.
Roughly the page cache implementation is 200-300 LOC tops all concentrated
in one place in the code.
provide at best some marginal value. In the case of the mlx5e cache
code the results from pktgen really weren't very impressive in the
first place. Also, the cache suffers from HOL blocking where we can
Well, with pktgen you won't notice a huge improvement since the pages are freed
in the stack directly from our rx receive handler (gro_receive), those
pages will go back to the page allocator and get requested immediately
again from the driver (no stress).
The real improvements are seen when you really stress the page allocator with
real uses cases such as TCP/UDP with user applications, where the
pages are held longer than the driver needs, the page cache in this
case will play an important role of reducing the stress
on the page allocater since with those use cases we are juggling with
more pages than pktgen could.
With more stress (#cores TCP streams) our humble page cache some times
can't hold ! and it will get full fast enough and will fail to recycle
for a huge percentage of the driver pages requests.
Before our own page cache we used dev_alloc_skb which used its own
cache "page_frag_cache" and it worked nice enough. So i don't really
recommend removing the page cache, until we have
a generic RX page cache solution for all device drivers.
block the whole cache due to an outstanding reference on just one page
(something that you wouldn't see in pktgen but is likely to happen in
real applications).
Re the HOL issue, we have some upcoming patches that would drastically
improve the HOL blocking issue (we will simple swap the HOL on every
sample).
(Send from phone in car)
Driver Safe :) ..
To Tom, have you measured the effect of this page cache? Before claiming it
is ineffective.
My previous measurements show approx 20℅ speedup on a UDP test with delivery
to remote CPU.
Removing the cache would of cause be a good usecase for speeding up the page
allocator (PCP). Which Mel Gorman and me are working on. AFAIK current page
order0 cost 240 cycles. Mel have reduced til to 180, and without NUMA 150
cycles. And with bulking this can be amortized to 80 cycles.
Are you trying to say that we won't need the cache if you manage to
deliver those optimizations ?
can you compare those optimizations with the page_frag_cache from
dev_alloc_skb ?
--Jesper
On Sat, Jan 21, 2017 at 12:31 PM, Saeed Mahameed
[off-list ref] wrote:
On Sat, Jan 21, 2017 at 9:12 PM, kernel netdev [off-list ref] wrote:
quoted
Den 21. jan. 2017 7.10 PM skrev "Tom Herbert" [off-list ref]:
On Thu, Jan 19, 2017 at 11:14 AM, Saeed Mahameed
[off-list ref] wrote:
quoted
On Thu, Jan 19, 2017 at 9:03 AM, Eric Dumazet [off-list ref]
wrote:
quoted
From: Eric Dumazet <edumazet@google.com>
A driver using dev_alloc_page() must not reuse a page allocated from
emergency memory reserve.
Otherwise all packets using this page will be immediately dropped,
unless for very specific sockets having SOCK_MEMALLOC bit set.
This issue might be hard to debug, because only a fraction of received
packets would be dropped.
Hi Eric,
When you say reuse, you mean point to the same page from several SKBs ?
Because in our page cache implementation we don't reuse pages that
already passed to the stack,
we just keep them in the page cache until the ref count drop back to
one, so we recycle them (i,e they will be re-used only when no one
else is using them).
Saeed,
Speaking of the mlx page cache can we remove this or a least make it
optional to use. It is another example of complex functionality being
put into drivers that makes things like backports more complicated and
Re complexity, I am not sure the mlx page cache is that complex,
we just wrap alloc_page/put_page with our own page cache calls.
Roughly the page cache implementation is 200-300 LOC tops all concentrated
in one place in the code.
Taken as part of the RX buffer management code the whole thing in very
complicated and seems to be completely bereft of any comments in the
code as to how things are supposed to work.
quoted
provide at best some marginal value. In the case of the mlx5e cache
code the results from pktgen really weren't very impressive in the
first place. Also, the cache suffers from HOL blocking where we can
Well, with pktgen you won't notice a huge improvement since the pages are freed
in the stack directly from our rx receive handler (gro_receive), those
pages will go back to the page allocator and get requested immediately
again from the driver (no stress).
The real improvements are seen when you really stress the page allocator with
real uses cases such as TCP/UDP with user applications, where the
pages are held longer than the driver needs, the page cache in this
case will play an important role of reducing the stress
on the page allocater since with those use cases we are juggling with
more pages than pktgen could.
With more stress (#cores TCP streams) our humble page cache some times
can't hold ! and it will get full fast enough and will fail to recycle
for a huge percentage of the driver pages requests.
Before our own page cache we used dev_alloc_skb which used its own
cache "page_frag_cache" and it worked nice enough. So i don't really
recommend removing the page cache, until we have
a generic RX page cache solution for all device drivers.
quoted
block the whole cache due to an outstanding reference on just one page
(something that you wouldn't see in pktgen but is likely to happen in
real applications).
Re the HOL issue, we have some upcoming patches that would drastically
improve the HOL blocking issue (we will simple swap the HOL on every
sample).
quoted
(Send from phone in car)
Driver Safe :) ..
quoted
To Tom, have you measured the effect of this page cache? Before claiming it
is ineffective.
My previous measurements show approx 20℅ speedup on a UDP test with delivery
to remote CPU.
Removing the cache would of cause be a good usecase for speeding up the page
allocator (PCP). Which Mel Gorman and me are working on. AFAIK current page
order0 cost 240 cycles. Mel have reduced til to 180, and without NUMA 150
cycles. And with bulking this can be amortized to 80 cycles.
Are you trying to say that we won't need the cache if you manage to
deliver those optimizations ?
can you compare those optimizations with the page_frag_cache from
dev_alloc_skb ?
quoted
--Jesper
On Sat, 21 Jan 2017 22:31:26 +0200 Saeed Mahameed [off-list ref] wrote:
quoted
My previous measurements show approx 20℅ speedup on a UDP test with delivery
to remote CPU.
Removing the cache would of cause be a good usecase for speeding up the page
allocator (PCP). Which Mel Gorman and me are working on. AFAIK current page
order0 cost 240 cycles. Mel have reduced til to 180, and without NUMA 150
cycles. And with bulking this can be amortized to 80 cycles.
Are you trying to say that we won't need the cache if you manage to
deliver those optimizations ?
These page alloc optimization are good, but not fast-enough to replace
your cache. Maybe I can improve it further, but it will be very hard
to compete with a recycle cache (it can skip many checks the page alloc
need, as it knows the page state).
Said in another way, the gap will be significantly smaller, and you
will not see a big boost from using this cache.
BUT there are other advantages of using a guaranteed recycle pool
facility (like the page_pool). Namely, (1) DMA-overhead: keeping page
DMA mapped to counter DMA+IOMMU overhead, (2) RX-zero-copy: opens up
for a software memory model solution for pre-VMA-mapping pages to
userspace (See: [1] for argument how this avoids leaking kernel mem,
but only expose/leak packet-data mem)
can you compare those optimizations with the page_frag_cache from
dev_alloc_skb ?
IHMO the page_frag_cache is indirectly a bulk page allocator facility,
which is limiting the number of times the page allocator is called, and
thus amortize the cost of talking to the page allocator. Performance
wise, it is likely comparable to your page cache, and likely faster
than the 80 cycles order0-bulking.
BUT we generally worry about using these 32K pages, as it opens a
window for attackers pinning down memory.
[1] https://prototype-kernel.readthedocs.io/en/latest/vm/page_pool/design/memory_model_nic.html
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
From: Jesper Dangaard Brouer <redacted>
Date: Mon, 23 Jan 2017 09:39:40 +0100
BUT there are other advantages of using a guaranteed recycle pool
facility (like the page_pool). Namely, (1) DMA-overhead: keeping page
DMA mapped to counter DMA+IOMMU overhead, (2) RX-zero-copy: opens up
for a software memory model solution for pre-VMA-mapping pages to
userspace (See: [1] for argument how this avoids leaking kernel mem,
but only expose/leak packet-data mem)
+1
On Sun, Jan 22, 2017 at 7:50 PM, Tom Herbert [off-list ref] wrote:
On Sat, Jan 21, 2017 at 12:31 PM, Saeed Mahameed
[off-list ref] wrote:
quoted
On Sat, Jan 21, 2017 at 9:12 PM, kernel netdev [off-list ref] wrote:
quoted
Den 21. jan. 2017 7.10 PM skrev "Tom Herbert" [off-list ref]:
On Thu, Jan 19, 2017 at 11:14 AM, Saeed Mahameed
[off-list ref] wrote:
quoted
On Thu, Jan 19, 2017 at 9:03 AM, Eric Dumazet [off-list ref]
wrote:
quoted
From: Eric Dumazet <edumazet@google.com>
A driver using dev_alloc_page() must not reuse a page allocated from
emergency memory reserve.
Otherwise all packets using this page will be immediately dropped,
unless for very specific sockets having SOCK_MEMALLOC bit set.
This issue might be hard to debug, because only a fraction of received
packets would be dropped.
Hi Eric,
When you say reuse, you mean point to the same page from several SKBs ?
Because in our page cache implementation we don't reuse pages that
already passed to the stack,
we just keep them in the page cache until the ref count drop back to
one, so we recycle them (i,e they will be re-used only when no one
else is using them).
Saeed,
Speaking of the mlx page cache can we remove this or a least make it
optional to use. It is another example of complex functionality being
put into drivers that makes things like backports more complicated and
Re complexity, I am not sure the mlx page cache is that complex,
we just wrap alloc_page/put_page with our own page cache calls.
Roughly the page cache implementation is 200-300 LOC tops all concentrated
in one place in the code.
Taken as part of the RX buffer management code the whole thing in very
complicated and seems to be completely bereft of any comments in the
code as to how things are supposed to work.
The Idea is fairly simple, maybe we could have made it cleaner if we
better decoupled the page
cache from the rx logic.
The page cache works in parallel to the rx logic, it takes its owns ref counts,
this way the rx logic will continue working as if there was no cache
(get/put_pages).
but instead of of the kernel (get/put_pages) it will call the internal
cache get/put_cache_page.
Yeah I agree, some comments could help. We will work on this when we
fix the HOL issues.