Skip to content

Commit df7fba6

Browse files
Jack Morgensteinrolandd
Jack Morgenstein
authored andcommitted
IB/mlx4: Fix possible deadlock on sm_lock spinlock
The sm_lock spinlock is taken in the process context by mlx4_ib_modify_device, and in the interrupt context by update_sm_ah, so we need to take that spinlock with irqsave, and release it with irqrestore. Lockdeps reports this as follows: [ INFO: inconsistent lock state ] 3.5.0+ #20 Not tainted inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes: (&(&ibdev->sm_lock)->rlock){?.+...}, at: [<ffffffffa028af1d>] update_sm_ah+0xad/0x100 [mlx4_ib] {HARDIRQ-ON-W} state was registered at: [<ffffffff810b84a0>] mark_irqflags+0x120/0x190 [<ffffffff810b9ce7>] __lock_acquire+0x307/0x4c0 [<ffffffff810b9f51>] lock_acquire+0xb1/0x150 [<ffffffff815523b1>] _raw_spin_lock+0x41/0x50 [<ffffffffa028d563>] mlx4_ib_modify_device+0x63/0x240 [mlx4_ib] [<ffffffffa026d1fc>] ib_modify_device+0x1c/0x20 [ib_core] [<ffffffffa026c353>] set_node_desc+0x83/0xc0 [ib_core] [<ffffffff8136a150>] dev_attr_store+0x20/0x30 [<ffffffff81201fd6>] sysfs_write_file+0xe6/0x170 [<ffffffff8118da38>] vfs_write+0xc8/0x190 [<ffffffff8118dc01>] sys_write+0x51/0x90 [<ffffffff8155b869>] system_call_fastpath+0x16/0x1b ... *** DEADLOCK *** 1 lock held by swapper/0/0: stack backtrace: Pid: 0, comm: swapper/0 Not tainted 3.5.0+ #20 Call Trace: <IRQ> [<ffffffff810b7bea>] print_usage_bug+0x18a/0x190 [<ffffffff810b7370>] ? print_irq_inversion_bug+0x210/0x210 [<ffffffff810b7fb2>] mark_lock_irq+0xf2/0x280 [<ffffffff810b8290>] mark_lock+0x150/0x240 [<ffffffff810b84ef>] mark_irqflags+0x16f/0x190 [<ffffffff810b9ce7>] __lock_acquire+0x307/0x4c0 [<ffffffffa028af1d>] ? update_sm_ah+0xad/0x100 [mlx4_ib] [<ffffffff810b9f51>] lock_acquire+0xb1/0x150 [<ffffffffa028af1d>] ? update_sm_ah+0xad/0x100 [mlx4_ib] [<ffffffff815523b1>] _raw_spin_lock+0x41/0x50 [<ffffffffa028af1d>] ? update_sm_ah+0xad/0x100 [mlx4_ib] [<ffffffffa026b2fa>] ? ib_create_ah+0x1a/0x40 [ib_core] [<ffffffffa028af1d>] update_sm_ah+0xad/0x100 [mlx4_ib] [<ffffffff810c27c3>] ? is_module_address+0x23/0x30 [<ffffffffa028b05b>] handle_port_mgmt_change_event+0xeb/0x150 [mlx4_ib] [<ffffffffa028c177>] mlx4_ib_event+0x117/0x160 [mlx4_ib] [<ffffffff81552501>] ? _raw_spin_lock_irqsave+0x61/0x70 [<ffffffffa022718c>] mlx4_dispatch_event+0x6c/0x90 [mlx4_core] [<ffffffffa0221b40>] mlx4_eq_int+0x500/0x950 [mlx4_core] Reported by: Or Gerlitz <[email protected]> Tested-by: Bart Van Assche <[email protected]> Signed-off-by: Jack Morgenstein <[email protected]> Signed-off-by: Roland Dreier <[email protected]>
1 parent 0d7614f commit df7fba6

File tree

2 files changed

+13
-8
lines changed

2 files changed

+13
-8
lines changed

