From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Pawe=c5=82_Staszewski?= Subject: Re: Kernel 4.19 network performance - forwarding/routing normal users traffic Date: Fri, 2 Nov 2018 20:02:12 +0100 Message-ID: <08828f13-1e47-e65b-caa8-2319167fc495@itcare.pl> References: <61697e49-e839-befc-8330-fc00187c48ee@itcare.pl> <61e30474-b5e9-4dc8-a8a6-90cdd17d2a66@gmail.com> <8e10bf68-f3b3-98f2-91a5-25b151756dd6@itcare.pl> <20181101102213.2fa2643d@redhat.com> <20181101152716.GA13895@intel.com> <20181102052356.GA17587@intel.com> <20181102124037.352b15de@redhat.com> <20181102142024.GA18343@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: Saeed Mahameed , "eric.dumazet@gmail.com" , "netdev@vger.kernel.org" , Tariq Toukan , "ilias.apalodimas@linaro.org" , "yoel@kviknet.dk" , "mgorman@techsingularity.net" To: Aaron Lu , Jesper Dangaard Brouer Return-path: Received: from smtp7.iq.pl ([86.111.240.244]:38090 "EHLO smtp7.iq.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729898AbeKCEKh (ORCPT ); Sat, 3 Nov 2018 00:10:37 -0400 In-Reply-To: <20181102142024.GA18343@intel.com> Content-Language: pl Sender: netdev-owner@vger.kernel.org List-ID: W dniu 02.11.2018 o 15:20, Aaron Lu pisze: > On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer wrote: >> On Fri, 2 Nov 2018 13:23:56 +0800 >> Aaron Lu wrote: >> >>> On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote: >>>> On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote: >>>>> On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer >>>>> wrote: >>>>> ... ... >>>>>> Section copied out: >>>>>> >>>>>> mlx5e_poll_tx_cq >>>>>> | >>>>>> --16.34%--napi_consume_skb >>>>>> | >>>>>> |--12.65%--__free_pages_ok >>>>>> | | >>>>>> | --11.86%--free_one_page >>>>>> | | >>>>>> | |--10.10% >>>>>> --queued_spin_lock_slowpath >>>>>> | | >>>>>> | --0.65%--_raw_spin_lock >>>>> This callchain looks like it is freeing higher order pages than order >>>>> 0: >>>>> __free_pages_ok is only called for pages whose order are bigger than >>>>> 0. >>>> mlx5 rx uses only order 0 pages, so i don't know where these high order >>>> tx SKBs are coming from.. >>> Perhaps here: >>> __netdev_alloc_skb(), __napi_alloc_skb(), __netdev_alloc_frag() and >>> __napi_alloc_frag() will all call page_frag_alloc(), which will use >>> __page_frag_cache_refill() to get an order 3 page if possible, or fall >>> back to an order 0 page if order 3 page is not available. >>> >>> I'm not sure if your workload will use the above code path though. >> TL;DR: this is order-0 pages (code-walk trough proof below) >> >> To Aaron, the network stack *can* call __free_pages_ok() with order-0 >> pages, via: >> >> static void skb_free_head(struct sk_buff *skb) >> { >> unsigned char *head = skb->head; >> >> if (skb->head_frag) >> skb_free_frag(head); >> else >> kfree(head); >> } >> >> static inline void skb_free_frag(void *addr) >> { >> page_frag_free(addr); >> } >> >> /* >> * Frees a page fragment allocated out of either a compound or order 0 page. >> */ >> void page_frag_free(void *addr) >> { >> struct page *page = virt_to_head_page(addr); >> >> if (unlikely(put_page_testzero(page))) >> __free_pages_ok(page, compound_order(page)); >> } >> EXPORT_SYMBOL(page_frag_free); > I think here is a problem - order 0 pages are freed directly to buddy, > bypassing per-cpu-pages. This might be the reason lock contention > appeared on free path. Can someone apply below diff and see if lock > contention is gone? Will test it tonight > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index e2ef1c17942f..65c0ae13215a 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr) > { > struct page *page = virt_to_head_page(addr); > > - if (unlikely(put_page_testzero(page))) > - __free_pages_ok(page, compound_order(page)); > + if (unlikely(put_page_testzero(page))) { > + unsigned int order = compound_order(page); > + > + if (order == 0) > + free_unref_page(page); > + else > + __free_pages_ok(page, order); > + } > } > EXPORT_SYMBOL(page_frag_free); > >> Notice for the mlx5 driver it support several RX-memory models, so it >> can be hard to follow, but from the perf report output we can see that >> is uses mlx5e_skb_from_cqe_linear, which use build_skb. >> >> --13.63%--mlx5e_skb_from_cqe_linear >> | >> --5.02%--build_skb >> | >> --1.85%--__build_skb >> | >> --1.00%--kmem_cache_alloc >> >> /* build_skb() is wrapper over __build_skb(), that specifically >> * takes care of skb->head and skb->pfmemalloc >> * This means that if @frag_size is not zero, then @data must be backed >> * by a page fragment, not kmalloc() or vmalloc() >> */ >> struct sk_buff *build_skb(void *data, unsigned int frag_size) >> { >> struct sk_buff *skb = __build_skb(data, frag_size); >> >> if (skb && frag_size) { >> skb->head_frag = 1; >> if (page_is_pfmemalloc(virt_to_head_page(data))) >> skb->pfmemalloc = 1; >> } >> return skb; >> } >> EXPORT_SYMBOL(build_skb); >> >> It still doesn't prove, that the @data is backed by by a order-0 page. >> For the mlx5 driver is uses mlx5e_page_alloc_mapped -> >> page_pool_dev_alloc_pages(), and I can see perf report using >> __page_pool_alloc_pages_slow(). >> >> The setup for page_pool in mlx5 uses order=0. >> >> /* Create a page_pool and register it with rxq */ >> pp_params.order = 0; >> pp_params.flags = 0; /* No-internal DMA mapping in page_pool */ >> pp_params.pool_size = pool_size; >> pp_params.nid = cpu_to_node(c->cpu); >> pp_params.dev = c->pdev; >> pp_params.dma_dir = rq->buff.map_dir; >> >> /* page_pool can be used even when there is no rq->xdp_prog, >> * given page_pool does not handle DMA mapping there is no >> * required state to clear. And page_pool gracefully handle >> * elevated refcnt. >> */ >> rq->page_pool = page_pool_create(&pp_params); >> if (IS_ERR(rq->page_pool)) { >> err = PTR_ERR(rq->page_pool); >> rq->page_pool = NULL; >> goto err_free; >> } >> err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq, >> MEM_TYPE_PAGE_POOL, rq->page_pool); > Thanks for the detailed analysis, I'll need more time to understand the > whole picture :-) >