Bug 238565 - panic: vinvalbuf: dirty bufs during unmount if clustered writes return errors
Summary: panic: vinvalbuf: dirty bufs during unmount if clustered writes return errors
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-fs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-14 18:05 UTC by Alan Somers
Modified: 2021-10-07 19:42 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Somers freebsd_committer freebsd_triage 2019-06-14 18:05:17 UTC
It seems that if a clustered write experiences an error while fsyncing during unmount, vinvalbuf will panic.  This only happens with clustered writes, not normal writes.  So far I've only been able to reproduce it on fusefs, but I suspect that all file systems which use clustering (UFS, ext2, msdosfs, fusefs) are affected.  The problem is easiest to reproduce with fusefs just because fuse makes it easy to inject errors at any point.

The panic can be reproduced on the projects/fuse2 branch by doing the following:
$ sudo sysctl sysctl vfs.fusefs.data_cache_mode=2
$ cd /usr/tests/sys/fs/fusefs
$ ./write --gtest_also_run_disabled_tests  --gtest_filter=WriteCluster.DISABLED_cluster_write_err -v


fsync: giving up on dirty (error = 5) 0xfffff80049732960: tag fuse, type VREG
    usecount 0, writecount 0, refcount 5
    flags (VI_ACTIVE|VI_DOINGINACT)
    v_object 0xfffff80049318900 ref 0 pages 48 cleanbuf 0 dirtybuf 3
    lock type fuse: EXCL by thread 0xfffff8000337f5a0 (pid 3944, write, tid 100085)
nodeid: 42, parent nodeid: 0, nlookup: 1, flag: 0
panic: vinvalbuf: dirty bufs
cpuid = 0
time = 1560534385
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0031dd84c0
vpanic() at vpanic+0x19d/frame 0xfffffe0031dd8510
panic() at panic+0x43/frame 0xfffffe0031dd8570
bufobj_invalbuf() at bufobj_invalbuf+0x2ca/frame 0xfffffe0031dd85d0
vgonel() at vgonel+0x15e/frame 0xfffffe0031dd8640
vflush() at vflush+0x22c/frame 0xfffffe0031dd8790
fuse_vfsop_unmount() at fuse_vfsop_unmount+0xad/frame 0xfffffe0031dd8800
dounmount() at dounmount+0x4ae/frame 0xfffffe0031dd8860
sys_unmount() at sys_unmount+0x300/frame 0xfffffe0031dd8990
amd64_syscall() at amd64_syscall+0x276/frame 0xfffffe0031dd8ab0
fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe0031dd8ab0
--- syscall (22, FreeBSD ELF64, sys_unmount), rip = 0x80052d9aa, rsp = 0x7fffffffe6e8, rbp = 0x7fffffffe700 ---
KDB: enter: panic
Comment 1 commit-hook freebsd_committer freebsd_triage 2019-06-14 18:15:38 UTC
A commit references this bug:

Author: asomers
Date: Fri Jun 14 18:14:53 UTC 2019
New revision: 349036
URL: https://svnweb.freebsd.org/changeset/base/349036

Log:
  fusefs: enable write clustering

  Enable write clustering in fusefs whenever cache mode is set to writeback
  and the "async" mount option is used.  With default values for MAXPHYS,
  DFLTPHYS, and the fuse max_write mount parameter, that means sequential
  writes will now be written 128KB at a time instead of 64KB.

  Also, add a regression test for PR 238565, a panic during unmount that
  probably affects UFS, ext2, and msdosfs as well as fusefs.

  PR:		238565
  Sponsored by:	The FreeBSD Foundation

Changes:
  projects/fuse2/sbin/mount_fusefs/mount_fusefs.8
  projects/fuse2/sbin/mount_fusefs/mount_fusefs.c
  projects/fuse2/sys/fs/fuse/fuse_io.c
  projects/fuse2/sys/fs/fuse/fuse_vfsops.c
  projects/fuse2/tests/sys/fs/fusefs/mockfs.cc
  projects/fuse2/tests/sys/fs/fusefs/mockfs.hh
  projects/fuse2/tests/sys/fs/fusefs/utils.cc
  projects/fuse2/tests/sys/fs/fusefs/utils.hh
  projects/fuse2/tests/sys/fs/fusefs/write.cc
Comment 2 commit-hook freebsd_committer freebsd_triage 2019-08-07 00:40:24 UTC
A commit references this bug:

