* Re: [PATCH v3 net-next 0/6] net: Move system_long_wq to system_dfl_long_wq
From: Jacob Keller @ 2026-07-20 22:35 UTC (permalink / raw)
To: Marco Crivellari, linux-kernel, netdev
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Michal Hocko, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe Leroy (CS GROUP), Ethan Nelson-Moore, Haren Myneni,
Madhavan Srinivasan, MD Danish Anwar, Michael Ellerman,
Mika Westerberg, Nicholas Piggin, Nick Child, Petko Manolov,
Richard Cheng, Rick Lindsley, Roger Quadros, Yehezkel Bernat
In-Reply-To: <20260720100902.155605-1-marco.crivellari@suse.com>
On 7/20/2026 3:08 AM, Marco Crivellari wrote:
> Hello,
>
> Currently the code uses the per-cpu workqueue system_long_wq to schedule
> long running works.
>
> Unbound works could benefit from scheduler task placement, to optimize
> performance and power consumption. Another good reason to have this unbound,
> is the "queue_delayed_work()" function, used to enqueue the work item.
> More details on this will follow in the next section.
>
> Recently, a new unbound workqueue specific for long running work has been
> added:
>
> c116737e972e ("workqueue: Add system_dfl_long_wq for long unbound works")
>
> ~~~ Details about queue_delayed_work ~~~
>
> system_long_wq is a per-cpu workqueue and it is used as a parameter of
> queue_delayed_work(). This function schedule an item that it will later
> be enqueued (once the timer will fire). __queue_delayed_work() does the job
> receiving as "cpu" WORK_CPU_UNBOUND:
>
> if (housekeeping_enabled(HK_TYPE_TIMER)) {
> // [....]
> } else {
> if (likely(cpu == WORK_CPU_UNBOUND))
> add_timer_global(timer);
> else
> add_timer_on(timer, cpu);
> }
>
> The timer is global, so can fire everywhere, and the work item will be
> enqueued where the timer fired.
>
> Since the workqueue work doesn't rely on per-cpu variables, there is no
> obvious reason that justify the use of a per-cpu workqueue. So change the
> workqueue with the new system_dfl_long_wq, so that the used workqueue is
> now unbound and can benefit from scheduler task placement.
>
Ok. So if I am understanding this correctly, the current code uses
system_long_wq which is per-CPU, but is fired using an unbound timer. As
a result, whichever CPU the timer triggers on will be the one which
selects the work queue. From there, the work item will be enqueued to
that work queue and remain on that work queue until resolving with no
way for scheduler to adjust it?
With the new change, we schedule on the system_dfl_long_wq which *isn't*
per CPU, so the scheduler is free to move the task around and
reschedule. As a result we get better overall behavior with more input
from the scheduler, instead of effective randomness from the timer which
is then forced so that such long running task cannot migrate?
That sounds like a pretty good improvement for the cases where the
queued work doesn't depend on any per-cpu behavior. Nice!
I am not sure I can speak to any of the individual drivers here since I
wouldn't know whether moving that particular work item would be
affected.. so feel free to take this review with a grain of salt :)
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply
* Re: [PATCH net-next v9 1/3] net: airoha: rename airoha_priv_flags to airoha_dev_flags
From: Jacob Keller @ 2026-07-20 22:36 UTC (permalink / raw)
To: Lorenzo Bianconi, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Alexander Lobakin, linux-arm-kernel, linux-mediatek,
netdev
In-Reply-To: <20260721-airoha-ethtool-priv_flags-v9-1-9c15d8b71a56@kernel.org>
On 7/20/2026 3:03 PM, Lorenzo Bianconi wrote:
> Rename the airoha_priv_flags enum to airoha_dev_flags and the
> AIROHA_PRIV_F_WAN flag to AIROHA_DEV_F_WAN. The "priv_flags" naming
> dates back to an earlier design that used ethtool private flags; since
> this series switched to tc qdisc offload for LAN/WAN configuration,
> align the naming to reflect that these are per-device flags rather than
> ethtool private flags. No functional change.
>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply
* Re: [PATCH net-next v9 2/3] net: airoha: fix ETS QoS stats counter underflow and cross-channel corruption
From: Jacob Keller @ 2026-07-20 22:42 UTC (permalink / raw)
To: Lorenzo Bianconi, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Alexander Lobakin, linux-arm-kernel, linux-mediatek,
netdev
In-Reply-To: <20260721-airoha-ethtool-priv_flags-v9-2-9c15d8b71a56@kernel.org>
On 7/20/2026 3:03 PM, Lorenzo Bianconi wrote:
> airoha_qdma_get_tx_ets_stats() has two bugs:
> - The hardware counters read via airoha_qdma_rr() are 32-bit values
> but are stored in u64 locals and subtracted from u64 baselines. When
> a 32-bit hardware counter wraps around, the subtraction produces a
> large underflow value passed to _bstats_update().
This issue would only be a problem during rollover, which depending on
how fast the counts increment may not be a big problem. I could see this
not being worth going to net since it could be rare enough that it isn't
considered a widespread issue...
> - The baseline counters (cpu_tx_packets, fwd_tx_packets) are stored as
> single per-device fields, but airoha_qdma_get_tx_ets_stats() is
> called with different channel values (0-3). Each call reads a
> different channel's hardware counter but overwrites the same
> baseline, corrupting the delta computation for other channels.
>
However, this issue seems like its going to cause a problem every time
you read because any time you use a mix of channels you will get
corrupted values?
> Fix both by:
> - Narrowing the counter locals and baselines to u32 so that 32-bit
> unsigned subtraction handles wrap-around naturally.
> - Grouping the baselines into a per-channel qos_stats array so each
> channel tracks its own previous counter value independently.
> - Splitting the delta addition into two statements so the first u32
> delta is widened to u64 on assignment and the second is added in
> u64 arithmetic, preventing overflow when both deltas are large.
>
> Fixes: 20bf7d07c956 ("net: airoha: Add sched ETS offload support")
This targets a commit which merged in v6.14, but the patch is part of a
series aimed at net-next. Could you explain why this shouldn't be
separated out and put as a fix in net? It seems pretty obvious that
users can easily reproduce problems by requesting stats from each
channel? Or is this not really possible to trigger from userspace until
patch 3/3?
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/net/ethernet/airoha/airoha_eth.c | 18 +++++++++++-------
> drivers/net/ethernet/airoha/airoha_eth.h | 7 ++++---
> 2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 41c1a0ffbdd8..aaf2a4717d12 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2482,16 +2482,20 @@ static int airoha_qdma_get_tx_ets_stats(struct net_device *netdev, int channel,
> {
> struct airoha_gdm_dev *dev = netdev_priv(netdev);
> struct airoha_qdma *qdma = dev->qdma;
> + u32 cpu_tx_packets, fwd_tx_packets;
> + u64 tx_packets;
>
> - u64 cpu_tx_packets = airoha_qdma_rr(qdma, REG_CNTR_VAL(channel << 1));
> - u64 fwd_tx_packets = airoha_qdma_rr(qdma,
> - REG_CNTR_VAL((channel << 1) + 1));
> - u64 tx_packets = (cpu_tx_packets - dev->cpu_tx_packets) +
> - (fwd_tx_packets - dev->fwd_tx_packets);
> + cpu_tx_packets = airoha_qdma_rr(qdma, REG_CNTR_VAL(channel << 1));
> + fwd_tx_packets = airoha_qdma_rr(qdma,
> + REG_CNTR_VAL((channel << 1) + 1));
> + tx_packets = (u32)(cpu_tx_packets -
> + dev->qos_stats[channel].cpu_tx_packets);
> + tx_packets += (u32)(fwd_tx_packets -
> + dev->qos_stats[channel].fwd_tx_packets);
>
> _bstats_update(opt->stats.bstats, 0, tx_packets);
> - dev->cpu_tx_packets = cpu_tx_packets;
> - dev->fwd_tx_packets = fwd_tx_packets;
> + dev->qos_stats[channel].cpu_tx_packets = cpu_tx_packets;
> + dev->qos_stats[channel].fwd_tx_packets = fwd_tx_packets;
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> index bf1c249255bd..bf44be9f0954 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.h
> +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> @@ -553,9 +553,10 @@ struct airoha_gdm_dev {
> struct airoha_eth *eth;
>
> DECLARE_BITMAP(qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS);
> - /* qos stats counters */
> - u64 cpu_tx_packets;
> - u64 fwd_tx_packets;
> + struct {
> + u32 cpu_tx_packets;
> + u32 fwd_tx_packets;
> + } qos_stats[AIROHA_NUM_QOS_CHANNELS];
>
> u32 flags;
> int nbq;
>
^ permalink raw reply
* Re: [PATCH net-next] net: stmmac: Simplify ioctl handling
From: Vadim Fedorenko @ 2026-07-20 22:43 UTC (permalink / raw)
To: Andrew Lunn
Cc: Maxime Chevallier, Andrew Lunn, Jakub Kicinski, davem,
Eric Dumazet, Paolo Abeni, Simon Horman, Maxime Coquelin,
Alexandre Torgue, Russell King, thomas.petazzoni,
Alexis Lothoré, netdev, linux-kernel, linux-arm-kernel,
linux-stm32
In-Reply-To: <1e640dcb-43ce-4a3b-a74a-ed8e48706b86@lunn.ch>
On 20.07.2026 19:12, Andrew Lunn wrote:
> On Mon, Jul 20, 2026 at 04:17:32PM +0100, Vadim Fedorenko wrote:
>> On 19.07.2026 17:13, Andrew Lunn wrote:
>>>> Looking at this, I'm wondering if we can't just get rid of SIOCSHWTSTAMP
>>>> handling in phy_mii_ioctl(). Looks like we can ?
>>>
>>> I'm not sure about that. We need Richards input.
>>>
>>> The code in phy_mii_ioctl() allows the MAC to be bypassed, it goes
>>> straight to a PHY based stamper. It could be the MAC has no idea the
>>> PHY has this capability, so it has not implemented the .ndo?
>>>
>>> It might be we need to hoist the code from phy_mii_ioctl() into
>>> dev_{sg}et_hwtstamp()?
>>
>> Hi Andrew!
>>
>> I think I've converted all phy drivers while removing support for
>> SIOCSHWTSTAMP/SIOCGHWTSTAMP from netdev ioctl. I believe it's impossible right
>> now to reach SIOCSHWTSTAMP path of phy_mii_ioctl via ioctl on net device.
>
> Lets look at this, using a random example:
>
> drivers/net/ethernet/marvell/mv643xx_eth.c
>
> mv643xx_eth_netdev_ops has nothing about time stamping. However it
> does have a mv643xx_eth_ioctl. Which calls phy_mii_ioctl().
>
> Lets say this Marvell MAC driver was paired with a
> nxp-c45-tja11xx. nxp_c45_probe() does:
>
> priv->mii_ts.rxtstamp = nxp_c45_rxtstamp;
> priv->mii_ts.txtstamp = nxp_c45_txtstamp;
> priv->mii_ts.hwtstamp_set = nxp_c45_hwtstamp_set;
> priv->mii_ts.hwtstamp_get = nxp_c45_hwtstamp_get;
> priv->mii_ts.ts_info = nxp_c45_ts_info;
> phydev->mii_ts = &priv->mii_ts;
>
> So it looks like in phy_mii_ioctl(), the conditions:
>
> case SIOCSHWTSTAMP:
> if (phydev->mii_ts && phydev->mii_ts->hwtstamp_set) {
>
> are fulfilled, and
>
> ret = phydev->mii_ts->hwtstamp_set(phydev->mii_ts,
> &kernel_cfg,
> &extack);
>
> will happen.
>
> Now, this combination of MAC and PHY is very unlikely but it proves
> the point. As far as i remember, Richard added this code for the
> dp83640 PHY device, but i don't remember what MAC driver it was paired
> with. He wanted to make PHY support just work without the MAC driver
> even caring.
looks like it won't work now. we have to create helpers in phy to fix it.
I can work on it, but I don't have such HW combination to test. Do you have some
HW to test this combination?
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] net/sched: sch_fq_pie: add per-flow statistics via class ops
From: Jakub Kicinski @ 2026-07-20 22:46 UTC (permalink / raw)
To: Hemendra M. Naik
Cc: netdev, davem, edumazet, pabeni, horms, jiri, jhs, shuah,
linux-kernel, linux-kselftest, vishy0777, tahiliani
In-Reply-To: <20260630183702.170798-2-hemendranaik@gmail.com>
On Wed, 1 Jul 2026 00:07:01 +0530 Hemendra M. Naik wrote:
> FQ-PIE schedules independent PIE controllers per flow but exposes no
> per-flow AQM state. Without class-level statistics there is no way to
> observe the per-flow drop probability, queue delay, deficit or
> dequeue rate from userspace.
Please un-indent the commit msg.
^ permalink raw reply
* Re: [PATCH net-next v9 3/3] net: airoha: defer GDM3/GDM4 WAN mode and GDM2 loopback to QoS offload
From: Jacob Keller @ 2026-07-20 22:48 UTC (permalink / raw)
To: Lorenzo Bianconi, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Alexander Lobakin, linux-arm-kernel, linux-mediatek,
netdev, Madhur Agrawal
In-Reply-To: <20260721-airoha-ethtool-priv_flags-v9-3-9c15d8b71a56@kernel.org>
On 7/20/2026 3:03 PM, Lorenzo Bianconi wrote:
> GDM3 and GDM4 ports require GDM2 loopback to be enabled for hardware
> QoS offload to function. Without it, HTB and ETS offload on these ports
> do not work.
> Previously, GDM3/GDM4 ports were automatically configured as WAN with
> GDM2 loopback enabled during ndo_init(). Add the capability to configure
> GDM3/GDM4 as WAN/LAN on demand when QoS offload is created or destroyed.
> Hook airoha_enable_qos_for_gdm34() into TC_HTB_CREATE so that requesting
> HTB offload on a GDM3/GDM4 LAN port switches it to WAN mode and enables
> GDM2 loopback, with proper rollback on failure. Introduce the
> AIROHA_DEV_F_QOS flag to track whether a device has an active HTB
> qdisc; clear it on TC_HTB_DESTROY. The device keeps its WAN role after
> qdisc teardown so that its configuration is preserved until another
> device explicitly needs the WAN role for QoS offload.
> If another GDM3/GDM4 device already holds the WAN role without an active
> QoS qdisc, demote it to LAN before promoting the requesting device. Skip
> the demotion when the requesting device is itself already the WAN device.
> Since airoha_dev_set_qdma() can now be called on a running device to
> migrate between QDMA blocks, make dev->qdma an RCU pointer so the TX
> path can safely dereference it without holding RTNL.
> Hold flow_offload_mutex in airoha_enable_qos_for_gdm34() and
> airoha_disable_qos_for_gdm34() around the dev->flags update,
> airoha_dev_set_qdma() and GDM2 loopback configuration, serializing
> against concurrent airoha_ppe_hw_init() in the TC_SETUP_CLSFLOWER
> offload path.
> Introduce airoha_qdma_deref() helper that wraps rcu_dereference_protected()
> with a lockdep condition accepting either rtnl_lock or flow_offload_mutex,
> and use it across all control-path dereferences of the RCU-protected
> dev->qdma pointer.
> Add airoha_disable_gdm2_loopback() to disable GDM2 hw loopback.
>
A minor nit which may just be my personal preference/style:
I had trouble following this commit message since it goes through a lot
of detail about various problems with dereferencing and other changes
related to allowing the defered configuration of WAN mode.
I do appreciate this detail as it helps understand the changes and
motivations. However.. It might benefit from some additional line breaks
for spacing to help readability.
^ permalink raw reply
* [syzbot] [crypto?] possible deadlock in __alloc_workqueue (3)
From: syzbot @ 2026-07-20 22:51 UTC (permalink / raw)
To: daniel.m.jordan, linux-crypto, linux-kernel, netdev,
steffen.klassert, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: f6f3b36c15ed net: ethernet: qualcomm: remove unneeded 'fas..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=152cacb9580000
kernel config: https://syzkaller.appspot.com/x/.config?x=5c4196ba0e33631d
dashboard link: https://syzkaller.appspot.com/bug?extid=5f117b3024eab6a14c35
compiler: Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/47ae09206bf6/disk-f6f3b36c.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/9e3daf94ae30/vmlinux-f6f3b36c.xz
kernel image: https://storage.googleapis.com/syzbot-assets/c266cfd24092/bzImage-f6f3b36c.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5f117b3024eab6a14c35@syzkaller.appspotmail.com
wlan1 speed is unknown, defaulting to 1000
wlan1 speed is unknown, defaulting to 1000
wlan1 speed is unknown, defaulting to 1000
infiniband syz1: set down
infiniband syz1: added wlan1
======================================================
WARNING: possible circular locking dependency detected
syzkaller #0 Not tainted
------------------------------------------------------
syz.1.835/9438 is trying to acquire lock:
ffffffff8ec848a0 (fs_reclaim){+.+.}-{0:0}, at: might_alloc include/linux/sched/mm.h:317 [inline]
ffffffff8ec848a0 (fs_reclaim){+.+.}-{0:0}, at: slab_pre_alloc_hook mm/slub.c:4565 [inline]
ffffffff8ec848a0 (fs_reclaim){+.+.}-{0:0}, at: slab_alloc_node mm/slub.c:4925 [inline]
ffffffff8ec848a0 (fs_reclaim){+.+.}-{0:0}, at: __do_kmalloc_node mm/slub.c:5361 [inline]
ffffffff8ec848a0 (fs_reclaim){+.+.}-{0:0}, at: __kmalloc_noprof+0xbc/0x720 mm/slub.c:5387
but task is already holding lock:
ffffffff8e9f6d00 (wq_pool_mutex){+.+.}-{4:4}, at: __alloc_workqueue+0xa9c/0x2060 kernel/workqueue.c:5895
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #7 (wq_pool_mutex){+.+.}-{4:4}:
__mutex_lock_common kernel/locking/mutex.c:646 [inline]
__mutex_lock+0x19d/0x1550 kernel/locking/mutex.c:821
__alloc_workqueue+0xa9c/0x2060 kernel/workqueue.c:5895
alloc_workqueue_va kernel/workqueue.c:5946 [inline]
alloc_workqueue_noprof+0xe3/0x210 kernel/workqueue.c:5962
padata_alloc+0xbe/0x360 kernel/padata.c:964
pcrypt_init_padata+0x27/0x100 crypto/pcrypt.c:335
pcrypt_init+0x60/0xc0 crypto/pcrypt.c:360
do_one_initcall+0x250/0x870 init/main.c:1347
do_initcall_level+0x10a/0x1a0 init/main.c:1409
do_initcalls+0x59/0xa0 init/main.c:1425
kernel_init_freeable+0x29d/0x3e0 init/main.c:1658
kernel_init+0x1d/0x1d0 init/main.c:1548
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
-> #6 (cpu_hotplug_lock){++++}-{0:0}:
percpu_down_read_internal include/linux/percpu-rwsem.h:53 [inline]
percpu_down_read include/linux/percpu-rwsem.h:77 [inline]
cpus_read_lock+0x42/0x160 kernel/cpu.c:490
static_key_slow_inc+0x12/0x30 kernel/jump_label.c:190
nbd_reconnect_socket drivers/block/nbd.c:1379 [inline]
nbd_genl_reconfigure+0x1301/0x1e80 drivers/block/nbd.c:2468
genl_family_rcv_msg_doit+0x233/0x340 net/netlink/genetlink.c:1114
genl_family_rcv_msg net/netlink/genetlink.c:1194 [inline]
genl_rcv_msg+0x614/0x7a0 net/netlink/genetlink.c:1209
netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2556
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1218
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x7bb/0x940 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
sock_sendmsg_nosec+0x13a/0x180 net/socket.c:775
__sock_sendmsg net/socket.c:790 [inline]
____sys_sendmsg+0x54e/0x850 net/socket.c:2684
___sys_sendmsg+0x2a5/0x360 net/socket.c:2738
__sys_sendmsg net/socket.c:2770 [inline]
__do_sys_sendmsg net/socket.c:2775 [inline]
__se_sys_sendmsg net/socket.c:2773 [inline]
__x64_sys_sendmsg+0x1b1/0x290 net/socket.c:2773
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
-> #5 (&nsock->tx_lock){+.+.}-{4:4}:
__mutex_lock_common kernel/locking/mutex.c:646 [inline]
__mutex_lock+0x19d/0x1550 kernel/locking/mutex.c:821
nbd_handle_cmd drivers/block/nbd.c:1143 [inline]
nbd_queue_rq+0x373/0x1150 drivers/block/nbd.c:1207
blk_mq_dispatch_rq_list+0x499/0x1990 block/blk-mq.c:2117
__blk_mq_do_dispatch_sched block/blk-mq-sched.c:168 [inline]
blk_mq_do_dispatch_sched block/blk-mq-sched.c:182 [inline]
__blk_mq_sched_dispatch_requests+0xd36/0x1580 block/blk-mq-sched.c:307
blk_mq_sched_dispatch_requests+0xd7/0x190 block/blk-mq-sched.c:329
blk_mq_run_work_fn+0x16c/0x300 block/blk-mq.c:2532
process_one_work kernel/workqueue.c:3322 [inline]
process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
worker_thread+0xa47/0xfb0 kernel/workqueue.c:3486
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
-> #4 (&cmd->lock){+.+.}-{4:4}:
__mutex_lock_common kernel/locking/mutex.c:646 [inline]
__mutex_lock+0x19d/0x1550 kernel/locking/mutex.c:821
nbd_queue_rq+0xc1/0x1150 drivers/block/nbd.c:1199
blk_mq_dispatch_rq_list+0x499/0x1990 block/blk-mq.c:2117
__blk_mq_do_dispatch_sched block/blk-mq-sched.c:168 [inline]
blk_mq_do_dispatch_sched block/blk-mq-sched.c:182 [inline]
__blk_mq_sched_dispatch_requests+0xd36/0x1580 block/blk-mq-sched.c:307
blk_mq_sched_dispatch_requests+0xd7/0x190 block/blk-mq-sched.c:329
blk_mq_run_work_fn+0x16c/0x300 block/blk-mq.c:2532
process_one_work kernel/workqueue.c:3322 [inline]
process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
worker_thread+0xa47/0xfb0 kernel/workqueue.c:3486
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
-> #3 (set->srcu){.+.+}-{0:0}:
srcu_lock_sync include/linux/srcu.h:199 [inline]
__synchronize_srcu+0xc9/0x2f0 kernel/rcu/srcutree.c:1481
elevator_switch+0x1e8/0x7b0 block/elevator.c:576
elevator_change+0x2fa/0x480 block/elevator.c:681
elevator_set_default+0x375/0x440 block/elevator.c:754
blk_register_queue+0x3f3/0x4e0 block/blk-sysfs.c:992
__add_disk+0x6cb/0xe30 block/genhd.c:528
add_disk_fwnode+0xfb/0x4b0 block/genhd.c:597
add_disk include/linux/blkdev.h:800 [inline]
nbd_dev_add+0x733/0xb60 drivers/block/nbd.c:2021
nbd_init+0x15f/0x1e0 drivers/block/nbd.c:2729
do_one_initcall+0x250/0x870 init/main.c:1347
do_initcall_level+0x10a/0x1a0 init/main.c:1409
do_initcalls+0x59/0xa0 init/main.c:1425
kernel_init_freeable+0x29d/0x3e0 init/main.c:1658
kernel_init+0x1d/0x1d0 init/main.c:1548
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
-> #2 (&q->elevator_lock){+.+.}-{4:4}:
__mutex_lock_common kernel/locking/mutex.c:646 [inline]
__mutex_lock+0x19d/0x1550 kernel/locking/mutex.c:821
elevator_change+0x1af/0x480 block/elevator.c:679
elevator_set_none+0xb5/0x140 block/elevator.c:769
blk_mq_elv_switch_none block/blk-mq.c:5101 [inline]
__blk_mq_update_nr_hw_queues block/blk-mq.c:5146 [inline]
blk_mq_update_nr_hw_queues+0x5ef/0x19f0 block/blk-mq.c:5211
nbd_start_device+0x189/0xb30 drivers/block/nbd.c:1526
nbd_genl_connect+0x1597/0x1c10 drivers/block/nbd.c:2276
genl_family_rcv_msg_doit+0x233/0x340 net/netlink/genetlink.c:1114
genl_family_rcv_msg net/netlink/genetlink.c:1194 [inline]
genl_rcv_msg+0x614/0x7a0 net/netlink/genetlink.c:1209
netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2556
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1218
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x7bb/0x940 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
sock_sendmsg_nosec+0x13a/0x180 net/socket.c:775
__sock_sendmsg net/socket.c:790 [inline]
____sys_sendmsg+0x54e/0x850 net/socket.c:2684
___sys_sendmsg+0x2a5/0x360 net/socket.c:2738
__sys_sendmsg net/socket.c:2770 [inline]
__do_sys_sendmsg net/socket.c:2775 [inline]
__se_sys_sendmsg net/socket.c:2773 [inline]
__x64_sys_sendmsg+0x1b1/0x290 net/socket.c:2773
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
-> #1 (&q->q_usage_counter(io)#50){++++}-{0:0}:
blk_alloc_queue+0x544/0x690 block/blk-core.c:504
blk_mq_alloc_queue block/blk-mq.c:4420 [inline]
__blk_mq_alloc_disk+0x194/0x390 block/blk-mq.c:4467
nbd_dev_add+0x494/0xb60 drivers/block/nbd.c:1991
nbd_init+0x15f/0x1e0 drivers/block/nbd.c:2729
do_one_initcall+0x250/0x870 init/main.c:1347
do_initcall_level+0x10a/0x1a0 init/main.c:1409
do_initcalls+0x59/0xa0 init/main.c:1425
kernel_init_freeable+0x29d/0x3e0 init/main.c:1658
kernel_init+0x1d/0x1d0 init/main.c:1548
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
-> #0 (fs_reclaim){+.+.}-{0:0}:
check_prev_add kernel/locking/lockdep.c:3165 [inline]
check_prevs_add kernel/locking/lockdep.c:3284 [inline]
validate_chain kernel/locking/lockdep.c:3908 [inline]
__lock_acquire+0x1520/0x2cf0 kernel/locking/lockdep.c:5237
lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5868
__fs_reclaim_acquire mm/page_alloc.c:4329 [inline]
fs_reclaim_acquire+0x71/0x100 mm/page_alloc.c:4343
might_alloc include/linux/sched/mm.h:317 [inline]
slab_pre_alloc_hook mm/slub.c:4565 [inline]
slab_alloc_node mm/slub.c:4925 [inline]
__do_kmalloc_node mm/slub.c:5361 [inline]
__kmalloc_noprof+0xbc/0x720 mm/slub.c:5387
_kmalloc_noprof include/linux/slab.h:973 [inline]
_kzalloc_noprof include/linux/slab.h:1290 [inline]
apply_wqattrs_prepare+0xee/0xdc0 kernel/workqueue.c:5410
apply_workqueue_attrs_locked kernel/workqueue.c:5495 [inline]
alloc_and_link_pwqs kernel/workqueue.c:5645 [inline]
__alloc_workqueue+0x117c/0x2060 kernel/workqueue.c:5897
alloc_workqueue_va kernel/workqueue.c:5946 [inline]
alloc_workqueue_noprof+0xe3/0x210 kernel/workqueue.c:5962
ib_mad_port_open drivers/infiniband/core/mad.c:3252 [inline]
ib_mad_init_device+0x993/0x2150 drivers/infiniband/core/mad.c:3339
add_client_context+0x37c/0x7b0 drivers/infiniband/core/device.c:732
enable_device_and_get+0x19c/0x3e0 drivers/infiniband/core/device.c:1341
ib_register_device+0x10af/0x1380 drivers/infiniband/core/device.c:1468
rxe_register_device+0x1e3/0x350 drivers/infiniband/sw/rxe/rxe_verbs.c:1543
rxe_net_add+0x81/0x110 drivers/infiniband/sw/rxe/rxe_net.c:625
rxe_newlink+0xf4/0x1c0 drivers/infiniband/sw/rxe/rxe.c:243
nldev_newlink+0x5bc/0x650 drivers/infiniband/core/nldev.c:1816
rdma_nl_rcv_msg drivers/infiniband/core/netlink.c:-1 [inline]
rdma_nl_rcv_skb drivers/infiniband/core/netlink.c:239 [inline]
rdma_nl_rcv+0x6ef/0xa40 drivers/infiniband/core/netlink.c:259
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x7bb/0x940 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
sock_sendmsg_nosec+0x13a/0x180 net/socket.c:775
__sock_sendmsg net/socket.c:790 [inline]
____sys_sendmsg+0x54e/0x850 net/socket.c:2684
___sys_sendmsg+0x2a5/0x360 net/socket.c:2738
__sys_sendmsg net/socket.c:2770 [inline]
__do_sys_sendmsg net/socket.c:2775 [inline]
__se_sys_sendmsg net/socket.c:2773 [inline]
__x64_sys_sendmsg+0x1b1/0x290 net/socket.c:2773
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
other info that might help us debug this:
Chain exists of:
fs_reclaim --> cpu_hotplug_lock --> wq_pool_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(wq_pool_mutex);
lock(cpu_hotplug_lock);
lock(wq_pool_mutex);
lock(fs_reclaim);
*** DEADLOCK ***
6 locks held by syz.1.835/9438:
#0: ffffffff9ab1b1a8 (&rdma_nl_types[idx].sem){.+.+}-{4:4}, at: rdma_nl_rcv_msg drivers/infiniband/core/netlink.c:164 [inline]
#0: ffffffff9ab1b1a8 (&rdma_nl_types[idx].sem){.+.+}-{4:4}, at: rdma_nl_rcv_skb drivers/infiniband/core/netlink.c:239 [inline]
#0: ffffffff9ab1b1a8 (&rdma_nl_types[idx].sem){.+.+}-{4:4}, at: rdma_nl_rcv+0x33d/0xa40 drivers/infiniband/core/netlink.c:259
#1: ffffffff8fd8ae28 (link_ops_rwsem){++++}-{4:4}, at: nldev_newlink+0x429/0x650 drivers/infiniband/core/nldev.c:1806
#2: ffffffff8fd7c308 (devices_rwsem){++++}-{4:4}, at: enable_device_and_get+0xff/0x3e0 drivers/infiniband/core/device.c:1331
#3: ffffffff8fd7c608 (clients_rwsem){++++}-{4:4}, at: enable_device_and_get+0x165/0x3e0 drivers/infiniband/core/device.c:1339
#4: ffff888020ea0620 (&device->client_data_rwsem){++++}-{4:4}, at: add_client_context+0x33e/0x7b0 drivers/infiniband/core/device.c:730
#5: ffffffff8e9f6d00 (wq_pool_mutex){+.+.}-{4:4}, at: __alloc_workqueue+0xa9c/0x2060 kernel/workqueue.c:5895
stack backtrace:
CPU: 1 UID: 0 PID: 9438 Comm: syz.1.835 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/25/2026
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
print_circular_bug+0x2e1/0x300 kernel/locking/lockdep.c:2043
check_noncircular+0x12e/0x150 kernel/locking/lockdep.c:2175
check_prev_add kernel/locking/lockdep.c:3165 [inline]
check_prevs_add kernel/locking/lockdep.c:3284 [inline]
validate_chain kernel/locking/lockdep.c:3908 [inline]
__lock_acquire+0x1520/0x2cf0 kernel/locking/lockdep.c:5237
lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5868
__fs_reclaim_acquire mm/page_alloc.c:4329 [inline]
fs_reclaim_acquire+0x71/0x100 mm/page_alloc.c:4343
might_alloc include/linux/sched/mm.h:317 [inline]
slab_pre_alloc_hook mm/slub.c:4565 [inline]
slab_alloc_node mm/slub.c:4925 [inline]
__do_kmalloc_node mm/slub.c:5361 [inline]
__kmalloc_noprof+0xbc/0x720 mm/slub.c:5387
_kmalloc_noprof include/linux/slab.h:973 [inline]
_kzalloc_noprof include/linux/slab.h:1290 [inline]
apply_wqattrs_prepare+0xee/0xdc0 kernel/workqueue.c:5410
apply_workqueue_attrs_locked kernel/workqueue.c:5495 [inline]
alloc_and_link_pwqs kernel/workqueue.c:5645 [inline]
__alloc_workqueue+0x117c/0x2060 kernel/workqueue.c:5897
alloc_workqueue_va kernel/workqueue.c:5946 [inline]
alloc_workqueue_noprof+0xe3/0x210 kernel/workqueue.c:5962
ib_mad_port_open drivers/infiniband/core/mad.c:3252 [inline]
ib_mad_init_device+0x993/0x2150 drivers/infiniband/core/mad.c:3339
add_client_context+0x37c/0x7b0 drivers/infiniband/core/device.c:732
enable_device_and_get+0x19c/0x3e0 drivers/infiniband/core/device.c:1341
ib_register_device+0x10af/0x1380 drivers/infiniband/core/device.c:1468
rxe_register_device+0x1e3/0x350 drivers/infiniband/sw/rxe/rxe_verbs.c:1543
rxe_net_add+0x81/0x110 drivers/infiniband/sw/rxe/rxe_net.c:625
rxe_newlink+0xf4/0x1c0 drivers/infiniband/sw/rxe/rxe.c:243
nldev_newlink+0x5bc/0x650 drivers/infiniband/core/nldev.c:1816
rdma_nl_rcv_msg drivers/infiniband/core/netlink.c:-1 [inline]
rdma_nl_rcv_skb drivers/infiniband/core/netlink.c:239 [inline]
rdma_nl_rcv+0x6ef/0xa40 drivers/infiniband/core/netlink.c:259
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x7bb/0x940 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
sock_sendmsg_nosec+0x13a/0x180 net/socket.c:775
__sock_sendmsg net/socket.c:790 [inline]
____sys_sendmsg+0x54e/0x850 net/socket.c:2684
___sys_sendmsg+0x2a5/0x360 net/socket.c:2738
__sys_sendmsg net/socket.c:2770 [inline]
__do_sys_sendmsg net/socket.c:2775 [inline]
__se_sys_sendmsg net/socket.c:2773 [inline]
__x64_sys_sendmsg+0x1b1/0x290 net/socket.c:2773
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f8013f9de59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f8014d82028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f8014225fa0 RCX: 00007f8013f9de59
RDX: 0000000004000000 RSI: 00002000000002c0 RDI: 000000000000000d
RBP: 00007f8014033e6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f8014226038 R14: 00007f8014225fa0 R15: 00007ffc03c3f938
</TASK>
smbdirect: ib_dev[syz1]: added: IB_CA max_fast_reg_page_list_len=512 device_cap_flags=0x1c001223c76 kernel_cap_flags=0x14 page_size_cap=0xfffff000
smbdirect: ib_dev[syz1]: num_ports=1 max_qp_rd_atom=128 max_qp_init_rd_atom=128 max_sgl_rd=0 max_sge_rd=32 max_cqe=32767 max_qp_wr=1048576 max_send_sge=32 max_recv_sge=32
smbdirect: ib_dev[syz1]PORT[1]: iwarp=0 ib=0 roce=1 v1=0 v2=1 core_cap_flags=0x803005
RDS/IB: syz1: added
smc: adding ib device syz1 with port count 1
smc: ib device syz1 port 1 has no pnetid
wlan1 speed is unknown, defaulting to 1000
wlan1 speed is unknown, defaulting to 1000
wlan1 speed is unknown, defaulting to 1000
wlan1 speed is unknown, defaulting to 1000
wlan1 speed is unknown, defaulting to 1000
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* Re: [PATCH v6 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
From: Nnamdi Onyeyiri @ 2026-07-20 22:53 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
linux-kernel
In-Reply-To: <DK3PKY6RN5FI.4DKPSHWNEBHM@etsalapatis.com>
On Mon, Jul 20, 2026 at 05:16:08PM -0400, Emil Tsalapatis wrote:
> On Mon Jul 20, 2026 at 1:15 PM EDT, Nnamdi Onyeyiri wrote:
> > recvfrom()/recv() are documented as only returning EAGAIN for blocking sockets
> > when they have a receive timeout configured. However, adding a blocking
> > ipv4 tcp socket without a receive timeout to a sockmap will cause EAGAIN errors
> > sporadically. A socket with a receive timeout may return EAGAIN before the
> > timeout expires.
> >
> > There are 2 code paths affected by this:
> >
> > 1. tcp_bpf_recvmsg() - Used when the socket has been added to a sockmap
> > that has no verdict program attached.
> >
> > 2. tcp_bpf_recvmsg_parser() - Used when the socket has been added to a
> > sockmap that has a verdict program. To reproduce this issue, it is
> > enough for the verdict program to do nothing but return SK_PASS.
> >
> > In both cases this happens when tcp_msg_wait_data() wakes spuriously
> > (returning 0). To fix it, we now loop back to msg_bytes_ready instead
> > of returning -EAGAIN on spurious wakeup.
> >
> > To ensure the looping does not cause sockets with a SO_RCVTIMEO set to
> > wait excessively long, tcp_msg_wait_data() now takes a pointer to timeo,
> > allowing sk_wait_event() to update it as appropriate.
> >
> > The logic in tcp_bpf_recvmsg_parser() that allow it to handle signals,
> > socket errors and closuers in its loop was also added to tcp_bpf_recvmsg().
> >
> > Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
> > ---
> > net/ipv4/tcp_bpf.c | 69 ++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 60 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> > index cc0bd73f36b6..aa5c5d741599 100644
> > --- a/net/ipv4/tcp_bpf.c
> > +++ b/net/ipv4/tcp_bpf.c
> > @@ -179,7 +179,7 @@ EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
> >
> > #ifdef CONFIG_BPF_SYSCALL
> > static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
> > - long timeo)
> > + long *timeo)
> > {
> > DEFINE_WAIT_FUNC(wait, woken_wake_function);
> > int ret = 0;
> > @@ -187,12 +187,12 @@ static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
> > if (sk->sk_shutdown & RCV_SHUTDOWN)
> > return 1;
> >
> > - if (!timeo)
> > + if (!*timeo)
> > return ret;
> >
> > add_wait_queue(sk_sleep(sk), &wait);
> > sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
> > - ret = sk_wait_event(sk, &timeo,
> > + ret = sk_wait_event(sk, timeo,
> > !list_empty(&psock->ingress_msg) ||
> > !skb_queue_empty_lockless(&sk->sk_receive_queue), &wait);
> > sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
> > @@ -229,6 +229,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > int copied_from_self = 0;
> > int copied = 0;
> > u32 seq;
> > + long timeo;
> >
> > if (unlikely(flags & MSG_ERRQUEUE))
> > return inet_recv_error(sk, msg, len);
> > @@ -262,6 +263,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > }
> > }
> >
> > + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > +
> > msg_bytes_ready:
> > copied = __sk_msg_recvmsg(sk, psock, msg, len, flags, &copied_from_self);
> > /* The typical case for EFAULT is the socket was gracefully
> > @@ -280,7 +283,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > }
> > seq += copied_from_self;
> > if (!copied) {
> > - long timeo;
> > int data;
> >
> > if (sock_flag(sk, SOCK_DONE))
> > @@ -299,7 +301,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > goto out;
> > }
> >
> > - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > if (!timeo) {
> > copied = -EAGAIN;
> > goto out;
> > @@ -310,13 +311,15 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > goto out;
> > }
> >
> > - data = tcp_msg_wait_data(sk, psock, timeo);
> > + data = tcp_msg_wait_data(sk, psock, &timeo);
> > if (data < 0) {
> > copied = data;
> > goto unlock;
> > }
> > if (data && !sk_psock_queue_empty(psock))
> > goto msg_bytes_ready;
> > + if (!data && timeo > 0)
> > + goto msg_bytes_ready;
> > copied = -EAGAIN;
> > }
> > out:
> > @@ -355,6 +358,7 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > {
> > struct sk_psock *psock;
> > int copied, ret;
> > + long timeo;
> >
> > if (unlikely(flags & MSG_ERRQUEUE))
> > return inet_recv_error(sk, msg, len);
> > @@ -371,14 +375,59 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > return tcp_recvmsg(sk, msg, len, flags);
> > }
> > lock_sock(sk);
> > +
> > + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > +
> > msg_bytes_ready:
> > copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> > if (!copied) {
> > - long timeo;
> > int data;
> >
> > - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > - data = tcp_msg_wait_data(sk, psock, timeo);
> > + if (sock_flag(sk, SOCK_DONE)) {
> > + ret = 0;
> > + goto unlock;
> > + }
> > +
> > + if (sk->sk_err) {
> > + if (!sk_psock_queue_empty(psock))
> > + goto msg_bytes_ready;
> > + if (!skb_queue_empty(&sk->sk_receive_queue)) {
> > + release_sock(sk);
> > + sk_psock_put(sk, psock);
> > + return tcp_recvmsg(sk, msg, len, flags);
> > + }
> > + ret = sock_error(sk);
> > + goto unlock;
> > + }
> > +
> > + if (sk->sk_shutdown & RCV_SHUTDOWN) {
> > + if (!sk_psock_queue_empty(psock))
> > + goto msg_bytes_ready;
> > + if (!skb_queue_empty(&sk->sk_receive_queue)) {
> > + release_sock(sk);
> > + sk_psock_put(sk, psock);
> > + return tcp_recvmsg(sk, msg, len, flags);
> > + }
> > + ret = 0;
> > + goto unlock;
>
> These two error handling routines above look identical. Can you refactor
> them?
>
Will do. My understanding is the same logic is needed to address the
issue Sashiko raised with the SOCK_DONE check as well.
> > + }
> > +
> > + if (sk->sk_state == TCP_CLOSE) {
> > + ret = -ENOTCONN;
> > + goto unlock;
> > + }
> > +
> > + if (!timeo) {
> > + ret = -EAGAIN;
> > + goto unlock;
> > + }
> > +
>
> Since this handling (which Sashiko flags by the way, correctly AFAICT)
> are taken from tcp_bpf_recvmsg, there is obvious overlap between the two
> functions. Please factor those out so that they share the logic between
> them.
>
> pw-bot: cr
>
Sashiko highlighted the "if (!timeo)" and signal_pending early returns
when MSG_DONTWAIT is set, but I think I'm missing part of the picture.
By the time we reach these branches, haven't we already checked for data
in sk_receive_queue (line 372, after the patch is applied to 7.2-rc2)
[copied below for ease of viewing]:
if (!skb_queue_empty(&sk->sk_receive_queue) &&
sk_psock_queue_empty(psock)) {
sk_psock_put(sk, psock);
return tcp_recvmsg(sk, msg, len, flags);
}
and in the psock (line 382) [again copied below for viewing]:
copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> > + if (signal_pending(current)) {
> > + ret = sock_intr_errno(timeo);
> > + goto unlock;
> > + }
> > +
> > + data = tcp_msg_wait_data(sk, psock, &timeo);
> > if (data < 0) {
> > ret = data;
> > goto unlock;
> > @@ -390,6 +439,8 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > sk_psock_put(sk, psock);
> > return tcp_recvmsg(sk, msg, len, flags);
> > }
> > + if (!data && timeo > 0)
> > + goto msg_bytes_ready;
> > copied = -EAGAIN;
> > }
> > ret = copied;
>
^ permalink raw reply
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox