Page MenuHomeFreeBSD

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

Authored by chs on May 16 2018, 5:07 PM.



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

rS FreeBSD src repository - subversion
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.


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.