On 2025/1/24 3:15, Florian Fainelli wrote: > > Sorry for the late feedback, this patch causes the bgmac driver in is .ndo_open() function to return -ENOMEM, the call trace looks like this: Hi, Florian Thanks for the report. > > bgmac_open > -> bgmac_dma_init > -> bgmac_dma_rx_skb_for_slot > -> netdev_alloc_frag > > BGMAC_RX_ALLOC_SIZE = 10048 and PAGE_FRAG_CACHE_MAX_SIZE = 32768. I guess BGMAC_RX_ALLOC_SIZE being bigger than PAGE_SIZE is the problem here, as the frag API is not really supporting allocating fragment that is bigger than PAGE_SIZE, as it will fall back to allocate the base page when order-3 compound page allocation fails, see __page_frag_cache_refill(). Also, it seems strange that bgmac driver seems to always use jumbo frame size to allocate fragment, isn't more appropriate to allocate the fragment according to MTU? > > Eventually we land into __page_frag_alloc_align() with the following parameters across multiple successive calls: > > __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=0 > __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=10048 > __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=20096 > __page_frag_alloc_align: fragsz=10048, align_mask=-1, size=32768, offset=30144 > > So in that case we do indeed have offset + fragsz (40192) > size (32768) and so we would eventually return NULL. It seems the changing of '(unlikely(offset < 0))' checking to "fragsz > PAGE_SIZE" causes bgmac driver to break more easily here. But bgmac driver might likely break too if the system memory is severely fragmented when falling back to alllocate base page before this patch. > > Any idea on how to best fix that within the bgmac driver? Maybe use the page allocator API directly when allocating fragment with BGMAC_RX_ALLOC_SIZE > PAGE_SIZE for a quick fix. In the long term, maybe it makes sense to use the page_pool API as more drivers are converting to use page_pool API already.