drivers/infiniband/hw/mlx4/mad.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ static void update_sm_ah(struct mlx4_ib_dev *dev, u8 port_num, u16 lid, u8 sl)
125125
{
126126
struct ib_ah *new_ah;
127127
struct ib_ah_attr ah_attr;
128+
unsigned long flags;
128129

129130
if (!dev->send_agent[port_num - 1][0])
130131
return;
@@ -139,11 +140,11 @@ static void update_sm_ah(struct mlx4_ib_dev *dev, u8 port_num, u16 lid, u8 sl)
139140
if (IS_ERR(new_ah))
140141
return;
141142

142-
spin_lock(&dev->sm_lock);
143+
spin_lock_irqsave(&dev->sm_lock, flags);
143144
if (dev->sm_ah[port_num - 1])
144145
ib_destroy_ah(dev->sm_ah[port_num - 1]);
145146
dev->sm_ah[port_num - 1] = new_ah;
146-
spin_unlock(&dev->sm_lock);
147+
spin_unlock_irqrestore(&dev->sm_lock, flags);
147148
}
148149

149150
/*
@@ -197,13 +198,15 @@ static void smp_snoop(struct ib_device *ibdev, u8 port_num, struct ib_mad *mad,
197198
static void node_desc_override(struct ib_device *dev,
198199
struct ib_mad *mad)
199200
{
201+
unsigned long flags;
202+
200203
if ((mad->mad_hdr.mgmt_class == IB_MGMT_CLASS_SUBN_LID_ROUTED ||
201204
mad->mad_hdr.mgmt_class == IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE) &&
202205
mad->mad_hdr.method == IB_MGMT_METHOD_GET_RESP &&
203206
mad->mad_hdr.attr_id == IB_SMP_ATTR_NODE_DESC) {
204-
spin_lock(&to_mdev(dev)->sm_lock);
207+
spin_lock_irqsave(&to_mdev(dev)->sm_lock, flags);
205208
memcpy(((struct ib_smp *) mad)->data, dev->node_desc, 64);
206-
spin_unlock(&to_mdev(dev)->sm_lock);
209+
spin_unlock_irqrestore(&to_mdev(dev)->sm_lock, flags);
207210
}
208211
}
209212

@@ -213,6 +216,7 @@ static void forward_trap(struct mlx4_ib_dev *dev, u8 port_num, struct ib_mad *ma
213216
struct ib_mad_send_buf *send_buf;
214217
struct ib_mad_agent *agent = dev->send_agent[port_num - 1][qpn];
215218
int ret;
219+
unsigned long flags;
216220

217221
if (agent) {
218222
send_buf = ib_create_send_mad(agent, qpn, 0, 0, IB_MGMT_MAD_HDR,
@@ -225,13 +229,13 @@ static void forward_trap(struct mlx4_ib_dev *dev, u8 port_num, struct ib_mad *ma
225229
* wrong following the IB spec strictly, but we know
226230
* it's OK for our devices).
227231
*/
228-
spin_lock(&dev->sm_lock);
232+
spin_lock_irqsave(&dev->sm_lock, flags);
229233
memcpy(send_buf->mad, mad, sizeof *mad);
230234
if ((send_buf->ah = dev->sm_ah[port_num - 1]))
231235
ret = ib_post_send_mad(send_buf, NULL);
232236
else
233237
ret = -EINVAL;
234-
spin_unlock(&dev->sm_lock);
238+
spin_unlock_irqrestore(&dev->sm_lock, flags);
235239

236240
if (ret)
237241
ib_free_send_mad(send_buf);

drivers/infiniband/hw/mlx4/main.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,16 +423,17 @@ static int mlx4_ib_modify_device(struct ib_device *ibdev, int mask,
423423
struct ib_device_modify *props)
424424
{
425425
struct mlx4_cmd_mailbox *mailbox;
426+
unsigned long flags;
426427

427428
if (mask & ~IB_DEVICE_MODIFY_NODE_DESC)
428429
return -EOPNOTSUPP;
429430

430431
if (!(mask & IB_DEVICE_MODIFY_NODE_DESC))
431432
return 0;
432433

433-
spin_lock(&to_mdev(ibdev)->sm_lock);
434+
spin_lock_irqsave(&to_mdev(ibdev)->sm_lock, flags);
434435
memcpy(ibdev->node_desc, props->node_desc, 64);
435-
spin_unlock(&to_mdev(ibdev)->sm_lock);
436+
spin_unlock_irqrestore(&to_mdev(ibdev)->sm_lock, flags);
436437

437438
/*
438439
* If possible, pass node desc to FW, so it can generate

0 commit comments

Comments
 (0)