TPACKET_V3 retires a full or expired block while holding
sk_receive_queue.lock. prb_retire_current_block() takes
blk_fill_in_prog_lock for write to wait for in-flight skb_copy_bits()
before publishing the block.
On PREEMPT_RT a read lock can be held by a task that has been
preempted or is running on another CPU. The packet path or the
retirement timer then blocks on the write lock while holding
sk_receive_queue.lock, and the receive path resumes only after the
copy completes. This was observed as a receive stall of hundreds of
milliseconds in a PREEMPT_RT deployment; manual inspection found the
same unconditional write_lock() in both the full-block and the timer
retirement paths.
Try to take the write lock without blocking on PREEMPT_RT. If a copy
is still in progress, leave the current block and its metadata
untouched:
- the retirement timer re-arms itself and retries after the configured
interval;
- packet allocation returns NULL, so tpacket_rcv() drops this capture
record and accounts it in tp_drops, while the original packet remains
in the network stack;
- a later packet can retire the block once the copier has finished.
Do not dispatch the next block while the current block is still open.
The existing lock order and completion ordering are preserved, and no
new state or worker is introduced. The trade-off is a higher capture
drop rate while a copy is in progress; original traffic is unaffected.
Non-PREEMPT_RT kernels keep the unconditional write_lock(): there
read_lock() disables preemption, so the writer only waits for the copy
to finish, not for a scheduled-out task to run again.
The stall was reproduced on QEMU arm64 SMP PREEMPT_RT by delaying a
copier after frame reservation: without this change the next sender
blocks until the copier is released, with this change only that
capture record is dropped.
Fixes: 632ca50f2cbd ("af_packet: TPACKET_V3: replace busy-wait loop")
Signed-off-by: Jingxuan Xie <redacted>
---
Changes in v4:
- Limit the write_trylock() switch to PREEMPT_RT; non-PREEMPT_RT
kernels keep the blocking write lock (Willem).
- Leave the existing indentation of prb_retire_current_block()
unchanged (Willem).
- Add the discovery and reproduction information requested by
netdev-bot.
v3: https://lore.kernel.org/r/20261103092541.273847-1-jingxuanxie@deeproute.ai (local)
Testing:
- arm64 W=1 af_packet.o built with PREEMPT_RT=y and PREEMPT_RT=n.
- QEMU arm64 SMP PREEMPT_RT, 3 boots: psock_tpacket and psock_fanout
exit 0 (the fanout queue-length warning also appears on the
baseline); with a copier delayed in skb_copy_bits() while holding
blk_fill_in_prog_lock, a second sender returns in 0.7-1.8 ms and only
its capture record is dropped; the retirement timer retries and
publishes the block after the copier is released; close/unmap during
the delay does not hang; no lockdep or atomic-sleep splats.
- QEMU arm64 SMP PREEMPT (no RT) with the same delay: the second
sender blocks until the copier is released (503 ms) and no capture
record is dropped, matching the unpatched kernel.
net/packet/af_packet.c | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 494d628d10a5..4d56893c4fe5 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -198,7 +198,7 @@ static void packet_increment_head(struct packet_ring_buffer *buff);
static int prb_curr_blk_in_use(struct tpacket_block_desc *);
static void *prb_dispatch_next_block(struct tpacket_kbdq_core *,
struct packet_sock *);
-static void prb_retire_current_block(struct tpacket_kbdq_core *,
+static bool prb_retire_current_block(struct tpacket_kbdq_core *,
struct packet_sock *, unsigned int status);
static int prb_queue_frozen(struct tpacket_kbdq_core *);
static void prb_open_block(struct tpacket_kbdq_core *,
@@ -666,6 +666,24 @@ static void init_prb_bdqc(struct packet_sock *po,
prb_open_block(p1, pbd);
}
+/*
+ * Wait for an in-flight skb_copy_bits() before retiring the current block.
+ * On PREEMPT_RT the read lock can be held by a task that has been preempted
+ * or is running on another CPU, so do not block while holding
+ * sk_receive_queue.lock; return false and let the caller retry.
+ */
+static bool prb_wait_for_copy_bits(struct tpacket_kbdq_core *pkc)
+{
+ if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
+ if (!write_trylock(&pkc->blk_fill_in_prog_lock))
+ return false;
+ } else {
+ write_lock(&pkc->blk_fill_in_prog_lock);
+ }
+ write_unlock(&pkc->blk_fill_in_prog_lock);
+ return true;
+}
+
/*
* With a 1MB block-size, on a 1Gbps line, it will take
* i) ~8 ms to fill a block + ii) memcpy etc.@@ -700,9 +718,9 @@ static enum hrtimer_restart prb_retire_rx_blk_timer_expired(struct hrtimer *t)
*
*/
if (BLOCK_NUM_PKTS(pbd)) {
- /* Waiting for skb_copy_bits to finish... */
- write_lock(&pkc->blk_fill_in_prog_lock);
- write_unlock(&pkc->blk_fill_in_prog_lock);
+ /* Retry later if a packet copy is still in progress. */
+ if (!prb_wait_for_copy_bits(pkc))
+ goto refresh_timer;
}
if (!frozen) {@@ -727,6 +745,7 @@ static enum hrtimer_restart prb_retire_rx_blk_timer_expired(struct hrtimer *t)
}
}
+refresh_timer:
hrtimer_forward_now(&pkc->retire_blk_timer, pkc->interval_ktime);
spin_unlock(&po->sk.sk_receive_queue.lock);
return HRTIMER_RESTART;
@@ -935,7 +954,7 @@ static void *prb_dispatch_next_block(struct tpacket_kbdq_core *pkc,
return (void *)pkc->nxt_offset;
}
-static void prb_retire_current_block(struct tpacket_kbdq_core *pkc,
+static bool prb_retire_current_block(struct tpacket_kbdq_core *pkc,
struct packet_sock *po, unsigned int status)
{
struct tpacket_block_desc *pbd = GET_CURR_PBLOCK_DESC_FROM_CORE(pkc);@@ -952,13 +971,13 @@ static void prb_retire_current_block(struct tpacket_kbdq_core *pkc,
* the timer-handler already handled this case.
*/
if (!(status & TP_STATUS_BLK_TMO)) {
- /* Waiting for skb_copy_bits to finish... */
- write_lock(&pkc->blk_fill_in_prog_lock);
- write_unlock(&pkc->blk_fill_in_prog_lock);
+ /* Do not wait for copies while holding the queue lock. */
+ if (!prb_wait_for_copy_bits(pkc))
+ return false;
}
prb_close_block(pkc, pbd, po, status);
- return;
}
+ return true;
}
static int prb_curr_blk_in_use(struct tpacket_block_desc *pbd)@@ -1086,7 +1105,8 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
}
/* Ok, close the current block */
- prb_retire_current_block(pkc, po, 0);
+ if (!prb_retire_current_block(pkc, po, 0))
+ return NULL;
/* Now, try to dispatch the next block */
curr = (char *)prb_dispatch_next_block(pkc, po);
--
2.25.1