On Thu, Jul 25, 2019 at 04:09:26PM +0800, Jian-Hong Pan wrote: > Each skb as the element in RX ring was expected with sized buffer 8216 > (RTK_PCI_RX_BUF_SIZE) bytes. However, the skb buffer's true size is > 16640 bytes for alignment after allocated, x86_64 for example. And, the rtw88 advertise IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454, so maximum AMSDU packet can be approximately 12kB. This might be accidental, but having 16kB skb's allow to handle such big AMSDUs. If you shrink buf size, you can probably override memory after buffer end. > difference will be enlarged 512 times (RTK_MAX_RX_DESC_NUM). > To prevent that much wasted memory, this patch follows David's > suggestion [1] and uses general buffer arrays, instead of skbs as the > elements in RX ring. > > [1] https://www.spinics.net/lists/linux-wireless/msg187870.html > > Signed-off-by: Jian-Hong Pan <jian-hong@xxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> This does not fix any serious problem, it actually most likely introduce memory corruption problem described above. Should not be targeted to stable anyway. > - dev_kfree_skb_any(skb); > + devm_kfree(rtwdev->dev, buf); For what this is needed? devm_ allocations are used exactly to avoid manual freeing. > + len = pkt_stat.pkt_len + pkt_offset; > + skb = dev_alloc_skb(len); > + if (WARN_ONCE(!skb, "rx routine starvation\n")) > goto next_rp; > > /* put the DMA data including rx_desc from phy to new skb */ > - skb_put_data(new, skb->data, new_len); > + skb_put_data(skb, rx_desc, len); Coping big packets it quite inefficient. What drivers usually do is copy only for small packets and for big ones allocate new rx buf (drop packet alloc if fail) and pas old buf to network stack via skb_add_rx_frag(). See iwlmvm as example. Stanislaw
