rxrpc, afs: Fix peer hash locking vs RCU callback
JIRA: https://issues.redhat.com/browse/RHEL-78217 commit 79d458c13056559d49b5e41fbc4b6890e68cf65b Author: David Howells <dhowells@redhat.com> Date: Thu Jan 23 08:59:12 2025 +0000 rxrpc, afs: Fix peer hash locking vs RCU callback In its address list, afs now retains pointers to and refs on one or more rxrpc_peer objects. The address list is freed under RCU and at this time, it puts the refs on those peers. Now, when an rxrpc_peer object runs out of refs, it gets removed from the peer hash table and, for that, rxrpc has to take a spinlock. However, it is now being called from afs's RCU cleanup, which takes place in BH context - but it is just taking an ordinary spinlock. The put may also be called from non-BH context, and so there exists the possibility of deadlock if the BH-based RCU cleanup happens whilst the hash spinlock is held. This led to the attached lockdep complaint. Fix this by changing spinlocks of rxnet->peer_hash_lock back to BH-disabling locks. ================================ WARNING: inconsistent lock state 6.13.0-rc5-build2+ #1223 Tainted: G E -------------------------------- inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. swapper/1/0 [HC0[0]:SC1[1]:HE1:SE0] takes: ffff88810babe228 (&rxnet->peer_hash_lock){+.?.}-{3:3}, at: rxrpc_put_peer+0xcb/0x180 {SOFTIRQ-ON-W} state was registered at: mark_usage+0x164/0x180 __lock_acquire+0x544/0x990 lock_acquire.part.0+0x103/0x280 _raw_spin_lock+0x2f/0x40 rxrpc_peer_keepalive_worker+0x144/0x440 process_one_work+0x486/0x7c0 process_scheduled_works+0x73/0x90 worker_thread+0x1c8/0x2a0 kthread+0x19b/0x1b0 ret_from_fork+0x24/0x40 ret_from_fork_asm+0x1a/0x30 irq event stamp: 972402 hardirqs last enabled at (972402): [<ffffffff8244360e>] _raw_spin_unlock_irqrestore+0x2e/0x50 hardirqs last disabled at (972401): [<ffffffff82443328>] _raw_spin_lock_irqsave+0x18/0x60 softirqs last enabled at (972300): [<ffffffff810ffbbe>] handle_softirqs+0x3ee/0x430 softirqs last disabled at (972313): [<ffffffff810ffc54>] __irq_exit_rcu+0x44/0x110 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&rxnet->peer_hash_lock); <Interrupt> lock(&rxnet->peer_hash_lock); *** DEADLOCK *** 1 lock held by swapper/1/0: #0: ffffffff83576be0 (rcu_callback){....}-{0:0}, at: rcu_lock_acquire+0x7/0x30 stack backtrace: CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: G E 6.13.0-rc5-build2+ #1223 Tainted: [E]=UNSIGNED_MODULE Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014 Call Trace: <IRQ> dump_stack_lvl+0x57/0x80 print_usage_bug.part.0+0x227/0x240 valid_state+0x53/0x70 mark_lock_irq+0xa5/0x2f0 mark_lock+0xf7/0x170 mark_usage+0xe1/0x180 __lock_acquire+0x544/0x990 lock_acquire.part.0+0x103/0x280 _raw_spin_lock+0x2f/0x40 rxrpc_put_peer+0xcb/0x180 afs_free_addrlist+0x46/0x90 [kafs] rcu_do_batch+0x2d2/0x640 rcu_core+0x2f7/0x350 handle_softirqs+0x1ee/0x430 __irq_exit_rcu+0x44/0x110 irq_exit_rcu+0xa/0x30 sysvec_apic_timer_interrupt+0x7f/0xa0 </IRQ> Fixes: 72904d7b9bfb ("rxrpc, afs: Allow afs to pin rxrpc_peer objects") Signed-off-by: David Howells <dhowells@redhat.com> cc: Marc Dionne <marc.dionne@auristor.com> cc: Simon Horman <horms@kernel.org> cc: linux-afs@lists.infradead.org Link: https://patch.msgid.link/2095618.1737622752@warthog.procyon.org.uk Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Marc Dionne <mdionne@redhat.com>
This commit is contained in:
Marc Dionne
parent
9440d73673
commit
14d4891d8e
|
|
@ -240,7 +240,7 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
|
|||
bool use;
|
||||
int slot;
|
||||
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
|
||||
while (!list_empty(collector)) {
|
||||
peer = list_entry(collector->next,
|
||||
|
|
@ -251,7 +251,7 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
|
|||
continue;
|
||||
|
||||
use = __rxrpc_use_local(peer->local, rxrpc_local_use_peer_keepalive);
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
|
||||
if (use) {
|
||||
keepalive_at = peer->last_tx_at + RXRPC_KEEPALIVE_TIME;
|
||||
|
|
@ -271,17 +271,17 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
|
|||
*/
|
||||
slot += cursor;
|
||||
slot &= mask;
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
list_add_tail(&peer->keepalive_link,
|
||||
&rxnet->peer_keepalive[slot & mask]);
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
rxrpc_unuse_local(peer->local, rxrpc_local_unuse_peer_keepalive);
|
||||
}
|
||||
rxrpc_put_peer(peer, rxrpc_peer_put_keepalive);
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
}
|
||||
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -311,7 +311,7 @@ void rxrpc_peer_keepalive_worker(struct work_struct *work)
|
|||
* second; the bucket at cursor + 1 goes at now + 1s and so
|
||||
* on...
|
||||
*/
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
list_splice_init(&rxnet->peer_keepalive_new, &collector);
|
||||
|
||||
stop = cursor + ARRAY_SIZE(rxnet->peer_keepalive);
|
||||
|
|
@ -323,7 +323,7 @@ void rxrpc_peer_keepalive_worker(struct work_struct *work)
|
|||
}
|
||||
|
||||
base = now;
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
|
||||
rxnet->peer_keepalive_base = base;
|
||||
rxnet->peer_keepalive_cursor = cursor;
|
||||
|
|
|
|||
|
|
@ -313,10 +313,10 @@ void rxrpc_new_incoming_peer(struct rxrpc_local *local, struct rxrpc_peer *peer)
|
|||
hash_key = rxrpc_peer_hash_key(local, &peer->srx);
|
||||
rxrpc_init_peer(local, peer, hash_key);
|
||||
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
hash_add_rcu(rxnet->peer_hash, &peer->hash_link, hash_key);
|
||||
list_add_tail(&peer->keepalive_link, &rxnet->peer_keepalive_new);
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -348,7 +348,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
|
|||
return NULL;
|
||||
}
|
||||
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
|
||||
/* Need to check that we aren't racing with someone else */
|
||||
peer = __rxrpc_lookup_peer_rcu(local, srx, hash_key);
|
||||
|
|
@ -361,7 +361,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
|
|||
&rxnet->peer_keepalive_new);
|
||||
}
|
||||
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
|
||||
if (peer)
|
||||
rxrpc_free_peer(candidate);
|
||||
|
|
@ -411,10 +411,10 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
|
|||
|
||||
ASSERT(hlist_empty(&peer->error_targets));
|
||||
|
||||
spin_lock(&rxnet->peer_hash_lock);
|
||||
spin_lock_bh(&rxnet->peer_hash_lock);
|
||||
hash_del_rcu(&peer->hash_link);
|
||||
list_del_init(&peer->keepalive_link);
|
||||
spin_unlock(&rxnet->peer_hash_lock);
|
||||
spin_unlock_bh(&rxnet->peer_hash_lock);
|
||||
|
||||
rxrpc_free_peer(peer);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue