Re: [PATCH net] net/mlx5e: Do not recycle pages from emergency reserve

On Sat, 2017-01-21 at 20:12 +0100, kernel netdev 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:
        > On Thu, Jan 19, 2017 at 9:03 AM, Eric Dumazet
        [off-list ref] wrote:
        >> From: Eric Dumazet [off-list ref]
        >>
        >> 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
        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
        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).


(Send from phone in car)


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.

I find this a bit strange. When you have time (ie not while driving your
car or during week end) please give more details, for example on message
size. Was it before skb_condense() was added ?

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.


--Jesper

On Sat, 21 Jan 2017 11:26:49 -0800
Eric Dumazet [off-list ref] wrote:

quoted

My previous measurements show approx 20℅ speedup on a UDP test with
delivery to remote CPU.
  

I find this a bit strange. When you have time (ie not while driving your
car or during week end) please give more details, for example on message
size.

I tested this with both 64 bytes and 1500 bytes.  After I moved to 50G
and 100G testing then I don't need to use 64 bytes packets to provoke
the bottlenecks in the stack ;-)

Was it before skb_condense() was added ?

It tested this just before skb_condense() was added.  BUT
skb_condense() does not get activated when using mlx5, because uses
build_skb() ie. not using frags.  

For people that don't realize this:
 Eric's optimization in skb_condense() is about trading remote CPU
 atomic refcnt (put_page) for copy + local CPU refcnt dec.

My measurements show cycles cost local=31 vs. remote=208, thus a
estimated saving around 177 cycles.  Which is spend on calling a fairly
complex function __pskb_pull_tail(), and only works for more complex
SKBs with frags.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

On Mon, Jan 23, 2017 at 11:14 AM, Jesper Dangaard Brouer
[off-list ref] wrote:

On Sat, 21 Jan 2017 11:26:49 -0800
Eric Dumazet [off-list ref] wrote:

quoted

quoted

My previous measurements show approx 20℅ speedup on a UDP test with
delivery to remote CPU.

I find this a bit strange. When you have time (ie not while driving your
car or during week end) please give more details, for example on message
size.

I tested this with both 64 bytes and 1500 bytes.  After I moved to 50G
and 100G testing then I don't need to use 64 bytes packets to provoke
the bottlenecks in the stack ;-)

Exactly! for XDP like uses cases, page cache maybe a non required optimization.
but when you start testing a typical TCP use cases over 50/100G link
you will need
more buffers (pages) to host the traffic for longer periods, you will
hit that bottleneck.

quoted

Was it before skb_condense() was added ?

It tested this just before skb_condense() was added.  BUT
skb_condense() does not get activated when using mlx5, because uses
build_skb() ie. not using frags.

Well, we can always replace build_skb with alloc_skb +
memcpy(skb->data, headlen) + add_skb_frag(payload)
does it it worth it ? and is it healthy that both skb->data and
skb_shinfo(skb)->frags[i] point to the same page ?

For people that don't realize this:
 Eric's optimization in skb_condense() is about trading remote CPU
 atomic refcnt (put_page) for copy + local CPU refcnt dec.

My measurements show cycles cost local=31 vs. remote=208, thus a
estimated saving around 177 cycles.  Which is spend on calling a fairly
complex function __pskb_pull_tail(), and only works for more complex
SKBs with frags.

--
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer