Skip to content

Commit 5914781

Browse files
etantilovanguy11
authored andcommitted
idpf: fix PREEMPT_RT raw/bh spinlock nesting for async VC handling
Switch from using the completion's raw spinlock to a local lock in the idpf_vc_xn struct. The conversion is safe because complete/_all() are called outside the lock and there is no reason to share the completion lock in the current logic. This avoids invalid wait context reported by the kernel due to the async handler taking BH spinlock: [ 805.726977] ============================= [ 805.726991] [ BUG: Invalid wait context ] [ 805.727006] 7.0.0-rc2-net-devq-031026+ #28 Tainted: G S OE [ 805.727026] ----------------------------- [ 805.727038] kworker/u261:0/572 is trying to lock: [ 805.727051] ff190da6a8dbb6a0 (&vport_config->mac_filter_list_lock){+...}-{3:3}, at: idpf_mac_filter_async_handler+0xe9/0x260 [idpf] [ 805.727099] other info that might help us debug this: [ 805.727111] context-{5:5} [ 805.727119] 3 locks held by kworker/u261:0/572: [ 805.727132] #0: ff190da6db3e6148 ((wq_completion)idpf-0000:83:00.0-mbx){+.+.}-{0:0}, at: process_one_work+0x4b5/0x730 [ 805.727163] #1: ff3c6f0a6131fe50 ((work_completion)(&(&adapter->mbx_task)->work)){+.+.}-{0:0}, at: process_one_work+0x1e5/0x730 [ 805.727191] #2: ff190da765190020 (&x->wait#34){+.+.}-{2:2}, at: idpf_recv_mb_msg+0xc8/0x710 [idpf] [ 805.727218] stack backtrace: ... [ 805.727238] Workqueue: idpf-0000:83:00.0-mbx idpf_mbx_task [idpf] [ 805.727247] Call Trace: [ 805.727249] <TASK> [ 805.727251] dump_stack_lvl+0x77/0xb0 [ 805.727259] __lock_acquire+0xb3b/0x2290 [ 805.727268] ? __irq_work_queue_local+0x59/0x130 [ 805.727275] lock_acquire+0xc6/0x2f0 [ 805.727277] ? idpf_mac_filter_async_handler+0xe9/0x260 [idpf] [ 805.727284] ? _printk+0x5b/0x80 [ 805.727290] _raw_spin_lock_bh+0x38/0x50 [ 805.727298] ? idpf_mac_filter_async_handler+0xe9/0x260 [idpf] [ 805.727303] idpf_mac_filter_async_handler+0xe9/0x260 [idpf] [ 805.727310] idpf_recv_mb_msg+0x1c8/0x710 [idpf] [ 805.727317] process_one_work+0x226/0x730 [ 805.727322] worker_thread+0x19e/0x340 [ 805.727325] ? __pfx_worker_thread+0x10/0x10 [ 805.727328] kthread+0xf4/0x130 [ 805.727333] ? __pfx_kthread+0x10/0x10 [ 805.727336] ret_from_fork+0x32c/0x410 [ 805.727345] ? __pfx_kthread+0x10/0x10 [ 805.727347] ret_from_fork_asm+0x1a/0x30 [ 805.727354] </TASK> Fixes: 34c21fa ("idpf: implement virtchnl transaction manager") Cc: [email protected] Suggested-by: Sebastian Andrzej Siewior <[email protected]> Reported-by: Ray Zhang <[email protected]> Signed-off-by: Emil Tantilov <[email protected]> Reviewed-by: Aleksandr Loktionov <[email protected]> Acked-by: Sebastian Andrzej Siewior <[email protected]> Tested-by: Samuel Salin <[email protected]> Signed-off-by: Tony Nguyen <[email protected]>
1 parent 1caa871 commit 5914781

2 files changed

Lines changed: 8 additions & 11 deletions

File tree

drivers/net/ethernet/intel/idpf/idpf_virtchnl.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -287,26 +287,21 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter, struct idpf_ctlq_info *asq,
287287
return err;
288288
}
289289

290-
/* API for virtchnl "transaction" support ("xn" for short).
291-
*
292-
* We are reusing the completion lock to serialize the accesses to the
293-
* transaction state for simplicity, but it could be its own separate synchro
294-
* as well. For now, this API is only used from within a workqueue context;
295-
* raw_spin_lock() is enough.
296-
*/
290+
/* API for virtchnl "transaction" support ("xn" for short). */
291+
297292
/**
298293
* idpf_vc_xn_lock - Request exclusive access to vc transaction
299294
* @xn: struct idpf_vc_xn* to access
300295
*/
301296
#define idpf_vc_xn_lock(xn) \
302-
raw_spin_lock(&(xn)->completed.wait.lock)
297+
spin_lock(&(xn)->lock)
303298

304299
/**
305300
* idpf_vc_xn_unlock - Release exclusive access to vc transaction
306301
* @xn: struct idpf_vc_xn* to access
307302
*/
308303
#define idpf_vc_xn_unlock(xn) \
309-
raw_spin_unlock(&(xn)->completed.wait.lock)
304+
spin_unlock(&(xn)->lock)
310305

311306
/**
312307
* idpf_vc_xn_release_bufs - Release reference to reply buffer(s) and
@@ -338,6 +333,7 @@ static void idpf_vc_xn_init(struct idpf_vc_xn_manager *vcxn_mngr)
338333
xn->state = IDPF_VC_XN_IDLE;
339334
xn->idx = i;
340335
idpf_vc_xn_release_bufs(xn);
336+
spin_lock_init(&xn->lock);
341337
init_completion(&xn->completed);
342338
}
343339

drivers/net/ethernet/intel/idpf/idpf_virtchnl.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ typedef int (*async_vc_cb) (struct idpf_adapter *, struct idpf_vc_xn *,
4242
* struct idpf_vc_xn - Data structure representing virtchnl transactions
4343
* @completed: virtchnl event loop uses that to signal when a reply is
4444
* available, uses kernel completion API
45-
* @state: virtchnl event loop stores the data below, protected by the
46-
* completion's lock.
45+
* @lock: protects the transaction state fields below
46+
* @state: virtchnl event loop stores the data below, protected by @lock
4747
* @reply_sz: Original size of reply, may be > reply_buf.iov_len; it will be
4848
* truncated on its way to the receiver thread according to
4949
* reply_buf.iov_len.
@@ -58,6 +58,7 @@ typedef int (*async_vc_cb) (struct idpf_adapter *, struct idpf_vc_xn *,
5858
*/
5959
struct idpf_vc_xn {
6060
struct completion completed;
61+
spinlock_t lock;
6162
enum idpf_vc_xn_state state;
6263
size_t reply_sz;
6364
struct kvec reply;

0 commit comments

Comments
 (0)