Page MenuHomeFreeBSD

change ffs_blkfree() trim to not bypass geom_vfs, avoids panics
ClosedPublic

Authored by chs on May 16 2018, 5:07 PM.
Tags
None
Referenced Files
F81615810: D15456.diff
Fri, Apr 19, 12:36 AM
Unknown Object (File)
Thu, Mar 28, 6:39 PM
Unknown Object (File)
Mar 13 2024, 6:27 AM
Unknown Object (File)
Feb 27 2024, 4:30 PM
Unknown Object (File)
Dec 22 2023, 11:21 PM
Unknown Object (File)
May 8 2023, 2:25 PM
Unknown Object (File)
Apr 7 2023, 4:24 PM
Unknown Object (File)
Mar 21 2023, 11:56 AM
Subscribers
None

Details

Summary

my test that makes disks disappear out from under mounted file systems triggers this panic:

g_vfs_done():da32p8[WRITE(offset=283753381888, length=8388608)]error = 6
(da32:mps3:0:7:0): Periph destroyed
panic: consumer not attached in g_io_request
cpuid = 3
time = 152064376
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe06724bd180
vpanic() at vpanic+0x18d/frame 0xfffffe06724bd1e0
vpanic() at vpanic/frame 0xfffffe06724bd260
g_io_request() at g_io_request+0x3f7/frame 0xfffffe06724bd2b0
ffs_blkfree() at ffs_blkfree+0x17b/frame 0xfffffe06724bd310
ffs_balloc_ufs2() at ffs_balloc_ufs2+0x19a9/frame 0xfffffe06724bd4b0
ffs_write() at ffs_write+0x20f/frame 0xfffffe06724bd550
VOP_WRITE_APV() at VOP_WRITE_APV+0x15a/frame 0xfffffe06724bd660
vn_write() at vn_write+0x245/frame 0xfffffe06724bd6e0
vn_io_fault1() at vn_io_fault1+0x179/frame 0xfffffe06724bd820
vn_io_fault() at vn_io_fault+0x195/frame 0xfffffe06724bd890
dofilewrite() at dofilewrite+0xa7/frame 0xfffffe06724bd8e0
kern_writev() at kern_writev+0x68/frame 0xfffffe06724bd930
sys_write() at sys_write+0x86/frame 0xfffffe06724bd980

the problem here is that the refcounting that protects against this is in the geom_vfs layer, but ffs_blkfree() bypasses geom_vfs and calls g_io_request() directly. this patch changes ffs_blkfree() to use g_vfs_strategy() instead, thus protecting these trims from disappearing disks.

Kirk suggested using his upcoming M_TRIM malloc type instead of M_TEMP, but that's not in the tree yet so I left that bit for later.

Test Plan

without this patch, my test that makes the disk disappear crashes within a few minutes. with this patch I no longer see this problem.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

We can change to M_TRIM when Kirk commits his stuff :)

This revision is now accepted and ready to land.May 16 2018, 7:32 PM

This looks good to me. I'll change the M_TEMP to M_TRIM when I check my changes into the tree.

After I did my commit, I noticed something odd. g_vfs_strategy is generally unused outside of geom, but BO_STRATEGY is used instead.

FreeBSD/sys/ufs/ffs/ffs_alloc.c
2359

I think this should be BO_STRATEGY(ump->um_bo, bp); instead. I know that there's one other call to g_vfs_strategy in ufs, but all the other filesystems use BO_STRATEGY instead. Of course, they use that in the context of their vop_strategy_t routine, not for a direct I/O call unrelated to an existing I/O, so I'm unsure.