| Message ID | 1654558751-3702-1-git-send-email-chen45464546@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v4] net: ethernet: mtk_eth_soc: fix misuse of mem alloc interface netdev[napi]_alloc_frag | expand |
On Tue, 7 Jun 2022 07:39:11 +0800 Chen Lin wrote: > +static inline void *mtk_max_lro_buf_alloc(gfp_t gfp_mask) No need for inline, compiler will inline this anyway. > +{ > + void *data; unsigned long data; then you can move the cast from the long line to the return statement, saving us from the strange indentation. > + data = (void *)__get_free_pages(gfp_mask | > + __GFP_COMP | __GFP_NOWARN, > + get_order(mtk_max_frag_size(MTK_MAX_LRO_RX_LENGTH))); > + > + return data; > +}
At 2022-06-08 07:14:13, "Jakub Kicinski" <kuba@kernel.org> wrote: >On Tue, 7 Jun 2022 07:39:11 +0800 Chen Lin wrote: >> +static inline void *mtk_max_lro_buf_alloc(gfp_t gfp_mask) > >No need for inline, compiler will inline this anyway. > >> +{ >> + void *data; > >unsigned long data; then you can move the cast from the long line to >the return statement, saving us from the strange indentation. > >> + data = (void *)__get_free_pages(gfp_mask | >> + __GFP_COMP | __GFP_NOWARN, >> + get_order(mtk_max_frag_size(MTK_MAX_LRO_RX_LENGTH))); >> + >> + return data; >> +} I'll do it like below : +static void *mtk_max_lro_buf_alloc(gfp_t gfp_mask) +{ + unsigned long data; + unsigned int size = mtk_max_frag_size(MTK_MAX_LRO_RX_LENGTH); + + data = __get_free_pages(gfp_mask | __GFP_COMP | __GFP_NOWARN, + get_order(size)); + + return (void *)data; +} Through analysis of the ASM code from objdump, I confirmed that the inline is not necessary. Thanks for your tips. Also, I confirmed that create a new local variable 'size' will not affect the generation of a constant 'order' parameter at compile time. ASM code of calling 'mtk_max_lro_buf_alloc': 'mtk_max_lro_buf_alloc' inlined and 'order'(w1) is a constant 0x2 0000000000004530 <mtk_napi_rx>: ... 4a98: 52854400 mov w0, #0x2a20 // #10784 4a9c: 52800041 mov w1, #0x2 // #2 4aa0: 72a00080 movk w0, #0x4, lsl #16 4aa4: 94000000 bl 0 <__get_free_pages> 4aa8: f90033e0 str x0, [sp, #96] 0000000000000730 <mtk_rx_alloc>: ... 7fc: 2a1703e0 mov w0, w23 800: 52800041 mov w1, #0x2 // #2 804: 7140047f cmp w3, #0x1, lsl #12 808: 54fffe49 b.ls 7d0 <mtk_rx_alloc+0xa0> // b.plast 80c: 94000000 bl 0 <__get_free_pages> The compiler is smart.
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index b3b3c07..3da162e 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -899,6 +899,17 @@ static bool mtk_rx_get_desc(struct mtk_eth *eth, struct mtk_rx_dma_v2 *rxd, return true; } +static inline void *mtk_max_lro_buf_alloc(gfp_t gfp_mask) +{ + void *data; + + data = (void *)__get_free_pages(gfp_mask | + __GFP_COMP | __GFP_NOWARN, + get_order(mtk_max_frag_size(MTK_MAX_LRO_RX_LENGTH))); + + return data; +} + /* the qdma core needs scratch memory to be setup */ static int mtk_init_fq_dma(struct mtk_eth *eth) { @@ -1467,7 +1478,10 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget, goto release_desc; /* alloc new buffer */ - new_data = napi_alloc_frag(ring->frag_size); + if (ring->frag_size <= PAGE_SIZE) + new_data = napi_alloc_frag(ring->frag_size); + else + new_data = mtk_max_lro_buf_alloc(GFP_ATOMIC); if (unlikely(!new_data)) { netdev->stats.rx_dropped++; goto release_desc; @@ -1914,7 +1928,10 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag) return -ENOMEM; for (i = 0; i < rx_dma_size; i++) { - ring->data[i] = netdev_alloc_frag(ring->frag_size); + if (ring->frag_size <= PAGE_SIZE) + ring->data[i] = netdev_alloc_frag(ring->frag_size); + else + ring->data[i] = mtk_max_lro_buf_alloc(GFP_KERNEL); if (!ring->data[i]) return -ENOMEM; }
When rx_flag == MTK_RX_FLAGS_HWLRO, rx_data_len = MTK_MAX_LRO_RX_LENGTH(4096 * 3) > PAGE_SIZE. netdev_alloc_frag is for alloction of page fragment only. Reference to other drivers and Documentation/vm/page_frags.rst Branch to use __get_free_pages when ring->frag_size > PAGE_SIZE. Signed-off-by: Chen Lin <chen45464546@163.com> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)