Skip to content

Commit 5e7fee5

Browse files
azaki1smb49
azaki1
authored and
smb49
committed
idpf: convert control queue mutex to a spinlock
BugLink: https://bugs.launchpad.net/bugs/2122072 [ Upstream commit b2beb5bb2cd90d7939e470ed4da468683f41baa3 ] With VIRTCHNL2_CAP_MACFILTER enabled, the following warning is generated on module load: [ 324.701677] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:578 [ 324.701684] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1582, name: NetworkManager [ 324.701689] preempt_count: 201, expected: 0 [ 324.701693] RCU nest depth: 0, expected: 0 [ 324.701697] 2 locks held by NetworkManager/1582: [ 324.701702] #0: ffffffff9f7be770 (rtnl_mutex){....}-{3:3}, at: rtnl_newlink+0x791/0x21e0 [ 324.701730] #1: ff1100216c380368 (_xmit_ETHER){....}-{2:2}, at: __dev_open+0x3f0/0x870 [ 324.701749] Preemption disabled at: [ 324.701752] [<ffffffff9cd23b9d>] __dev_open+0x3dd/0x870 [ 324.701765] CPU: 30 UID: 0 PID: 1582 Comm: NetworkManager Not tainted 6.15.0-rc5+ #2 PREEMPT(voluntary) [ 324.701771] Hardware name: Intel Corporation M50FCP2SBSTD/M50FCP2SBSTD, BIOS SE5C741.86B.01.01.0001.2211140926 11/14/2022 [ 324.701774] Call Trace: [ 324.701777] <TASK> [ 324.701779] dump_stack_lvl+0x5d/0x80 [ 324.701788] ? __dev_open+0x3dd/0x870 [ 324.701793] __might_resched.cold+0x1ef/0x23d <..> [ 324.701818] __mutex_lock+0x113/0x1b80 <..> [ 324.701917] idpf_ctlq_clean_sq+0xad/0x4b0 [idpf] [ 324.701935] ? kasan_save_track+0x14/0x30 [ 324.701941] idpf_mb_clean+0x143/0x380 [idpf] <..> [ 324.701991] idpf_send_mb_msg+0x111/0x720 [idpf] [ 324.702009] idpf_vc_xn_exec+0x4cc/0x990 [idpf] [ 324.702021] ? rcu_is_watching+0x12/0xc0 [ 324.702035] idpf_add_del_mac_filters+0x3ed/0xb50 [idpf] <..> [ 324.702122] __hw_addr_sync_dev+0x1cf/0x300 [ 324.702126] ? find_held_lock+0x32/0x90 [ 324.702134] idpf_set_rx_mode+0x317/0x390 [idpf] [ 324.702152] __dev_open+0x3f8/0x870 [ 324.702159] ? __pfx___dev_open+0x10/0x10 [ 324.702174] __dev_change_flags+0x443/0x650 <..> [ 324.702208] netif_change_flags+0x80/0x160 [ 324.702218] do_setlink.isra.0+0x16a0/0x3960 <..> [ 324.702349] rtnl_newlink+0x12fd/0x21e0 The sequence is as follows: rtnl_newlink()-> __dev_change_flags()-> __dev_open()-> dev_set_rx_mode() - > # disables BH and grabs "dev->addr_list_lock" idpf_set_rx_mode() -> # proceed only if VIRTCHNL2_CAP_MACFILTER is ON __dev_uc_sync() -> idpf_add_mac_filter -> idpf_add_del_mac_filters -> idpf_send_mb_msg() -> idpf_mb_clean() -> idpf_ctlq_clean_sq() # mutex_lock(cq_lock) Fix by converting cq_lock to a spinlock. All operations under the new lock are safe except freeing the DMA memory, which may use vunmap(). Fix by requesting a contiguous physical memory for the DMA mapping. Fixes: a251eee ("idpf: add SRIOV support and other ndo_ops") Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com> Reviewed-by: Simon Horman <horms@kernel.org> Tested-by: Samuel Salin <Samuel.salin@intel.com> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Noah Wager <noah.wager@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
1 parent ba1af25 commit 5e7fee5

File tree

3 files changed

+20
-17
lines changed

3 files changed

+20
-17
lines changed

‎drivers/net/ethernet/intel/idpf/idpf_controlq.c‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,15 @@ static void idpf_ctlq_init_rxq_bufs(struct idpf_ctlq_info *cq)
9696
*/
9797
static void idpf_ctlq_shutdown(struct idpf_hw *hw, struct idpf_ctlq_info *cq)
9898
{
99-
mutex_lock(&cq->cq_lock);
99+
spin_lock(&cq->cq_lock);
100100

101101
/* free ring buffers and the ring itself */
102102
idpf_ctlq_dealloc_ring_res(hw, cq);
103103

104104
/* Set ring_size to 0 to indicate uninitialized queue */
105105
cq->ring_size = 0;
106106

107-
mutex_unlock(&cq->cq_lock);
108-
mutex_destroy(&cq->cq_lock);
107+
spin_unlock(&cq->cq_lock);
109108
}
110109

