On 2015/12/15 6:47, Julian Calaby wrote:
Hi Fengwei, On Mon, Dec 14, 2015 at 9:06 PM, Fengwei Yin <fengwei.yin@xxxxxxxxxx> 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> --- Changes from v1: * Move switch block out of while loop. * Remove the warning of unknown channel because we didn't deal with it. drivers/net/wireless/ath/wcn36xx/dxe.c | 50 ++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c index f8dfa05..6b61874 100644 --- a/drivers/net/wireless/ath/wcn36xx/dxe.c +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c @@ -467,6 +467,18 @@ out_err: } +#define GET_CH_CTRL_VALUE(x) \ + ({ u32 __v = WCN36XX_DXE_CTRL_RX_H; \ + if ((x) == WCN36XX_DXE_CH_RX_L) \ + __v = WCN36XX_DXE_CTRL_RX_L; \ + __v; }) + +#define GET_CH_INT_MASK(x) \ + ({ u32 __v = WCN36XX_DXE_INT_CH3_MASK; \ + if ((x) == WCN36XX_DXE_CH_RX_L) \ + __v = WCN36XX_DXE_INT_CH1_MASK; \ + __v; }) +Why add these ugly macros if you're only calling them once?static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch) { @@ -474,36 +486,34 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn, struct wcn36xx_dxe_desc *dxe = ctl->desc; dma_addr_t dma_addr; struct sk_buff *skb; + int ret = 0, int_mask; + u32 value; +Surely something like: if (ch->ch_type == WCN36XX_DXE_CH_RX_L) { value = WCN36XX_DXE_CTRL_RX_L; int_mask = WCN36XX_DXE_INT_CH1_MASK; } else { value = WCN36XX_DXE_CTRL_RX_H; int_mask = WCN36XX_DXE_INT_CH3_MASK; } would be much cleaner.
OK. I will remove the ugly macros. Thanks a lot for reviewing it. Regards Yin, Fengwei
Thanks,
-- 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
