Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 21 Mar 2019 23:01:57 +0000 (UTC)
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r345398 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs
Message-ID:  <201903212301.x2LN1vrM067692@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: asomers
Date: Thu Mar 21 23:01:56 2019
New Revision: 345398
URL: https://svnweb.freebsd.org/changeset/base/345398

Log:
  fusefs: Don't treat fsync the same as fdatasync
  
  For an unknown reason, fusefs was _always_ sending the fdatasync operation
  instead of fsync.  Now it correctly sends one or the other.
  
  Also, remove the Fsync.fsync_metadata_only test, along with the recently
  removed Fsync.nop.  They should never have been added.  The kernel shouldn't
  keep track of which files have dirty data; that's the daemon's job.
  
  PR:		236473
  Sponsored by:	The FreeBSD Foundation

Modified:
  projects/fuse2/sys/fs/fuse/fuse_internal.c
  projects/fuse2/sys/fs/fuse/fuse_internal.h
  projects/fuse2/sys/fs/fuse/fuse_vnops.c
  projects/fuse2/tests/sys/fs/fusefs/fsync.cc

Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.c	Thu Mar 21 22:40:05 2019	(r345397)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.c	Thu Mar 21 23:01:56 2019	(r345398)
@@ -281,32 +281,45 @@ fuse_internal_fsync_callback(struct fuse_ticket *tick,
 int
 fuse_internal_fsync(struct vnode *vp,
     struct thread *td,
-    struct ucred *cred,
-    struct fuse_filehandle *fufh,
-    int waitfor)
+    int waitfor,
+    bool datasync)
 {
-	int op = FUSE_FSYNC;
 	struct fuse_fsync_in *ffsi;
 	struct fuse_dispatcher fdi;
+	struct fuse_filehandle *fufh;
+	struct fuse_vnode_data *fvdat = VTOFUD(vp);
+	int op = FUSE_FSYNC;
+	int type = 0;
 	int err = 0;
 
-	if (vnode_isdir(vp)) {
-		op = FUSE_FSYNCDIR;
+	if (!fsess_isimpl(vnode_mount(vp),
+	    (vnode_vtype(vp) == VDIR ? FUSE_FSYNCDIR : FUSE_FSYNC))) {
+		return 0;
 	}
-	fdisp_init(&fdi, sizeof(*ffsi));
-	fdisp_make_vp(&fdi, op, vp, td, cred);
-	ffsi = fdi.indata;
-	ffsi->fh = fufh->fh_id;
+	for (type = 0; type < FUFH_MAXTYPE; type++) {
+		fufh = &(fvdat->fufh[type]);
+		if (FUFH_IS_VALID(fufh)) {
+			if (vnode_isdir(vp)) {
+				op = FUSE_FSYNCDIR;
+			}
+			fdisp_init(&fdi, sizeof(*ffsi));
+			fdisp_make_vp(&fdi, op, vp, td, NULL);
+			ffsi = fdi.indata;
+			ffsi->fh = fufh->fh_id;
 
-	ffsi->fsync_flags = 1;		/* datasync */
+			if (datasync)
+				ffsi->fsync_flags = 1;
 
-	if (waitfor == MNT_WAIT) {
-		err = fdisp_wait_answ(&fdi);
-	} else {
-		fuse_insert_callback(fdi.tick, fuse_internal_fsync_callback);
-		fuse_insert_message(fdi.tick);
+			if (waitfor == MNT_WAIT) {
+				err = fdisp_wait_answ(&fdi);
+			} else {
+				fuse_insert_callback(fdi.tick,
+					fuse_internal_fsync_callback);
+				fuse_insert_message(fdi.tick);
+			}
+			fdisp_destroy(&fdi);
+		}
 	}
-	fdisp_destroy(&fdi);
 
 	return err;
 }

Modified: projects/fuse2/sys/fs/fuse/fuse_internal.h
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.h	Thu Mar 21 22:40:05 2019	(r345397)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.h	Thu Mar 21 23:01:56 2019	(r345398)
@@ -198,8 +198,8 @@ void fuse_internal_cache_attrs(struct vnode *vp, struc
 
 /* fsync */
 
-int fuse_internal_fsync(struct vnode *vp, struct thread *td,
-    struct ucred *cred, struct fuse_filehandle *fufh, int waitfor);
+int fuse_internal_fsync(struct vnode *vp, struct thread *td, int waitfor,
+	bool datasync);
 int fuse_internal_fsync_callback(struct fuse_ticket *tick, struct uio *uio);
 
 /* readdir */

Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_vnops.c	Thu Mar 21 22:40:05 2019	(r345397)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c	Thu Mar 21 23:01:56 2019	(r345398)
@@ -120,6 +120,7 @@ static vop_access_t fuse_vnop_access;
 static vop_close_t fuse_vnop_close;
 static vop_create_t fuse_vnop_create;
 static vop_deleteextattr_t fuse_vnop_deleteextattr;
+static vop_fdatasync_t fuse_vnop_fdatasync;
 static vop_fsync_t fuse_vnop_fsync;
 static vop_getattr_t fuse_vnop_getattr;
 static vop_getextattr_t fuse_vnop_getextattr;
@@ -154,6 +155,7 @@ struct vop_vector fuse_vnops = {
 	.vop_create = fuse_vnop_create,
 	.vop_deleteextattr = fuse_vnop_deleteextattr,
 	.vop_fsync = fuse_vnop_fsync,
+	.vop_fdatasync = fuse_vnop_fdatasync,
 	.vop_getattr = fuse_vnop_getattr,
 	.vop_getextattr = fuse_vnop_getextattr,
 	.vop_inactive = fuse_vnop_inactive,
@@ -410,22 +412,34 @@ out:
 }
 
 /*
- * Our vnop_fsync roughly corresponds to the FUSE_FSYNC method. The Linux
- * version of FUSE also has a FUSE_FLUSH method.
- *
- * On Linux, fsync() synchronizes a file's complete in-core state with that
- * on disk. The call is not supposed to return until the system has completed
- * that action or until an error is detected.
- *
- * Linux also has an fdatasync() call that is similar to fsync() but is not
- * required to update the metadata such as access time and modification time.
- */
+    struct vnop_fdatasync_args {
+	struct vop_generic_args a_gen;
+	struct vnode * a_vp;
+	struct thread * a_td;
+    };
+*/
+static int
+fuse_vnop_fdatasync(struct vop_fdatasync_args *ap)
+{
+	struct vnode *vp = ap->a_vp;
+	struct thread *td = ap->a_td;
+	int waitfor = MNT_WAIT;
 
+	int err = 0;
+
+	if (fuse_isdeadfs(vp)) {
+		return 0;
+	}
+	if ((err = vop_stdfdatasync_buf(ap)))
+		return err;
+
+	return fuse_internal_fsync(vp, td, waitfor, true);
+}
+
 /*
     struct vnop_fsync_args {
-	struct vnodeop_desc *a_desc;
+	struct vop_generic_args a_gen;
 	struct vnode * a_vp;
-	struct ucred * a_cred;
 	int  a_waitfor;
 	struct thread * a_td;
     };
@@ -436,31 +450,15 @@ fuse_vnop_fsync(struct vop_fsync_args *ap)
 	struct vnode *vp = ap->a_vp;
 	struct thread *td = ap->a_td;
 	int waitfor = ap->a_waitfor;
+	int err = 0;
 
-	struct fuse_filehandle *fufh;
-	struct fuse_vnode_data *fvdat = VTOFUD(vp);
-
-	int type, err = 0;
-
 	if (fuse_isdeadfs(vp)) {
 		return 0;
 	}
 	if ((err = vop_stdfsync(ap)))
 		return err;
 
-	if (!fsess_isimpl(vnode_mount(vp),
-	    (vnode_vtype(vp) == VDIR ? FUSE_FSYNCDIR : FUSE_FSYNC))) {
-		goto out;
-	}
-	for (type = 0; type < FUFH_MAXTYPE; type++) {
-		fufh = &(fvdat->fufh[type]);
-		if (FUFH_IS_VALID(fufh)) {
-			err = fuse_internal_fsync(vp, td, NULL, fufh, waitfor);
-		}
-	}
-
-out:
-	return err;
+	return fuse_internal_fsync(vp, td, waitfor, false);
 }
 
 /*

Modified: projects/fuse2/tests/sys/fs/fusefs/fsync.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/fsync.cc	Thu Mar 21 22:40:05 2019	(r345397)
+++ projects/fuse2/tests/sys/fs/fusefs/fsync.cc	Thu Mar 21 23:01:56 2019	(r345398)
@@ -76,7 +76,6 @@ void expect_write(uint64_t ino, uint64_t size, const v
 };
 
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236379 */
-/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */
 TEST_F(Fsync, DISABLED_aio_fsync)
 {
 	const char FULLPATH[] = "mountpoint/some_file.txt";
@@ -227,8 +226,7 @@ TEST_F(Fsync, fdatasync)
 	/* Deliberately leak fd.  close(2) will be tested in release.cc */
 }
 
-/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */
-TEST_F(Fsync, DISABLED_fsync)
+TEST_F(Fsync, fsync)
 {
 	const char FULLPATH[] = "mountpoint/some_file.txt";
 	const char RELPATH[] = "some_file.txt";
@@ -248,38 +246,5 @@ TEST_F(Fsync, DISABLED_fsync)
 	ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno);
 	ASSERT_EQ(0, fsync(fd)) << strerror(errno);
 
-	/* Deliberately leak fd.  close(2) will be tested in release.cc */
-}
-
-/* Fsync should sync a file with dirty metadata but clean data */
-/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */
-TEST_F(Fsync, DISABLED_fsync_metadata_only)
-{
-	const char FULLPATH[] = "mountpoint/some_file.txt";
-	const char RELPATH[] = "some_file.txt";
-	uint64_t ino = 42;
-	int fd;
-	mode_t mode = 0755;
-
-	expect_lookup(RELPATH, ino);
-	expect_open(ino, 0, 1);
-	expect_getattr(ino, 0);
-	EXPECT_CALL(*m_mock, process(
-		ResultOf([=](auto in) {
-			return (in->header.opcode == FUSE_SETATTR);
-		}, Eq(true)),
-		_)
-	).WillOnce(Invoke(ReturnImmediate([=](auto i __unused, auto out) {
-		SET_OUT_HEADER_LEN(out, attr);
-		out->body.attr.attr.ino = ino;	// Must match nodeid
-		out->body.attr.attr.mode = S_IFREG | mode;
-	})));
-
-	expect_fsync(ino, 0, 0);
-
-	fd = open(FULLPATH, O_RDWR);
-	ASSERT_LE(0, fd) << strerror(errno);
-	ASSERT_EQ(0, fchmod(fd, mode)) << strerror(errno);
-	ASSERT_EQ(0, fsync(fd)) << strerror(errno);
 	/* Deliberately leak fd.  close(2) will be tested in release.cc */
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903212301.x2LN1vrM067692>