111110
/**
@@ -173,7 +172,7 @@ int idpf_ctlq_add(struct idpf_hw *hw,
173172

174173
idpf_ctlq_init_regs(hw, cq, is_rxq);
175174

176-
mutex_init(&cq->cq_lock);
175+
spin_lock_init(&cq->cq_lock);
177176

178177
list_add(&cq->cq_list, &hw->cq_list_head);
179178

@@ -272,7 +271,7 @@ int idpf_ctlq_send(struct idpf_hw *hw, struct idpf_ctlq_info *cq,
272271
int err = 0;
273272
int i;
274273

275-
mutex_lock(&cq->cq_lock);
274+
spin_lock(&cq->cq_lock);
276275

277276
/* Ensure there are enough descriptors to send all messages */
278277
num_desc_avail = IDPF_CTLQ_DESC_UNUSED(cq);
@@ -332,7 +331,7 @@ int idpf_ctlq_send(struct idpf_hw *hw, struct idpf_ctlq_info *cq,
332331
wr32(hw, cq->reg.tail, cq->next_to_use);
333332

334333
err_unlock:
335-
mutex_unlock(&cq->cq_lock);
334+
spin_unlock(&cq->cq_lock);
336335

337336
return err;
338337
}
@@ -364,7 +363,7 @@ int idpf_ctlq_clean_sq(struct idpf_ctlq_info *cq, u16 *clean_count,
364363
if (*clean_count > cq->ring_size)
365364
return -EBADR;
366365

367-
mutex_lock(&cq->cq_lock);
366+
spin_lock(&cq->cq_lock);
368367

369368
ntc = cq->next_to_clean;
370369

@@ -397,7 +396,7 @@ int idpf_ctlq_clean_sq(struct idpf_ctlq_info *cq, u16 *clean_count,
397396

398397
cq->next_to_clean = ntc;
399398

400-
mutex_unlock(&cq->cq_lock);
399+
spin_unlock(&cq->cq_lock);
401400

402401
/* Return number of descriptors actually cleaned */
403402
*clean_count = i;
@@ -435,7 +434,7 @@ int idpf_ctlq_post_rx_buffs(struct idpf_hw *hw, struct idpf_ctlq_info *cq,
435434
if (*buff_count > 0)
436435
buffs_avail = true;
437436

438-
mutex_lock(&cq->cq_lock);
437+
spin_lock(&cq->cq_lock);
439438

440439
if (tbp >= cq->ring_size)
441440
tbp = 0;
@@ -524,7 +523,7 @@ int idpf_ctlq_post_rx_buffs(struct idpf_hw *hw, struct idpf_ctlq_info *cq,
524523
wr32(hw, cq->reg.tail, cq->next_to_post);
525524
}
526525

527-
mutex_unlock(&cq->cq_lock);
526+
spin_unlock(&cq->cq_lock);
528527

529528
/* return the number of buffers that were not posted */
530529
*buff_count = *buff_count - i;
@@ -552,7 +551,7 @@ int idpf_ctlq_recv(struct idpf_ctlq_info *cq, u16 *num_q_msg,
552551
u16 i;
553552

554553
/* take the lock before we start messing with the ring */
555-
mutex_lock(&cq->cq_lock);
554+
spin_lock(&cq->cq_lock);
556555

557556
ntc = cq->next_to_clean;
558557

@@ -614,7 +613,7 @@ int idpf_ctlq_recv(struct idpf_ctlq_info *cq, u16 *num_q_msg,
614613

615614
cq->next_to_clean = ntc;
616615

617-
mutex_unlock(&cq->cq_lock);
616+
spin_unlock(&cq->cq_lock);
618617

619618
*num_q_msg = i;
620619
if (*num_q_msg == 0)

‎drivers/net/ethernet/intel/idpf/idpf_controlq_api.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ struct idpf_ctlq_info {
9999

100100
enum idpf_ctlq_type cq_type;
101101
int q_id;
102-
struct mutex cq_lock; /* control queue lock */
102+
spinlock_t cq_lock; /* control queue lock */
103103
/* used for interrupt processing */
104104
u16 next_to_use;
105105
u16 next_to_clean;

‎drivers/net/ethernet/intel/idpf/idpf_lib.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2328,8 +2328,12 @@ void *idpf_alloc_dma_mem(struct idpf_hw *hw, struct idpf_dma_mem *mem, u64 size)
23282328
struct idpf_adapter *adapter = hw->back;
23292329
size_t sz = ALIGN(size, 4096);
23302330

2331-
mem->va = dma_alloc_coherent(&adapter->pdev->dev, sz,
2332-
&mem->pa, GFP_KERNEL);
2331+
/* The control queue resources are freed under a spinlock, contiguous
2332+
* pages will avoid IOMMU remapping and the use vmap (and vunmap in
2333+
* dma_free_*() path.
2334+
*/
2335+
mem->va = dma_alloc_attrs(&adapter->pdev->dev, sz, &mem->pa,
2336+
GFP_KERNEL, DMA_ATTR_FORCE_CONTIGUOUS);
23332337
mem->size = sz;
23342338

23352339
return mem->va;
@@ -2344,8 +2348,8 @@ void idpf_free_dma_mem(struct idpf_hw *hw, struct idpf_dma_mem *mem)
23442348
{
23452349
struct idpf_adapter *adapter = hw->back;
23462350

2347-
dma_free_coherent(&adapter->pdev->dev, mem->size,
2348-
mem->va, mem->pa);
2351+
dma_free_attrs(&adapter->pdev->dev, mem->size,
2352+
mem->va, mem->pa, DMA_ATTR_FORCE_CONTIGUOUS);
23492353
mem->size = 0;
23502354
mem->va = NULL;
23512355
mem->pa = 0;

0 commit comments

Comments
 (0)