Re: [PATCH v2] datagram: return from __skb_recv_datagram() as soon as possible

[Willem de Bruijn]

On Sun, Jul 15, 2018 at 4:17 AM Baoyou Xie <baoyou.xie@xxxxxxxxx> wrote:
>
> We got a soft lockup in a heavy busy cloud server where RIP is
> at _raw_spin_unlock_irqrestore+0x1b/0x40:
>         [] finish_wait+0x56/0x70
>         [] __skb_recv_datagram+0x3fb/0x5a0
>         [] ? datagram_poll+0x100/0x100
>         [] skb_recv_datagram+0x41/0x60
>         [] netlink_recvmsg+0x62/0x450
>         [] sock_recvmsg+0xbf/0x100
>         [] ? futex_wait+0x193/0x280
>         [] ? finish_task_switch+0x108/0x170
>         [] SYSC_recvfrom+0xe8/0x160
>         [] ? __schedule+0x3c8/0x990
>         [] SyS_recvfrom+0xe/0x10
>         [] system_call_fastpath+0x16/0x1b
>
> In fact, a mistake exists in __skb_recv_datagram(). For example,
> if a datagram come in persistently after go through the socket
> queue, then __skb_wait_for_more_packets() will find out that the
> last peeked skb is not the real last one, so it return 0. this
> results in long time outer loop, and can trigger soft lockup.

Is this with MSG_PEEK?

If the above occurs, that implies that the queue is not empty so the
next iteration of the loop in __skb_recv_datagram should return
the oldest packet on the queue.

This is a netlink socket. Those do not support SO_PEEK_OFF,
simplifying the problem somewhat.

I do not yet see how this can loop until timeout if data is queued.

> So this patch changes the loop condition to prevent soft lockup.

Bounding waiting time in this manner should not be needed, as
__skb_wait_for_more_packets reduces remaining timeo on wake.