Re: [PATCH net 2/6] net/smc: set SOCK_NOSPACE when send_remaining but no sndbuf_space left
From: Paolo Abeni
Date: Tue Dec 03 2024 - 05:04:33 EST
On 11/28/24 13:14, Guangguan Wang wrote:
> When application sending data more than sndbuf_space, there have chances
> application will sleep in epoll_wait, and will never be wakeup again. This
> is caused by a race between smc_poll and smc_cdc_tx_handler.
>
> application tasklet
> smc_tx_sendmsg(len > sndbuf_space) |
> epoll_wait for EPOLL_OUT,timeout=0 |
> smc_poll |
> if (!smc->conn.sndbuf_space) |
> | smc_cdc_tx_handler
> | atomic_add sndbuf_space
> | smc_tx_sndbuf_nonfull
> | if (!test_bit SOCK_NOSPACE)
> | do not sk_write_space;
> set_bit SOCK_NOSPACE; |
> return mask=0; |
>
> Application will sleep in epoll_wait as smc_poll returns 0. And
> smc_cdc_tx_handler will not call sk_write_space because the SOCK_NOSPACE
> has not be set. If there is no inflight cdc msg, sk_write_space will not be
> called any more, and application will sleep in epoll_wait forever.
> So set SOCK_NOSPACE when send_remaining but no sndbuf_space left in
> smc_tx_sendmsg, to ensure call sk_write_space in smc_cdc_tx_handler
> even when the above race happens.
I think it should be preferable to address the mentioned race the same
way as tcp_poll(). i.e. checking again smc->conn.sndbuf_space after
setting the NOSPACE bit with appropriate barrier, see:
https://elixir.bootlin.com/linux/v6.12.1/source/net/ipv4/tcp.c#L590
that will avoid additional, possibly unneeded atomic operation in the tx
path (the application could do the next sendmsg()/poll() call after that
the send buf has been freed) and will avoid some code duplication.
Cheers,
Paolo