Author: asomers
Date: Wed Aug  7 00:38:31 UTC 2019
New revision: 350665
URL: https://svnweb.freebsd.org/changeset/base/350665

Log:
  fusefs: merge from projects/fuse2

  This commit imports the new fusefs driver. It raises the protocol level
  from 7.8 to 7.23, fixes many bugs, adds a test suite for the driver, and
  adds many new features. New features include:

  * Optional kernel-side permissions checks (-o default_permissions)
  * Implement VOP_MKNOD, VOP_BMAP, and VOP_ADVLOCK
  * Allow interrupting FUSE operations
  * Support named pipes and unix-domain sockets in fusefs file systems
  * Forward UTIME_NOW during utimensat(2) to the daemon
  * kqueue support for /dev/fuse
  * Allow updating mounts with "mount -u"
  * Allow exporting fusefs file systems over NFS
  * Server-initiated invalidation of the name cache or data cache
  * Respect RLIMIT_FSIZE
  * Try to support servers as old as protocol 7.4

  Performance enhancements include:

  * Implement FUSE's FOPEN_KEEP_CACHE and FUSE_ASYNC_READ flags
  * Cache file attributes
  * Cache lookup entries, both positive and negative
  * Server-selectable cache modes: writethrough, writeback, or uncached
  * Write clustering
  * Readahead
  * Use counter(9) for statistical reporting

  PR:		199934 216391 233783 234581 235773 235774 235775
  PR:		236226 236231 236236 236291 236329 236381 236405
  PR:		236327 236466 236472 236473 236474 236530 236557
  PR:		236560 236844 237052 237181 237588 238565
  Reviewed by:	bcr (man pages)
  Reviewed by:	cem, ngie, rpokala, glebius, kib, bde, emaste (post-commit
  		review on project branch)
  MFC after:	3 weeks
  Relnotes:	yes
  Sponsored by:	The FreeBSD Foundation
  Pull Request:	https://reviews.freebsd.org/D21110

Changes:
_U  head/
  head/MAINTAINERS
  head/UPDATING
  head/etc/mtree/BSD.tests.dist
  head/sbin/mount_fusefs/mount_fusefs.8
  head/sbin/mount_fusefs/mount_fusefs.c
  head/share/man/man5/fusefs.5
  head/sys/fs/fuse/fuse.h
  head/sys/fs/fuse/fuse_device.c
  head/sys/fs/fuse/fuse_file.c
  head/sys/fs/fuse/fuse_file.h
  head/sys/fs/fuse/fuse_internal.c
  head/sys/fs/fuse/fuse_internal.h
  head/sys/fs/fuse/fuse_io.c
  head/sys/fs/fuse/fuse_io.h
  head/sys/fs/fuse/fuse_ipc.c
  head/sys/fs/fuse/fuse_ipc.h
  head/sys/fs/fuse/fuse_kernel.h
  head/sys/fs/fuse/fuse_main.c
  head/sys/fs/fuse/fuse_node.c
  head/sys/fs/fuse/fuse_node.h
  head/sys/fs/fuse/fuse_param.h
  head/sys/fs/fuse/fuse_vfsops.c
  head/sys/fs/fuse/fuse_vnops.c
  head/sys/sys/param.h
  head/tests/sys/fs/Makefile
  head/tests/sys/fs/fusefs/
Comment 3 commit-hook freebsd_committer freebsd_triage 2019-09-15 04:16:08 UTC
A commit references this bug:

Author: asomers
Date: Sun Sep 15 04:14:37 UTC 2019
New revision: 352351
URL: https://svnweb.freebsd.org/changeset/base/352351

