Hi Eric, Thanks for the comments. > From: Eric Dumazet [mailto:eric.dumazet@xxxxxxxxx] > Sent: Tuesday, March 29, 2016 6:29 PM > To: Wei-Ning Huang > Cc: Kalle Valo; Linux Wireless; LKML; Amitkumar Karwar; Nishant > Sarmukadam; Sameer Nanda; netdev@xxxxxxxxxxxxxxx; Sonny Rao; Douglas > Anderson > Subject: Re: [PATCH] mwifiex: add __GFP_REPEAT to skb allocation call > > On Tue, 2016-03-29 at 17:27 +0800, Wei-Ning Huang wrote: > > Adding some chromium devs to the thread. > > > > In, http://lxr.free-electrons.com/source/mm/page_alloc.c#L3152 > > > > The default mm retry allocation when 'order <= > > PAGE_ALLOC_COSTLY_ORDER' of gfp_mask contains __GFP_REPEAT. > > PAGE_ALLOC_COSTLY_ORDER is defined to be 3. On systems with page size > > = 4K, this means memory compaction and retry is only done when the > > size of allocation is <= 32K In mwifiex, the allocation size is 64K. > > > > > When we have system with > > memory fragmentation and allocation failed, there will be no retry. > > This is why we need to add __GFP_REPEAT here to allow the system to > > perform memory compaction and retry allocation. > > > > Maybe Amit@marvell can comment on if this is a good fix on this issue. > > I'm also aware that marvell is the progress of implementing > > scatter/gatter for mwifiex, which can also fix the issue. > > Before SG is implemented, you really need to copy incoming frames into > smallest chunks (to get lowest skb->truesize) and leave the 64KB > allocated stuff forever in the driver. We do have a 64KB pre-allocated buffer for receiving Rx data in our driver. > > __GFP_REPEAT wont really solve the issue. > > It seems the problem comes from the fact that the drivers calls > dev_kfree_skb_any() after calling mwifiex_deaggr_sdio_pkt(), instead of > recycling this very precious 64KB skb once memory gets fragmented. Our one time allocated 64k buffer read from firmware contains multiple data chunks. We have a feature called single port aggregation in which firmware attaches an aggregated buffer to single port. So sometimes a single data chunk can exceed 32k. dev_kfree_skb_any() is called to free those data chunks. > > Another problem is that mwifiex_deaggr_sdio_pkt() uses > mwifiex_alloc_dma_align_buf() with GFP_KERNEL | GFP_DMA > > Really GFP_DMA makes no sense here, since the skb is going to be > processed by the stack, which has no such requirement. > > Please use normal skb allocations there. Sure. I will submit a patch for this. Regards, Amitkumar ��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
