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: 34c21fa894a1 ("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]>
---
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 14 +++++---------
drivers/net/ethernet/intel/idpf/idpf_virtchnl.h | 5 +++--
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 113ecfc16dd7..582e0c8e9dc0 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -287,26 +287,21 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter, struct
idpf_ctlq_info *asq,
return err;
}
-/* API for virtchnl "transaction" support ("xn" for short).
- *
- * We are reusing the completion lock to serialize the accesses to the
- * transaction state for simplicity, but it could be its own separate synchro
- * as well. For now, this API is only used from within a workqueue context;
- * raw_spin_lock() is enough.
- */
+/* API for virtchnl "transaction" support ("xn" for short). */
+
/**
* idpf_vc_xn_lock - Request exclusive access to vc transaction
* @xn: struct idpf_vc_xn* to access
*/
#define idpf_vc_xn_lock(xn) \
- raw_spin_lock(&(xn)->completed.wait.lock)
+ spin_lock(&(xn)->lock)
/**
* idpf_vc_xn_unlock - Release exclusive access to vc transaction
* @xn: struct idpf_vc_xn* to access
*/
#define idpf_vc_xn_unlock(xn) \
- raw_spin_unlock(&(xn)->completed.wait.lock)
+ spin_unlock(&(xn)->lock)
/**
* 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)
xn->state = IDPF_VC_XN_IDLE;
xn->idx = i;
idpf_vc_xn_release_bufs(xn);
+ spin_lock_init(&xn->lock);
init_completion(&xn->completed);
}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
index fe065911ad5a..6876e3ed9d1b 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
@@ -42,8 +42,8 @@ typedef int (*async_vc_cb) (struct idpf_adapter *, struct
idpf_vc_xn *,
* struct idpf_vc_xn - Data structure representing virtchnl transactions
* @completed: virtchnl event loop uses that to signal when a reply is
* available, uses kernel completion API
- * @state: virtchnl event loop stores the data below, protected by the
- * completion's lock.
+ * @lock: protects the transaction state fields below
+ * @state: virtchnl event loop stores the data below, protected by @lock
* @reply_sz: Original size of reply, may be > reply_buf.iov_len; it will be
* truncated on its way to the receiver thread according to
* reply_buf.iov_len.
@@ -58,6 +58,7 @@ typedef int (*async_vc_cb) (struct idpf_adapter *, struct
idpf_vc_xn *,
*/
struct idpf_vc_xn {
struct completion completed;
+ spinlock_t lock;
enum idpf_vc_xn_state state;
size_t reply_sz;
struct kvec reply;
--
2.37.3