Log:
  MFC the new fusefs driver

  MFC r350665, r350990, r350992, r351039, r351042, r351061, r351066, r351113, r351560, r351961, r351963, r352021, r352025, r352230

  r350665:
  fusefs: merge from projects/fuse2

  This commit imports the new fusefs driver. It raises the protocol level
  from 7.8 to 7.23, fixes many bugs, adds a test suite for the driver, and
  adds many new features. New features include:

  * Optional kernel-side permissions checks (-o default_permissions)
  * Implement VOP_MKNOD, VOP_BMAP, and VOP_ADVLOCK
  * Allow interrupting FUSE operations
  * Support named pipes and unix-domain sockets in fusefs file systems
  * Forward UTIME_NOW during utimensat(2) to the daemon
  * kqueue support for /dev/fuse
  * Allow updating mounts with "mount -u"
  * Allow exporting fusefs file systems over NFS
  * Server-initiated invalidation of the name cache or data cache
  * Respect RLIMIT_FSIZE
  * Try to support servers as old as protocol 7.4

  Performance enhancements include:

  * Implement FUSE's FOPEN_KEEP_CACHE and FUSE_ASYNC_READ flags
  * Cache file attributes
  * Cache lookup entries, both positive and negative
  * Server-selectable cache modes: writethrough, writeback, or uncached
  * Write clustering
  * Readahead
  * Use counter(9) for statistical reporting

  PR:		199934 216391 233783 234581 235773 235774 235775
  PR:		236226 236231 236236 236291 236329 236381 236405
  PR:		236327 236466 236472 236473 236474 236530 236557
  PR:		236560 236844 237052 237181 237588 238565
  Reviewed by:	bcr (man pages)
  Reviewed by:	cem, ngie, rpokala, glebius, kib, bde, emaste (post-commit
  		review on project branch)
  Relnotes:	yes
  Sponsored by:	The FreeBSD Foundation
  Pull Request:	https://reviews.freebsd.org/D21110

  r350990:
  fusefs: add SVN Keywords to the test files

  Reported by:	SVN pre-commit hooks
  MFC-With:	r350665
  Sponsored by:	The FreeBSD Foundation

  r350992:
  fusefs: skip some tests when unsafe aio is disabled

  MFC-With:       r350665
  Sponsored by:   The FreeBSD Foundation

  r351039:
  fusefs: fix intermittency in the default_permissions.Unlink.ok test

  The test needs to expect a FUSE_FORGET operation. Most of the time the test
  would pass anyway, because by chance FUSE_FORGET would arrive after the
  unmount.

  MFC-With:	350665
  Sponsored by:	The FreeBSD Foundation

  r351042:
  fusefs: Fix the size of fuse_getattr_in

  In FUSE protocol 7.9, the size of the FUSE_GETATTR request has increased.
  However, the fusefs driver is currently not sending the additional fields.
  In our implementation, the additional fields are always zero, so I there
  haven't been any test failures until now.  But fusefs-lkl requires the
  request's length to be correct.

  Fix this bug, and also enhance the test suite to catch similar bugs.

  PR:		239830
  MFC-With:	350665
  Sponsored by:	The FreeBSD Foundation

  r351061:
  fusefs: fix the 32-bit build after 351042

  Reported by:	jhb
  MFC-With:	351042
  Sponsored by:	The FreeBSD Foundation

  r351066:
  fusefs: fix conditional from r351061

  The entirety of r351061 was a copy/paste error.  I'm sorry I've been
  comitting so hastily.

  Reported by:	rpokala
  Reviewed by:	rpokala
  MFC-With:	351061
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21265

  r351113:
  fusefs: don't send the namespace during listextattr

  The FUSE_LISTXATTR operation always returns the full list of a file's
  extended attributes, in all namespaces. There's no way to filter the list
  server-side. However, currently FreeBSD's fusefs driver sends a namespace
  string with the FUSE_LISTXATTR request. That behavior was probably copied
  from fuse_vnop_getextattr, which has an attribute name argument. It's
  been there ever since extended attribute support was added in r324620. This
  commit removes it.

  Reviewed by:	cem
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21280

  r351560:
  fusefs: Fix some bugs regarding the size of the LISTXATTR list

  * A small error in r338152 let to the returned size always being exactly
    eight bytes too large.

  * The FUSE_LISTXATTR operation works like Linux's listxattr(2): if the
    caller does not provide enough space, then the server should return ERANGE
    rather than return a truncated list.  That's true even though in FUSE's
    case the kernel doesn't provide space to the client at all; it simply
    requests a maximum size for the list.  We previously weren't handling the
    case where the server returns ERANGE even though the kernel requested as
    much size as the server had told us it needs; that can happen due to a
    race.

  * We also need to ensure that a pathological server that always returns
    ERANGE no matter what size we request in FUSE_LISTXATTR won't cause an
    infinite loop in the kernel.  As of this commit, it will instead cause an
    infinite loop that exits and enters the kernel on each iteration, allowing
    signals to be processed.

  Reviewed by:	cem
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21287

  r351961:
  Coverity fixes in fusefs(5)

  CID 1404532 fixes a signed vs unsigned comparison error in fuse_vnop_bmap.
  It could potentially have resulted in VOP_BMAP reporting too many
  consecutive blocks.

  CID 1404364 is much worse. It was an array access by an untrusted,
  user-provided variable. It could potentially have resulted in a malicious
  file system crashing the kernel or worse.

  Reported by:	Coverity
  Reviewed by:	emaste
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21466

  r351963:
  fusefs: coverity cleanup in the tests

  Address the following defects reported by Coverity:

  * Structurally dead code (CID 1404366): set m_quit before FAIL, not after

  * Unchecked return value of sysctlbyname (CID 1404321)

  * Unchecked return value of stat(2) (CID 1404471)

  * Unchecked return value of open(2) (CID 1404402, 1404529)

  * Unchecked return value of dup(2) (CID 1404478)

  * Buffer overflows. These are all false positives caused by the fact that
    Coverity thinks I'm using a buffer to store strings, when in fact I'm
    really just using it to store a byte array that happens to be initialized
    with a string. I'm changing the type from char to uint8_t in the hopes
    that it will placate Coverity. (CID 1404338, 1404350, 1404367, 1404376,
    1404379, 1404381, 1404388, 1404403, 1404425, 1404433, 1404434, 1404474,
    1404480, 1404484, 1404503, 1404505)

  * False positive file descriptor leak. I'm going to try to fix this with
    Coverity modeling, but I'll also change an EXPECT to ASSERT so we don't
    perform meaningless assertions after the failure. (CID 1404320, 1404324,
    1404440, 1404445).

  * Unannotated file descriptor leak. This will be followed up by a Coverity
    modeling change. (CID 1404326, 1404334, 1404336, 1404357, 1404361,
    1404372, 1404391, 1404395, 1404409, 1404430, 1404448, 1404451, 1404455,
    1404457, 1404458, 1404460)

  * Uninitialized variables in C++ constructors (CID 1404327, 1404346). In the
    case of m_maxphys, this actually led to part of the FUSE_INIT's response
    being set to stack garbage during the WriteCluster::clustering test.

  * Uninitialized sun_len field in struct sockaddr_un (CID 1404330, 1404371,
    1404429).

  Reported by:	Coverity
  Reviewed by:	emaste
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21457

  r352021:
  fusefs: suppress some Coverity resource leak CIDs in the tests

  The fusefs tests deliberately leak file descriptors.  To do otherwise would
  add extra complications to the tests' mock FUSE server.  This annotation
  should hopefully convince Coverity to shut up about the leaks.

  Reviewed by:	uqs
  Sponsored by:	The FreeBSD Foundation

  r352025:
  mount_fusefs: fix a segfault on memory allocation failure

  Reported by:	Coverity
  Coverity CID:	1354188
  Sponsored by:	The FreeBSD Foundation

  r352230:
  fusefs: Fix iosize for FUSE_WRITE in 7.8 compat mode

  When communicating with a FUSE server that implements version 7.8 (or older)
  of the FUSE protocol, the FUSE_WRITE request structure is 16 bytes shorter
  than normal. The protocol version check wasn't applied universally, leading
  to an extra 16 bytes being sent to such servers. The extra bytes were
  allocated and bzero()d, so there was no information disclosure.

  Reviewed by:	emaste
  MFC-With:	r350665
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21557

