I removed an SAS SSD and the system crashed with message panic: free: called with spinlock or critical section held The erroneous free is in pqisrc_device_mem_free. My kernel is based on 4c22848d1a7e3fc996adc0cb71e35d7be8b26ffb except I have INVARIANTS enabled. The drive identifies as SEAGATE XS960SE70004 (960 GB SAS SSD). It holds a ZFS pool which I exported before removing the drive. The system is an HPE Proliant DL 325 Gen 10 with the controller below: ses0: <HPE Smart Adapter 1.99> Fixed Enclosure Services SPC-3 SCSI device ses0: 1200.000MB/s transfers ses0: SES Device ses0: da0,pass0 in 'ArrayElement0000', SAS Slot: 1 phys at slot 1 ... ses0: da7,pass7 in 'ArrayElement0007', SAS Slot: 1 phys at slot 8 ses0: phy 0: SAS device type 1 phy 7 Target ( SSP ) ses0: phy 0: parent 51402ec013d6a5b4 addr 5000c5003e85f2bd I removed and reinserted da7. The panic appears to have been triggered by removal. Crash dump information follows. Unread portion of the kernel message buffer: [INFO]:[ pqisrc_display_device_info ] [ 324 ]removed scsi BTL 0:71:0: SEAGATE XS960SE70004 Physical SSDSmartPathCap- En- Exp+ qd=65535 [INFO]:[ pqisrc_remove_device ] [ 1302 ]vendor: SEAGATE XS960SE70004 model: XS960SE70004 bus:0 target:71 lun:0 is_physical_device:0x1 expose_device:0x1 volume_offline 0x0 volume_status 0x0 [INFO]:[ pqisrc_wait_for_device_commands_to_complete ] [ 515 ]Device Outstanding IO count = 0 panic: free: called with spinlock or critical section held cpuid = 11 time = 1692710548 KDB: stack backtrace: #0 0xffffffff80c19e05 at kdb_backtrace+0x65 #1 0xffffffff80bcf112 at vpanic+0x152 #2 0xffffffff80bcef13 at panic+0x43 #3 0xffffffff80ba4b5f at free+0xcf #4 0xffffffff811247ee at pqisrc_free_device+0x16e #5 0xffffffff811210ce at os_remove_device+0x7e #6 0xffffffff81125a3f at pqisrc_scan_devices+0xe7f #7 0xffffffff8112736d at pqisrc_ack_all_events+0x16d #8 0xffffffff80c2e87b at taskqueue_run_locked+0xab #9 0xffffffff80c2e78d at taskqueue_run+0x4d #10 0xffffffff80b8c9e6 at ithread_loop+0x256 #11 0xffffffff80b89910 at fork_exit+0x80 #12 0xffffffff8105f5ee at fork_trampoline+0xe Uptime: 20d2h33m19s Dumping 11245 out of 98100 MB:..1%..11%..21%..31%..41%..51%..61%..71%..81%..91% __curthread () at /usr/home/jfc/freebsd/src/sys/amd64/include/pcpu_aux.h:55 55 __asm("movq %%gs:%P1,%0" : "=r" (td) : "n" (offsetof(struct pcpu, (kgdb) #0 __curthread () at /usr/home/jfc/freebsd/src/sys/amd64/include/pcpu_aux.h:55 #1 doadump (textdump=<optimized out>) at /usr/home/jfc/freebsd/src/sys/kern/kern_shutdown.c:396 #2 0xffffffff80bced22 in kern_reboot (howto=260) at /usr/home/jfc/freebsd/src/sys/kern/kern_shutdown.c:484 #3 0xffffffff80bcf17f in vpanic ( fmt=0xffffffff811f6ecd "free: called with spinlock or critical section held", ap=ap@entry=0xfffffe01b7549a00) at /usr/home/jfc/freebsd/src/sys/kern/kern_shutdown.c:923 #4 0xffffffff80bcef13 in panic (fmt=<unavailable>) at /usr/home/jfc/freebsd/src/sys/kern/kern_shutdown.c:847 #5 0xffffffff80ba4b5f in free_dbg (mtp=0xffffffff81b49030 <M_SMARTPQI>, addrp=<optimized out>) at /usr/home/jfc/freebsd/src/sys/kern/kern_malloc.c:866 #6 free (addr=addr@entry=0xfffff80104da9700, mtp=0xffffffff81b49030 <M_SMARTPQI>) at /usr/home/jfc/freebsd/src/sys/kern/kern_malloc.c:904 #7 0xffffffff8112cef4 in os_mem_free (softs=softs@entry=0xfffffe01b852a000, addr=<unavailable>, addr@entry=0xfffff80104da9700 "", size=<unavailable>, size@entry=184) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_mem.c:192 #8 0xffffffff811247ee in pqisrc_device_mem_free (softs=0xfffffe01b852a000, device=0xfffff80104da9700) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_discovery.c:1432 #9 pqisrc_free_device (softs=softs@entry=0xfffffe01b852a000, device=device@entry=0xfffff80104da9700) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_discovery.c:1464 #10 0xffffffff811210ce in os_remove_device (softs=0xfffffe01b852a000, device=0xfffff80104da9700) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_cam.c:152 #11 0xffffffff81124673 in pqisrc_remove_device (softs=0x99d1f44cbec872bb, softs@entry=0xfffffe01b852a000, device=<unavailable>, device@entry=0xfffff80104da9700) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_discovery.c:1317 #12 0xffffffff81125a3f in pqisrc_update_device_list ( softs=0xfffffe01b852a000, new_device_list=0xfffff802f4277b80, num_new_devices=9) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_discovery.c:1597 #13 pqisrc_scan_devices (softs=softs@entry=0xfffffe01b852a000) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_discovery.c:1992 #14 0xffffffff8112736d in pqisrc_rescan_devices (softs=0xfffffe01b852a000) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_event.c:42 #15 pqisrc_ack_all_events (arg1=0xfffffe01b852a000) at /usr/home/jfc/freebsd/src/sys/dev/smartpqi/smartpqi_event.c:123 #16 0xffffffff80c2e87b in taskqueue_run_locked ( queue=queue@entry=0xfffff8010191be00) at /usr/home/jfc/freebsd/src/sys/kern/subr_taskqueue.c:514 #17 0xffffffff80c2e78d in taskqueue_run (queue=0xfffff8010191be00) at /usr/home/jfc/freebsd/src/sys/kern/subr_taskqueue.c:529 #18 0xffffffff80b8c9e6 in intr_event_execute_handlers (ie=0xfffff8010191bd00, p=<optimized out>) at /usr/home/jfc/freebsd/src/sys/kern/kern_intr.c:1169 #19 ithread_execute_handlers (ie=0xfffff8010191bd00, p=<optimized out>) at /usr/home/jfc/freebsd/src/sys/kern/kern_intr.c:1182 #20 ithread_loop (arg=arg@entry=0xfffff80101964c60) at /usr/home/jfc/freebsd/src/sys/kern/kern_intr.c:1270 #21 0xffffffff80b89910 in fork_exit ( callout=0xffffffff80b8c790 <ithread_loop>, arg=0xfffff80101964c60, frame=0xfffffe01b7549f40) at /usr/home/jfc/freebsd/src/sys/kern/kern_fork.c:1094 #22 <signal handler called>
Created attachment 244283 [details] move free out of spinlock The fix is simple, move a free call outside of the region protected by a spinlock. Static analysis ought to be able to detect this bug.
(In reply to John F. Carr from comment #1) Yes, there doesn't seem to be any particular reason to hold the lock across the free() calls. Though, I cannot see a reason to use a MTX_SPIN lock here at all. It looks like softs->devlist_lock should be a MTX_DEF mutex. Spin mutexes are only needed when synchronizing with interrupt handlers, and it doesn't look like this mutex has to deal with that. Though, fixing that would be more involved and would require some modification to this OS abstraction layer in the driver.
Sorry Benedict, I didn't mean to cc you. Given that 14.0 is soon approaching, I suspect it'd be better to simply commit the patch, but maybe Warner is in contact with the maintainer at Microsemi?
I have a new version of the driver that supposedly fixes this. https://reviews.freebsd.org/D41550 is the review. Maybe you can test it to see if that also fixes the problem?
(In reply to Warner Losh from comment #4) That revision appears to have restricted visibility - I can't open it.
Based on code inspection, the driver update in 7ea28254ec5376b5deb86c136e1838d0134dbb22 does not fix the bug.
Thanks for looking at this John. Does your fix for the old driver fix this?
Created attachment 244386 [details] move free out of spinlock (new driver) Because pqisrc_free_device has changed in the new driver a new patch is needed. I tested this by copying the new driver to 13.2-CURRENT, the version I am running on my HPE server. The new driver is very chatty, see below. Instead of a compile-time constant the variable logging_level in smartpqi_defines.h should be a tunable and the default value should not include PQISRC_FLAGS_NOTE. The driver appears to be trying to delete the drive twice. See the "Invalid device" note. The following is the dmesg output triggered by removing and replacing a drive, which would have caused a panic with the unpatched old driver. [NOTE]:[ pqisrc_display_device_info ] [ 274 ]removed scsi BTL 0:8:0: SEAGATE XS960SE70004 Physical SSDSmartPathCap- En- Exp+ qd=65535 [NOTE]:[ pqisrc_remove_device ] [ 1443 ]vendor: SEAGATE XS960SE70004 model: XS960SE70004 bus:0 target:8 lun:0 is_physical_device:0x1 expose_device:0x1 volume_offline 0x0 volume_status 0x0 [WARN]:[67:655.0][0,8,0][CPU 0][pqisrc_wait_for_device_commands_to_complete][430]:Device Outstanding IO count = 0 [NOTE]:[ pqisrc_free_device ] [ 1643 ]Giving back target 8 [NOTE]:[ pqisrc_delete_softs_entry ] [ 363 ]Invalid device, either it was already removed or never added [NOTE]:[ pqisrc_free_device ] [ 1673 ]Removed memory for device : B 0: T 8: L 0 da7 at smartpqi0 bus 0 scbus0 target 8 lun 0 da7: <SEAGATE XS960SE70004 0003> s/n HLJ03TL80000822150Z3 detached (da7:smartpqi0:0:8:0): Periph destroyed [NOTE]:[ pqisrc_add_softs_entry ] [ 288 ]Added device [7 of 10]: B 0: T 8: L 0 [NOTE]:[ pqisrc_add_device ] [ 1420 ]vendor: SEAGATE XS960SE70004 model: XS960SE70004 bus:0 target:8 lun:0 is_physical_device:0x1 expose_device:0x1 volume_offline 0x0 volume_status 0x0 [NOTE]:[ pqisrc_display_device_info ] [ 274 ]added scsi BTL 0:8:0: SEAGATE XS960SE70004 Physical SSDSmartPathCap- En- Exp+ qd=65535 ses0: da7,pass8 in 'ArrayElement0007', SAS Slot: 1 phys at slot 8 ses0: phy 0: SAS device type 1 phy 7 Target ( SSP ) ses0: phy 0: parent 51402ec013d6a5b4 addr 5000c5003e85f2bd da7 at smartpqi0 bus 0 scbus0 target 8 lun 0 da7: <SEAGATE XS960SE70004 0003> Fixed Direct Access SPC-5 SCSI device
I may have a simple power switch I can connect to my direct attached storage that would let me semi easily add remove the drive. Would that be enough to trigger this? I have a spartpqi card here but due to the vagaries of the lab env it has now drives right now... I could juggle though if you think that would trigger it on my system...
I expect powering off an external drive to work the same as removing a hot-swap drive. You may need INVARIANTS on to see the panic.
(In reply to John F. Carr from comment #10) Hi John, What is the frequency of this issue ? is it happening everytime? Do we need to follow any steps before hot-removing the drive inorder to observe the crash ? Thanks & Regards Hermes T K
This crash has happened all three times I removed an SAS SSD without a patched kernel. I have not tried with SATA drives. Based on code inspection, there is no race condition that would make the panic unpredictable. Without INVARIANTS you might see unpredictable behavior instead of a panic. I exported the ZFS pool before removing the drive so there should have been no references to the drive. It was sitting idle.
This change looks good by inspection. Committing to at least the current/14.x drivers.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=b064a4c9eed5b1dd2a40fc4fd2cb7e738b681547 commit b064a4c9eed5b1dd2a40fc4fd2cb7e738b681547 Author: John F. Carr <jfc@mit.edu> AuthorDate: 2023-10-19 03:02:42 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2023-10-19 03:06:40 +0000 smartpqi: Drop spinlock before freeing memory pqisrc_free_device frees the device softc with the os spinlock held. This causes crashes when devices are removed because the memory free might sleep (which is prohibited with spin locks held). Drop the spinlock before releasing the memory. MFC After: 2 days PR: 273289 Reviewed by: imp sys/dev/smartpqi/smartpqi_discovery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
I copied the latest driver from main (f07b267d8cc8) to my 13.2 system and verified there is no panic on removing and reinserting an SAS drive with INVARIANTS configured. [NOTE]:[ pqisrc_display_device_info ] [ 274 ]removed scsi BTL 0:8:0: SEAGATE XS960SE70004 Physical SSDSmartPathCap- En- Exp+ qd=65535 [NOTE]:[ pqisrc_remove_device ] [ 1443 ]vendor: SEAGATE XS960SE70004 model: XS960SE70004 bus:0 target:8 lun:0 is_physical_device:0x1 expose_device:0x1 volume_offline 0x0 volume_status 0x0 [WARN]:[67:655.0][0,8,0][CPU 0][pqisrc_wait_for_device_commands_to_complete][430]:Device Outstanding IO count = 0 [NOTE]:[ pqisrc_free_device ] [ 1643 ]Giving back target 8 [NOTE]:[ pqisrc_delete_softs_entry ] [ 363 ]Invalid device, either it was already removed or never added [NOTE]:[ pqisrc_free_device ] [ 1673 ]Removed memory for device : B 0: T 8: L 0 da7 at smartpqi0 bus 0 scbus0 target 8 lun 0 da7: <SEAGATE XS960SE70004 0003> s/n HLJ03TL80000822150Z3 detached (da7:smartpqi0:0:8:0): Periph destroyed [NOTE]:[ pqisrc_add_softs_entry ] [ 288 ]Added device [7 of 10]: B 0: T 8: L 0 [NOTE]:[ pqisrc_add_device ] [ 1420 ]vendor: SEAGATE XS960SE70004 model: XS960SE70004 bus:0 target:8 lun:0 is_physical_device:0x1 expose_device:0x1 volume_offline 0x0 volume_status 0x0 [NOTE]:[ pqisrc_display_device_info ] [ 274 ]added scsi BTL 0:8:0: SEAGATE XS960SE70004 Physical SSDSmartPathCap- En- Exp+ qd=65535 ses0: da7,pass8 in 'ArrayElement0007', SAS Slot: 1 phys at slot 8 ses0: phy 0: SAS device type 1 phy 7 Target ( SSP ) ses0: phy 0: parent 51402ec013d6a5b4 addr 5000c5003e85f2bd da7 at smartpqi0 bus 0 scbus0 target 8 lun 0 da7: <SEAGATE XS960SE70004 0003> Fixed Direct Access SPC-5 SCSI device da7: Serial Number HLJ03TL80000822150Z3 da7: 1200.000MB/s transfers da7: Command Queueing enabled da7: 915715MB (1875385008 512 byte sectors)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=1ad148a68ae74f3372b12b6e66fadf5ade384144 commit 1ad148a68ae74f3372b12b6e66fadf5ade384144 Author: John F. Carr <jfc@mit.edu> AuthorDate: 2023-10-19 03:25:31 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2023-10-19 21:21:11 +0000 smartpqi: Drop spinlock before freeing memory pqisrc_free_device frees the device softc with the os spinlock held. This causes crashes when devices are removed because the memory free might sleep (which is prohibited with spin locks held). Drop the spinlock before releasing the memory. MFC After: 2 days PR: 273289 Reviewed by: imp (cherry picked from commit b064a4c9eed5b1dd2a40fc4fd2cb7e738b681547) sys/dev/smartpqi/smartpqi_discovery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
A commit in branch releng/14.0 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8fe059425a0919274e2c5b25a6af0568d147518d commit 8fe059425a0919274e2c5b25a6af0568d147518d Author: John F. Carr <jfc@mit.edu> AuthorDate: 2023-10-19 03:25:31 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2023-10-19 21:37:29 +0000 smartpqi: Drop spinlock before freeing memory pqisrc_free_device frees the device softc with the os spinlock held. This causes crashes when devices are removed because the memory free might sleep (which is prohibited with spin locks held). Drop the spinlock before releasing the memory. MFC After: 2 days PR: 273289 Reviewed by: imp (cherry picked from commit b064a4c9eed5b1dd2a40fc4fd2cb7e738b681547) (cherry picked from commit 1ad148a68ae74f3372b12b6e66fadf5ade384144) Approved-by: re (gjb) sys/dev/smartpqi/smartpqi_discovery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
^Triage: assign to committer who resolved. Set mfc-stable13 flag as a possibililty.
merged the alt fix... and will merge it to 13.3 if re@ will let me
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=898e02d740741d13a8948cbad4e0d969a768fd30 commit 898e02d740741d13a8948cbad4e0d969a768fd30 Author: John F. Carr <jfc@mit.edu> AuthorDate: 2023-10-19 03:02:42 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2024-02-19 04:07:04 +0000 smartpqi: Drop spinlock before freeing memory pqisrc_free_device frees the device softc with the os spinlock held. This causes crashes when devices are removed because the memory free might sleep (which is prohibited with spin locks held). Drop the spinlock before releasing the memory. MFC After: 2 days PR: 273289 Reviewed by: imp This is the alternate fix from the box. (this is not a cherry pick of b064a4c9eed5b1dd2a40fc4fd2cb7e738b681547) sys/dev/smartpqi/smartpqi_discovery.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
A commit in branch releng/13.3 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=da2e35b0ec99e979963923ed6392b3e883f40eb2 commit da2e35b0ec99e979963923ed6392b3e883f40eb2 Author: John F. Carr <jfc@mit.edu> AuthorDate: 2023-10-19 03:02:42 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2024-02-19 06:59:36 +0000 smartpqi: Drop spinlock before freeing memory pqisrc_free_device frees the device softc with the os spinlock held. This causes crashes when devices are removed because the memory free might sleep (which is prohibited with spin locks held). Drop the spinlock before releasing the memory. MFC After: 2 days PR: 273289 Reviewed by: imp This is the alternate fix from the box. (this is not a cherry pick of b064a4c9eed5b1dd2a40fc4fd2cb7e738b681547) (cherry picked from commit 898e02d740741d13a8948cbad4e0d969a768fd30) Approved-by: re (cperciva) sys/dev/smartpqi/smartpqi_discovery.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)