On 2015/12/11 21:37, Bob Copeland wrote:
On Fri, Dec 11, 2015 at 09:14:04PM +0800, fengwei.yin wrote:On 2015/12/2 13:27, Fengwei Yin wrote:Lawrence reported that git clone could make system crash on a Qualcomm ARM soc based device (DragonBoard, 1G memory without swap) running 64bit Debian. It's turned out the crash is related with rx skb allocation failure. git could consume more than 600MB anonymous memory. And system is in extremely memory shortage case. But driver didn't handle the rx allocation failure case. This patch doesn't submit skb to upper layer if rx skb allocation fails. Instead, it reuse the old skb for rx DMA again. It's more like drop the packets if system is in memory shortage case. With this change, git clone is OOMed instead of system crash. Reported-by: King, Lawrence <lking@xxxxxxxxxxxxxxxx> Signed-off-by: Fengwei Yin <fengwei.yin@xxxxxxxxxx>Concept makes sense to me, but:
Thanks for looking at it.
dma_addr = dxe->dst_addr_l; - wcn36xx_dxe_fill_skb(wcn->dev, ctl); + ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl); + if (0 == ret) {I find this "success handling" to be unclear and traditionally this kind of thing is a source of bugs; how about instead:+ /* new skb allocation ok. Use the new one and queue + * the old one to network system. + */ + dma_unmap_single(wcn->dev, dma_addr, WCN36XX_PKT_SIZE, + DMA_FROM_DEVICE); + wcn36xx_rx_skb(wcn, skb); + }ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl); /* skip this frame if we can't alloc a new rx buffer */ if (ret) goto drop;
This can't work because we need to initialize the DMA for the old skb again.
Which is done in following
switch (ch->ch_type) {
block.
Regards
Yin, Fengwei
switch (ch->ch_type) { case WCN36XX_DXE_CH_RX_L: @@ -495,9 +504,6 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn, wcn36xx_warn("Unknown channel\n"); } - dma_unmap_single(wcn->dev, dma_addr, WCN36XX_PKT_SIZE, - DMA_FROM_DEVICE); - wcn36xx_rx_skb(wcn, skb);drop:ctl = ctl->next; dxe = ctl->desc; }
-- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