Changes:
_U  stable/12/
  stable/12/MAINTAINERS
  stable/12/UPDATING
  stable/12/etc/mtree/BSD.tests.dist
  stable/12/sbin/mount_fusefs/mount_fusefs.8
  stable/12/sbin/mount_fusefs/mount_fusefs.c
  stable/12/share/man/man5/fusefs.5
  stable/12/sys/fs/fuse/fuse.h
  stable/12/sys/fs/fuse/fuse_device.c
  stable/12/sys/fs/fuse/fuse_file.c
  stable/12/sys/fs/fuse/fuse_file.h
  stable/12/sys/fs/fuse/fuse_internal.c
  stable/12/sys/fs/fuse/fuse_internal.h
  stable/12/sys/fs/fuse/fuse_io.c
  stable/12/sys/fs/fuse/fuse_io.h
  stable/12/sys/fs/fuse/fuse_ipc.c
  stable/12/sys/fs/fuse/fuse_ipc.h
  stable/12/sys/fs/fuse/fuse_kernel.h
  stable/12/sys/fs/fuse/fuse_main.c
  stable/12/sys/fs/fuse/fuse_node.c
  stable/12/sys/fs/fuse/fuse_node.h
  stable/12/sys/fs/fuse/fuse_param.h
  stable/12/sys/fs/fuse/fuse_vfsops.c
  stable/12/sys/fs/fuse/fuse_vnops.c
  stable/12/sys/sys/param.h
  stable/12/tests/sys/fs/Makefile
  stable/12/tests/sys/fs/fusefs/
  stable/12/tests/sys/fs/fusefs/access.cc
  stable/12/tests/sys/fs/fusefs/allow_other.cc
  stable/12/tests/sys/fs/fusefs/bmap.cc
  stable/12/tests/sys/fs/fusefs/create.cc
  stable/12/tests/sys/fs/fusefs/default_permissions.cc
  stable/12/tests/sys/fs/fusefs/default_permissions_privileged.cc
  stable/12/tests/sys/fs/fusefs/destroy.cc
  stable/12/tests/sys/fs/fusefs/dev_fuse_poll.cc
  stable/12/tests/sys/fs/fusefs/fifo.cc
  stable/12/tests/sys/fs/fusefs/flush.cc
  stable/12/tests/sys/fs/fusefs/forget.cc
  stable/12/tests/sys/fs/fusefs/fsync.cc
  stable/12/tests/sys/fs/fusefs/fsyncdir.cc
  stable/12/tests/sys/fs/fusefs/getattr.cc
  stable/12/tests/sys/fs/fusefs/interrupt.cc
  stable/12/tests/sys/fs/fusefs/io.cc
  stable/12/tests/sys/fs/fusefs/link.cc
  stable/12/tests/sys/fs/fusefs/locks.cc
  stable/12/tests/sys/fs/fusefs/lookup.cc
  stable/12/tests/sys/fs/fusefs/mkdir.cc
  stable/12/tests/sys/fs/fusefs/mknod.cc
  stable/12/tests/sys/fs/fusefs/mockfs.cc
  stable/12/tests/sys/fs/fusefs/mockfs.hh
  stable/12/tests/sys/fs/fusefs/mount.cc
  stable/12/tests/sys/fs/fusefs/nfs.cc
  stable/12/tests/sys/fs/fusefs/notify.cc
  stable/12/tests/sys/fs/fusefs/open.cc
  stable/12/tests/sys/fs/fusefs/opendir.cc
  stable/12/tests/sys/fs/fusefs/read.cc
  stable/12/tests/sys/fs/fusefs/readdir.cc
  stable/12/tests/sys/fs/fusefs/readlink.cc
  stable/12/tests/sys/fs/fusefs/release.cc
  stable/12/tests/sys/fs/fusefs/releasedir.cc
  stable/12/tests/sys/fs/fusefs/rename.cc
  stable/12/tests/sys/fs/fusefs/rmdir.cc
  stable/12/tests/sys/fs/fusefs/setattr.cc
  stable/12/tests/sys/fs/fusefs/statfs.cc
  stable/12/tests/sys/fs/fusefs/symlink.cc
  stable/12/tests/sys/fs/fusefs/unlink.cc
  stable/12/tests/sys/fs/fusefs/utils.cc
  stable/12/tests/sys/fs/fusefs/utils.hh
  stable/12/tests/sys/fs/fusefs/write.cc
  stable/12/tests/sys/fs/fusefs/xattr.cc
