RE: rtw88: alloc_skb(32768, GFP_ATOMIC) fails, driver gets stuck

[Ping-Ke Shih <pkshih@realtek.com>]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> 
> alloc_skb fails (silently) therefore the RX URB is not submitted
> ever again. There are only 4 RX URBs.

Though only 4 RX URB, it might be possible more than 4 RX skb. 
In rtw_usb_read_port_complete(), queue RX skb into rtwusb->rx_queue, and kick
off rx_work. It means some RX skb are inflight, but not sure how many.

> 
> static void rtw_usb_rx_resubmit(struct rtw_usb *rtwusb, struct rx_usb_ctrl_block *rxcb)
> {
>         struct rtw_dev *rtwdev = rtwusb->rtwdev;
>         int error;
> 
>         rxcb->rx_skb = alloc_skb(RTW_USB_MAX_RECVBUF_SZ, GFP_ATOMIC);
>         if (!rxcb->rx_skb)
>                 return;
> 
>         usb_fill_bulk_urb(rxcb->rx_urb, rtwusb->udev,
>                           usb_rcvbulkpipe(rtwusb->udev, rtwusb->pipe_in),
>                           rxcb->rx_skb->data, RTW_USB_MAX_RECVBUF_SZ,
>                           rtw_usb_read_port_complete, rxcb);
> 
>         error = usb_submit_urb(rxcb->rx_urb, GFP_ATOMIC);
> 
> I added an error message there:
> 
>         rxcb->rx_skb = alloc_skb(RTW_USB_MAX_RECVBUF_SZ, GFP_ATOMIC);
>         if (!rxcb->rx_skb) {
>                 rtw_err(rtwdev, "failed to allocate rx_skb\n");
>                 return;
>         }

My first thought is to change GFP_ATOMIC to GFP_KERNEL, but kernel documentation
notes that 
"NEVER SLEEP IN A COMPLETION HANDLER. These are often called in atomic context."
However, I feel it is possible to do rtw_usb_rx_resubmit() in a work.

Another thought is to allocate a new skb with size urb->actual_length, and
copy received data to the new skb, and queue to rtwusb->rx_queue. Then reuse
the original rx_skb. This thought is based on what urb->actual_length would
be smaller than RTW_USB_MAX_RECVBUF_SZ, but not very sure if this is fact.