Bug 236475 - [ZFS] Invalid VOP_FSYNC options in vdev_file_io_start
Summary: [ZFS] Invalid VOP_FSYNC options in vdev_file_io_start
Status: Closed Not A Bug
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Andriy Gapon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-11 20:27 UTC by Alan Somers
Modified: 2019-03-22 17:48 UTC (History)
1 user (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-03-11 20:27:21 UTC
From inspection I can see that vdev_file_io_start provides invalid options to VOP_FSYNC.  The only valid options are MNT_WAIT, MNT_NOWAIT, and MNT_LAZY, but that function uses FSYNC | FDSYNC.  This bug has been present ever since the first import of ZFS.
Comment 1 commit-hook freebsd_committer freebsd_triage 2019-03-22 09:11:54 UTC
A commit references this bug:

Author: avg
Date: Fri Mar 22 09:11:46 UTC 2019
New revision: 345410
URL: https://svnweb.freebsd.org/changeset/base/345410

Log:
  ZFS vdev_file: use correct value for waitfor parameter of VOP_FSYNC

  PR:		236475
  Reported by:	asomers
  MFC after:	2 weeks

Changes:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c
Comment 2 Li-Wen Hsu freebsd_committer freebsd_triage 2019-03-22 16:37:28 UTC
After r345410, initialing ZFS triggers assertion failure:

https://ci.freebsd.org/job/FreeBSD-head-amd64-test/10583/consoleFull

panic: solaris assert: flag == FSYNC, file: /usr/src/sys/cddl/compat/opensolaris/sys/vnode.h, line: 238

Should this `ASSERT(flag == FSYNC);` also need to be changed?
Comment 3 commit-hook freebsd_committer freebsd_triage 2019-03-22 17:44:53 UTC
A commit references this bug:

Author: avg
Date: Fri Mar 22 17:44:47 UTC 2019
New revision: 345418
URL: https://svnweb.freebsd.org/changeset/base/345418

Log:
  Revert r345410, VOP_FSYNC change in ZFS vdev_file

  I overlooked the fact that that VOP_FSYNC() call is not a FreeBSD VFS
  call, but a macro that provides an illumos-compatible wrapper for the
  FreeBSD operation.

  PR:		236475
  Reported by:	lwhsu
  Pointyhat to:	avg

Changes:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c
Comment 4 Alan Somers freebsd_committer freebsd_triage 2019-03-22 17:48:15 UTC
Sorry for the false alarm, then.