Comment 4 Kirk McKusick freebsd_committer freebsd_triage 2021-05-29 00:12:03 UTC
Alan Somers <asomers@freebsd.org> reports that this bug still exists and can be demonstrated using this test:

cd /usr/tests/sys/fs/fusefs
sudo mkdir mountpoint
sudo chflags uchg mountpoint
sudo ./write --gtest_also_run_disabled_tests --gtest_filter=WriteCluster.DISABLED_cluster_write_err
Comment 5 Konstantin Belousov freebsd_committer freebsd_triage 2021-05-29 10:37:04 UTC
I do not understand the complain.

We do panic if there are dirty buffers discovered on a vnode at vflush() time.
It does not matter how the dirty buffer (AKA delayed-write buffer) was created,
by bdwrite() or by cluster_write().

Right now we panic on assumption that the presence of the dirty buffers that
we unable to write, either compromises filesystem metadata integrity, or
causes user data corruption.

We might consider adding a knob to (not very silently) ignore and destroy
such buffers on vflush().
Comment 6 Alan Somers freebsd_committer freebsd_triage 2021-05-29 13:37:01 UTC
I don't think panicing is appropriate, kib.  This situation arrises due to a simple EIO by the disk.  Yes that means likely data loss, but it doesn't mean that the OS is damaged, and a reboot won't fix it.  We don't panic on EIO with non-clustered writes.  Why should we panic with clustered writes?
Comment 7 Konstantin Belousov freebsd_committer freebsd_triage 2021-05-29 13:56:16 UTC
(In reply to Alan Somers from comment #6)
I do not see how clustered or non-clustered matters there at all.  We panic if
there are dirty buffers left.

Perhaps your filesystem leaves dirty buffers only on clustered write failure.
Comment 8 Kirk McKusick freebsd_committer freebsd_triage 2021-05-29 22:13:29 UTC
I am confused as to why the EIO error is not being returned. Presumably in bufobj_invalbuf() we attempt to do the write in the call to BO_SYNC(). If BO_SYNC() returns EIO, we should return it rather than falling through to the panic. So how is it that the EIO is not returned?
Comment 9 Konstantin Belousov freebsd_committer freebsd_triage 2021-05-29 23:29:51 UTC
(In reply to Kirk McKusick from comment #8)
> I am confused as to why the EIO error is not being returned. Presumably in bufobj_invalbuf() we attempt to do the write in the call to BO_SYNC(). If BO_SYNC() returns EIO, we should return it rather than falling through to the panic. So how is it that the EIO is not returned?

BO_SYNC() does not see any error.  Default implementation of BO_SYNC() is bufsync()
which is just redirection to VOP_FSYNC().  And default implementation of vop_fsync
for filesystems using buffer cache is vn_fsync_buf(), which does bawrite()
(non-clustered) or vfs_bio_awrite() (clustered).  In either case, the real
action occurs in bufdone()->brelse() occuring on write completion.  Look at
the conditional with the inner comment 'Failed write, redirty.'  It does exactly
that: if the async write failed, the buffer is redirtied and re-inserted into the
dirty list.

After several loops where the dirty buffer is found/awritten/redirtied, fsync
eventually gives up, and vinvalbuf() gets the control back to find the dirty
buffer on the list and panic.
Comment 10 Kirk McKusick freebsd_committer freebsd_triage 2021-05-30 06:11:59 UTC
(In reply to Konstantin Belousov from comment #9)
Is the EIO error saved in b_error? If so, could we walk the list of dirty buffers
and if we found any with non-zero b_error return that error rather than falling through and panicing.
Comment 11 Konstantin Belousov freebsd_committer freebsd_triage 2021-05-30 17:14:45 UTC
(In reply to Kirk McKusick from comment #10)
I think we can do somewhat simpler: just not panic in this situation.
Then vgone() does another vinvalbuf() call without V_SAVE, which flushes
everything.

I put the minimal viable patch at https://reviews.freebsd.org/D30555
Comment 12 commit-hook freebsd_committer freebsd_triage 2021-05-30 22:22:27 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=27006229f7a40a18a61a0e8fd270bc583326b690

commit 27006229f7a40a18a61a0e8fd270bc583326b690
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-05-30 16:52:42 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-05-30 22:20:53 +0000

    vinvalbuf: do not panic if we were unable to flush dirty buffers

    Return EBUSY instead and let caller to handle the issue.

    For vgone()/vnode reclamation, caller first does vinvalbuf(V_SAVE),
    which return EBUSY in case dirty buffers where not flushed. Then caller
    calls vinvalbuf(0) due to non-zero return, which gets rid of all dirty
    buffers without dependencies.

    PR:     238565
    Reviewed by:    asomers, mckusick
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D30555

 sys/kern/vfs_subr.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
Comment 13 commit-hook freebsd_committer freebsd_triage 2021-05-30 22:54:35 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=425bbe9e64f7af6bdb30a099bd90a32885de1ab8

commit 425bbe9e64f7af6bdb30a099bd90a32885de1ab8
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-05-30 22:51:56 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-05-30 22:51:56 +0000

    fusefs: reenable the WriteCluster.cluster_write_err test

    The underlying panic was just fixed by
    revision 27006229f7a40a18a61a0e8fd270bc583326b690

    PR:             238565
    MFC after:      1 week
    MFC with:       27006229f7a40a18a61a0e8fd270bc583326b690

 tests/sys/fs/fusefs/write.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 14 commit-hook freebsd_committer freebsd_triage 2021-06-06 19:39:44 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=047905dc42996fa353e149ea5afddefe877ac701

commit 047905dc42996fa353e149ea5afddefe877ac701
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-05-30 16:52:42 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-06-06 19:27:32 +0000

    vinvalbuf: do not panic if we were unable to flush dirty buffers

    PR:     238565

    (cherry picked from commit 27006229f7a40a18a61a0e8fd270bc583326b690)

 sys/kern/vfs_subr.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
Comment 15 commit-hook freebsd_committer freebsd_triage 2021-06-17 20:09:35 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b6de677b705d6f10f605f9a011c18406e9b0cb9a

commit b6de677b705d6f10f605f9a011c18406e9b0cb9a
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-05-30 22:51:56 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-06-17 20:09:15 +0000

    fusefs: reenable the WriteCluster.cluster_write_err test

    The underlying panic was just fixed by
    revision 27006229f7a40a18a61a0e8fd270bc583326b690

    PR:             238565
    MFC with:       27006229f7a40a18a61a0e8fd270bc583326b690

    (cherry picked from commit 425bbe9e64f7af6bdb30a099bd90a32885de1ab8)

 tests/sys/fs/fusefs/write.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 16 commit-hook freebsd_committer freebsd_triage 2021-10-07 19:41:53 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=7dcb5db451b66aa277bd59c61c18a5053c658915

commit 7dcb5db451b66aa277bd59c61c18a5053c658915
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-05-30 16:52:42 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-10-07 19:34:19 +0000

    vinvalbuf: do not panic if we were unable to flush dirty buffers

    Return EBUSY instead and let caller to handle the issue.

    For vgone()/vnode reclamation, caller first does vinvalbuf(V_SAVE),
    which return EBUSY in case dirty buffers where not flushed. Then caller
    calls vinvalbuf(0) due to non-zero return, which gets rid of all dirty
    buffers without dependencies.

    PR:     238565
    Reviewed by:    asomers, mckusick
    Sponsored by:   The FreeBSD Foundation
    Differential revision:  https://reviews.freebsd.org/D30555

    (cherry picked from commit 27006229f7a40a18a61a0e8fd270bc583326b690)

    fusefs: reenable the WriteCluster.cluster_write_err test

    The underlying panic was just fixed by
    revision 27006229f7a40a18a61a0e8fd270bc583326b690

    PR:             238565
    (cherry picked from commit 425bbe9e64f7af6bdb30a099bd90a32885de1ab8)

 sys/kern/vfs_subr.c          | 10 ++++------
 tests/sys/fs/fusefs/write.cc |  4 ++--
 2 files changed, 6 insertions(+), 8 deletions(-)
Comment 17 commit-hook freebsd_committer freebsd_triage 2021-10-07 19:42:55 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=7dcb5db451b66aa277bd59c61c18a5053c658915

commit 7dcb5db451b66aa277bd59c61c18a5053c658915
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-05-30 16:52:42 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-10-07 19:34:19 +0000

    vinvalbuf: do not panic if we were unable to flush dirty buffers

    Return EBUSY instead and let caller to handle the issue.

    For vgone()/vnode reclamation, caller first does vinvalbuf(V_SAVE),
    which return EBUSY in case dirty buffers where not flushed. Then caller
    calls vinvalbuf(0) due to non-zero return, which gets rid of all dirty
    buffers without dependencies.

    PR:     238565
    Reviewed by:    asomers, mckusick
    Sponsored by:   The FreeBSD Foundation
    Differential revision:  https://reviews.freebsd.org/D30555

    (cherry picked from commit 27006229f7a40a18a61a0e8fd270bc583326b690)

    fusefs: reenable the WriteCluster.cluster_write_err test

    The underlying panic was just fixed by
    revision 27006229f7a40a18a61a0e8fd270bc583326b690

    PR:             238565
    (cherry picked from commit 425bbe9e64f7af6bdb30a099bd90a32885de1ab8)

 sys/kern/vfs_subr.c          | 10 ++++------
 tests/sys/fs/fusefs/write.cc |  4 ++--
 2 files changed, 6 insertions(+), 8 deletions(-)