Re: [PATCH net] net/smc: don't spin in smc_rx_recvmsg() while a pipe holds spliced data

[Bryam Vargas]

On Thu, 15 Oct 2026 20:47:10 +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> [Severity: Medium]
> Can this wait miss the final wakeup on weakly ordered architectures?

Yes. Between the entry's store in add_wait_queue() and the read of
splice_pending the sleeper only has an unlock and a lock acquire, not
a full barrier; the release side reads the list after its own
smp_mb(), so the gap is the sleeper's. v2 puts an smp_mb() after
add_wait_queue(). Checked with herd7 against LKMM: the lost wake is
allowed without it, forbidden with it. smc_rx_wait() has the same
gap, but every CDC message that brings data wakes it again; this wait
has only the pipe release to wake it, so the barrier matters here.
Moving the decrement under lock_sock() is the change Hidayath Khan
announced in [1]; the barrier closes the window on its own and
doesn't depend on it.

> [Severity: Low]
> Should smc_rx_wait_splice() do the same?

Yes, v2 sets and clears SOCKWQ_ASYNC_WAITDATA around the sleep.
Measured on the v1 setup with one more case, an O_ASYNC reader owned
by the reading thread, blocked behind the pipe while the peer sends
one more byte: v1 returns -EINTR within 1 ms of the byte, v2 sleeps
on and reads the next 4096 bytes once the pipe is drained, as the
unpatched kernel does.

> [Severity: Medium]
> This is a pre-existing issue that this patch didn't introduce: can the
> splice path read a stale splbytes here?

It can; the fix is Hidayath's change in [1]. This patch doesn't touch
that path.

> [Severity: Medium]
> This is a pre-existing issue rather than one added by this patch, but
> smc_poll() still disagrees with this -EAGAIN:

The commit message of v2 now says so: a non-blocking read behind the
pipe returns -EAGAIN while poll() reports the socket readable, since
bytes_to_rcv counts the data behind the pipe. A level-triggered loop
that keeps the spliced bytes in a pipe gets EPOLLIN and -EAGAIN until
it drains that pipe; before the patch the same read didn't return
until the drain, asleep, one pass per tick, not spinning (measured).
Looking at splice_pending in smc_poll() would hide data splice() can
still take (a second splice with the first undrained returns the next
4096 bytes at once, measured), so v2 leaves poll() alone; the pipe
release wakes the socket.

[1] https://lore.kernel.org/all/7a174d1f-80d8-4294-ae66-829c9dacd662@xxxxxxxxxxxxx/

pw-bot: cr