> From: Niklas Schnelle <schne...@linux.ibm.com>
> Sent: Wednesday, July 8, 2020 5:14 PM
> Hi Parav, Hi Shay,
>
> On 7/8/20 12:43 PM, Parav Pandit wrote:
> > Hi Niklas,
> >
> ... snip ...
> >>>
> >
> > Sorry for my late response.
> > Yes, this looks good and I also found same in my analysis.
> > With latest code mlx5_pci_close() already does drain_health_wq(), so the
> additional call in remove_one() is redundant.
> > It should be just removed.
> > If you can verify below hunk in your setup, it will be really helpful.
> > You still need patch 42ea9f1b5c6 in your tree.
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > index 8b658908f044..ebec2318dbc4 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > @@ -1382,7 +1382,6 @@ static void remove_one(struct pci_dev *pdev)
> >
> > devlink_reload_disable(devlink);
> > mlx5_crdump_disable(dev);
> > - mlx5_drain_health_wq(dev);
> > mlx5_unload_one(dev, true);
> > mlx5_pci_close(dev);
> > mlx5_mdev_uninit(dev);
> >
>
> thanks for looking into this. Sadly it looks like this doesn't quite work, I'm
> getting the bewlow illegal dereference after removing the
> mlx5_drain_health_wq(dev) from remove_one() on top of v5.8-rc4.
> I think this is similar to what happned when I tried recreating the reordering
> on -rc3 which is why I contacted Shay as I wasn't able to get this to work
> again
> with the current code.
> (Sorry if there are formatting issues with the below, plugged this out of a
> x3270).
>
It is likely because events_cleanup() freed the memory using kvfree() that
health recovery context is trying to access in notifier chain.
While reviewing I see few more errors as below.
(a) mlx5_pci_err_detected() invokes mlx5_drain_health_wq() outside of
intf_state_mutex.
(b) mlx5_enter_error_state() in commit b6e0b6bebe0 read and updates dev state
outside of intf_state_mutex.
(c) due to drain_health_wq() introduction in mlx5_pci_close() in commit
42ea9f1b5c625 health_wq is flushed twice.
(d) priv->events freed is kvfree() but allocated using kzalloc().
This code certainly needs rework.
In meantime to avoid this regression, I believe below hunk eliminates error
introduced in patch 41798df9bfc.
Will you please help test it?
Shay and I did the review of below patch.
If it works I will get it through Saeed's tree and internal reviews.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c
b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index ebec2318dbc4..529df5703737 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -785,11 +785,6 @@ static int mlx5_pci_init(struct mlx5_core_dev *dev, struct
pci_dev *pdev,
static void mlx5_pci_close(struct mlx5_core_dev *dev)
{
- /* health work might still be active, and it needs pci bar in
- * order to know the NIC state. Therefore, drain the health WQ
- * before removing the pci bars
- */
- mlx5_drain_health_wq(dev);
iounmap(dev->iseg);
pci_clear_master(dev->pdev);
release_bar(dev->pdev);
@@ -1235,6 +1230,7 @@ void mlx5_unload_one(struct mlx5_core_dev *dev, bool
cleanup)
if (cleanup) {
mlx5_unregister_device(dev);
mlx5_devlink_unregister(priv_to_devlink(dev));
+ mlx5_drain_health_wq(dev);
} else {
mlx5_detach_device(dev);
}
@@ -1366,6 +1362,11 @@ static int init_one(struct pci_dev *pdev, const struct
pci_device_id *id)
return 0;
err_load_one:
+ /* health work might still be active, and it needs pci bar in
+ * order to know the NIC state. Therefore, drain the health WQ
+ * before removing the pci bars
+ */
+ mlx5_drain_health_wq(dev);
mlx5_pci_close(dev);
pci_init_err:
mlx5_mdev_uninit(dev);
> 64.735421¨ Unable to handle kernel pointer dereference in virtual kernel
> address space 64.735470¨ Failing address: 6b6b6b6b6b6b6000 TEID:
> 6b6b6b6b6b6b6803 64.735476¨ Fault in home space mode while using kernel
> ASCE.
> 64.735484¨ AS:0000000088da0007 R3:0000000000000024 64.735533¨ Oops:
> 0038 ilc:3 Ý#1¨ PREEMPT SMP 64.735538¨ Modules linked in: rpcrdma sunrpc
> rdma_ucm rdma_cm iw_cm ib_cm configfs mlx5_ib ib_uverbs ib_core
> mlx5_core dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> s390_trng ghash_s390 prng ctr aes_s390 des_s390 libdes sha3_512_s390
> sha3_256_s390 sha512_s390 sha1_s390 vfio_ccw vfi 64.735421¨ Unable to
> handle kernel pointer dereference in virtual kernel address space 64.735558¨
> CPU: 0 PID: 762 Comm: kworker/u128:3 Not tainted 5.8.0-rc4-dirty #2
> 64.735561¨ Hardware name: IBM 3906 M04 704 (z/VM 7.1.0) 64.735618¨
> Workqueue: mlx5_health0000:00:00.0 mlx5_fw_fatal_reporter_err_work
> mlx5_core 64.735623¨ Krnl PSW : 0704e00180000000 00000000876936c6
> (notifier_call_chain+0x3e/0xe8)01: HCPGSP2629I The virtual machine is
> placed in CP mode due to a SIGP stop from CPU 01.01: HCPGSP2629I The
> virtual machine is placed in CP mode due to a SIGP stop from CPU 00.
> 64.735702¨ R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0
> EA:3
> 64.735704¨ Krnl GPRS: 00000000a38fc246 0000000080000001
> 00000000e814db00 0000000000000080
> 64.735706¨ 0000000000000001 ffffffffffffffff 0000000000000000
> 0000000000000001
> 64.735708¨ 0000000000000080 0000000000000001 ffffffffffffffff
> 6b6b6b6b6b6b6b6b
> 64.735710¨ 00000000e5ff0100 0000000000000000 000003e000973bf8
> 000003e000973ba0
> 64.735716¨ Krnl Code: 00000000876936ba: b9040083 lgr %r8,%r3
> 64.735716¨ 00000000876936be: b9040074 lgr %r7,%r4
> 64.735716¨ #00000000876936c2: a7d80000 lhi %r13,0
> 64.735716¨ >00000000876936c6: e390b0080004 lg
> %r9,8(%r11)
> 64.735716¨ 00000000876936cc: e320b0000004 lg
> %r2,0(%r11)
> 64.735716¨ 00000000876936d2: c0e5ffffd773 brasl
> %r14,000000008768e5b8
> 64.735716¨ 00000000876936d8: ec280032007e cij
> %r2,0,8,000000008769373c
> 64.735716¨ 00000000876936de: e310b0000004 lg
> %r1,0(%r11)
> 64.735731¨ Call Trace:
> 64.735740¨ <00000000876936c6>¨ notifier_call_chain+0x3e/0xe8 64.735764¨
> <000000008769392c>¨ __atomic_notifier_call_chain+0x9c/0x138
> 64.735766¨ <00000000876939f2>¨ atomic_notifier_call_chain+0x2a/0x38
> 64.735782¨ <000003ff801e99c4>¨ mlx5_enter_error_state+0xec/0x100
> mlx5_core 64.735797¨ <000003ff801e9a0c>¨
> mlx5_fw_fatal_reporter_err_work+0x34/0xb8 mlx5_core¨ 64.735802¨
> <000000008768784c>¨ process_one_work+0x27c/0x478 64.735805¨
> <0000000087687aae>¨ worker_thread+0x66/0x368 64.735807¨
> <0000000087690a96>¨ kthread+0x176/0x1a0 64.735811¨
> <0000000088202820>¨ ret_from_fork+0x24/0x2c 64.735812¨ INFO: lockdep is
> turned off.
> 64.735814¨ Last Breaking-Event-Address:
> 64.735816¨ <0000000087693926>¨ __atomic_notifier_call_chain+0x96/0x138
> 64.735820¨ Kernel panic - not syncing: Fatal exception: panic_on_oops
> 00: HCPGIR450W CP entered; disabled wait PSW 00020001 80000000 00000000
> 8762103E
