| Message ID | 1459226840-36287-1-git-send-email-wnhuang@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On Tue, Mar 29, 2016 at 12:47:20PM +0800, Wei-Ning Huang wrote: > "single skb allocation failure" happens when system is under heavy > memory pressure. Add __GFP_REPEAT to skb allocation call so kernel > attempts to reclaim pages and retry the allocation. Oh, that's interesting, we're back to this symptom again. Nice to see this fix. Heavy memory pressure on 3.5 caused dev_alloc_skb failure in this driver. Tracked at OLPC as #12694.
Wei-Ning Huang <wnhuang@chromium.org> writes: > "single skb allocation failure" happens when system is under heavy > memory pressure. Add __GFP_REPEAT to skb allocation call so kernel > attempts to reclaim pages and retry the allocation. > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> Is this really a proper way to fix the issue? This is the first time I'm hearing about the flag and there isn't even a single user in drivers/net. I would like to get confirmation from others that __GFP_REPEAT is really ok to use in a wireless driver before I can take this.
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. Wei-Ning On Tue, Mar 29, 2016 at 4:37 PM, Kalle Valo <kvalo@codeaurora.org> wrote: > Wei-Ning Huang <wnhuang@chromium.org> writes: > >> "single skb allocation failure" happens when system is under heavy >> memory pressure. Add __GFP_REPEAT to skb allocation call so kernel >> attempts to reclaim pages and retry the allocation. >> >> Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > > Is this really a proper way to fix the issue? This is the first time I'm > hearing about the flag and there isn't even a single user in > drivers/net. I would like to get confirmation from others that > __GFP_REPEAT is really ok to use in a wireless driver before I can take > this. > > -- > Kalle Valo
> From: Wei-Ning Huang [mailto:wnhuang@google.com] > Sent: Tuesday, March 29, 2016 2:57 PM > To: Kalle Valo > Cc: Linux Wireless; LKML; Amitkumar Karwar; Nishant Sarmukadam; Sameer > Nanda; netdev@vger.kernel.org; Sonny Rao; Douglas Anderson > Subject: Re: [PATCH] mwifiex: add __GFP_REPEAT to skb allocation call > > 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. > > Wei-Ning > This fix would be useful. We have a feature called single port aggregation in which sometimes data received from SDIO interface can be >32k (but less than 64k). This feature improves throughput performance. We are preparing patches for scatter/gather feature. but scatter/gather won't be supported by some platforms. Hence this fix would still be needed. Regards, Amitkumar
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c index b2c839a..c64989c 100644 --- a/drivers/net/wireless/marvell/mwifiex/sdio.c +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c @@ -1124,7 +1124,8 @@ static void mwifiex_deaggr_sdio_pkt(struct mwifiex_adapter *adapter, break; } skb_deaggr = mwifiex_alloc_dma_align_buf(pkt_len, - GFP_KERNEL | GFP_DMA); + GFP_KERNEL | GFP_DMA | + __GFP_REPEAT); if (!skb_deaggr) break; skb_put(skb_deaggr, pkt_len); @@ -1374,7 +1375,8 @@ static int mwifiex_sdio_card_to_host_mp_aggr(struct mwifiex_adapter *adapter, /* copy pkt to deaggr buf */ skb_deaggr = mwifiex_alloc_dma_align_buf(len_arr[pind], GFP_KERNEL | - GFP_DMA); + GFP_DMA | + __GFP_REPEAT); if (!skb_deaggr) { mwifiex_dbg(adapter, ERROR, "skb allocation failure\t" "drop pkt len=%d type=%d\n", @@ -1416,7 +1418,8 @@ rx_curr_single: mwifiex_dbg(adapter, INFO, "info: RX: port: %d, rx_len: %d\n", port, rx_len); - skb = mwifiex_alloc_dma_align_buf(rx_len, GFP_KERNEL | GFP_DMA); + skb = mwifiex_alloc_dma_align_buf(rx_len, GFP_KERNEL | GFP_DMA | + __GFP_REPEAT); if (!skb) { mwifiex_dbg(adapter, ERROR, "single skb allocated fail,\t" @@ -1521,7 +1524,8 @@ static int mwifiex_process_int_status(struct mwifiex_adapter *adapter) rx_len = (u16) (rx_blocks * MWIFIEX_SDIO_BLOCK_SIZE); mwifiex_dbg(adapter, INFO, "info: rx_len = %d\n", rx_len); - skb = mwifiex_alloc_dma_align_buf(rx_len, GFP_KERNEL | GFP_DMA); + skb = mwifiex_alloc_dma_align_buf(rx_len, GFP_KERNEL | GFP_DMA | + __GFP_REPEAT); if (!skb) return -1;
"single skb allocation failure" happens when system is under heavy memory pressure. Add __GFP_REPEAT to skb allocation call so kernel attempts to reclaim pages and retry the allocation. Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> --- drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)