Skip to content

Commit e02c974

Browse files
etantilovgregkh
authored andcommitted
idpf: fix PREEMPT_RT raw/bh spinlock nesting for async VC handling
commit 5914781 upstream. 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: stable@vger.kernel.org Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Reported-by: Ray Zhang <sgzhang@google.com> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Tested-by: Samuel Salin <Samuel.salin@intel.com> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 576543b commit e02c974

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
@@ -284,26 +284,21 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter, u32 op,
284284
return err;
285285
}
286286

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

301296
/**
302297
* idpf_vc_xn_unlock - Release exclusive access to vc transaction
303298
* @xn: struct idpf_vc_xn* to access
304299
*/
305300
#define idpf_vc_xn_unlock(xn) \
306-
raw_spin_unlock(&(xn)->completed.wait.lock)
301+
spin_unlock(&(xn)->lock)
307302

308303
/**
309304
* idpf_vc_xn_release_bufs - Release reference to reply buffer(s) and
@@ -335,6 +330,7 @@ static void idpf_vc_xn_init(struct idpf_vc_xn_manager *vcxn_mngr)
335330
xn->state = IDPF_VC_XN_IDLE;
336331
xn->idx = i;
337332
idpf_vc_xn_release_bufs(xn);
333+
spin_lock_init(&xn->lock);
338334
init_completion(&xn->completed);
339335
}
340336

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)