Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 10 May 2019 15:02:29 +0000 (UTC)
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r347431 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs
Message-ID:  <201905101502.x4AF2TuH034088@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: asomers
Date: Fri May 10 15:02:29 2019
New Revision: 347431
URL: https://svnweb.freebsd.org/changeset/base/347431

Log:
  fusefs: fix running multiple daemons concurrently
  
  When a FUSE daemon dies or closes /dev/fuse, all of that daemon's pending
  requests must be terminated.  Previously that was done in /dev/fuse's
  .d_close method.  However, d_close only gets called on the *last* close of
  the device.  That means that if multiple daemons were running concurrently,
  all but the last daemon to close would leave their I/O hanging around.  The
  problem was easily visible just by running "kyua -v parallelism=2 test" in
  fusefs's test directory.
  
  Fix this bug by terminating a daemon's pending I/O during /dev/fuse's
  cdvpriv dtor method instead.  That method runs on every close of a file.
  
  Also, fix some potential races in the tests:
  * Clear SA_RESTART when registering the daemon's signal handler so read(2)
    will return EINTR.
  * Wait for the daemon to die before unmounting the mountpoint, so we won't
    see an unwanted FUSE_DESTROY operation in the mock file system.
  
  Sponsored by:	The FreeBSD Foundation

Modified:
  projects/fuse2/sys/fs/fuse/fuse_device.c
  projects/fuse2/tests/sys/fs/fusefs/mockfs.cc

Modified: projects/fuse2/sys/fs/fuse/fuse_device.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_device.c	Fri May 10 13:41:19 2019	(r347430)
+++ projects/fuse2/sys/fs/fuse/fuse_device.c	Fri May 10 15:02:29 2019	(r347431)
@@ -94,14 +94,12 @@ SDT_PROBE_DEFINE2(fusefs, , device, trace, "int", "cha
 static struct cdev *fuse_dev;
 
 static d_open_t fuse_device_open;
-static d_close_t fuse_device_close;
 static d_poll_t fuse_device_poll;
 static d_read_t fuse_device_read;
 static d_write_t fuse_device_write;
 
 static struct cdevsw fuse_device_cdevsw = {
 	.d_open = fuse_device_open,
-	.d_close = fuse_device_close,
 	.d_name = "fuse",
 	.d_poll = fuse_device_poll,
 	.d_read = fuse_device_read,
@@ -119,8 +117,31 @@ static void
 fdata_dtor(void *arg)
 {
 	struct fuse_data *fdata;
+	struct fuse_ticket *tick;
 
 	fdata = arg;
+	if (fdata == NULL)
+		return;
+
+	fdata_set_dead(fdata);
+
+	FUSE_LOCK();
+	fuse_lck_mtx_lock(fdata->aw_mtx);
+	/* wakup poll()ers */
+	selwakeuppri(&fdata->ks_rsel, PZERO + 1);
+	/* Don't let syscall handlers wait in vain */
+	while ((tick = fuse_aw_pop(fdata))) {
+		fuse_lck_mtx_lock(tick->tk_aw_mtx);
+		fticket_set_answered(tick);
+		tick->tk_aw_errno = ENOTCONN;
+		wakeup(tick);
+		fuse_lck_mtx_unlock(tick->tk_aw_mtx);
+		FUSE_ASSERT_AW_DONE(tick);
+		fuse_ticket_drop(tick);
+	}
+	fuse_lck_mtx_unlock(fdata->aw_mtx);
+	FUSE_UNLOCK();
+
 	fdata_trydestroy(fdata);
 }
 
@@ -142,41 +163,6 @@ fuse_device_open(struct cdev *dev, int oflags, int dev
 	else
 		SDT_PROBE2(fusefs, , device, trace, 1, "device open success");
 	return (error);
-}
-
-static int
-fuse_device_close(struct cdev *dev, int fflag, int devtype, struct thread *td)
-{
-	struct fuse_data *data;
-	struct fuse_ticket *tick;
-	int error;
-
-	error = devfs_get_cdevpriv((void **)&data);
-	if (error != 0)
-		return (error);
-	if (!data)
-		panic("no fuse data upon fuse device close");
-	fdata_set_dead(data);
-
-	FUSE_LOCK();
-	fuse_lck_mtx_lock(data->aw_mtx);
-	/* wakup poll()ers */
-	selwakeuppri(&data->ks_rsel, PZERO + 1);
-	/* Don't let syscall handlers wait in vain */
-	while ((tick = fuse_aw_pop(data))) {
-		fuse_lck_mtx_lock(tick->tk_aw_mtx);
-		fticket_set_answered(tick);
-		tick->tk_aw_errno = ENOTCONN;
-		wakeup(tick);
-		fuse_lck_mtx_unlock(tick->tk_aw_mtx);
-		FUSE_ASSERT_AW_DONE(tick);
-		fuse_ticket_drop(tick);
-	}
-	fuse_lck_mtx_unlock(data->aw_mtx);
-	FUSE_UNLOCK();
-
-	SDT_PROBE2(fusefs, , device, trace, 1, "device close");
-	return (0);
 }
 
 int

Modified: projects/fuse2/tests/sys/fs/fusefs/mockfs.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/mockfs.cc	Fri May 10 13:41:19 2019	(r347430)
+++ projects/fuse2/tests/sys/fs/fusefs/mockfs.cc	Fri May 10 15:02:29 2019	(r347431)
@@ -147,7 +147,7 @@ ReturnImmediate(std::function<void(const struct mockfs
 }
 
 void sigint_handler(int __unused sig) {
-	quit = 1;
+	// Don't do anything except interrupt the daemon's read(2) call
 }
 
 void debug_fuseop(const mockfs_buf_in *in)
@@ -280,6 +280,7 @@ void debug_fuseop(const mockfs_buf_in *in)
 MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions,
 	bool push_symlinks_in, bool ro, uint32_t flags)
 {
+	struct sigaction sa;
 	struct iovec *iov = NULL;
 	int iovlen = 0;
 	char fdstr[15];
@@ -340,7 +341,12 @@ MockFS::MockFS(int max_readahead, bool allow_other, bo
 		.WillByDefault(Invoke(this, &MockFS::process_default));
 
 	init(flags);
-	signal(SIGUSR1, sigint_handler);
+	bzero(&sa, sizeof(sa));
+	sa.sa_handler = sigint_handler;
+	sa.sa_flags = 0;	/* Don't set SA_RESTART! */
+	if (0 != sigaction(SIGUSR1, &sa, NULL))
+		throw(std::system_error(errno, std::system_category(),
+			"Couldn't handle SIGUSR1"));
 	if (pthread_create(&m_daemon_id, NULL, service, (void*)this))
 		throw(std::system_error(errno, std::system_category(),
 			"Couldn't Couldn't start fuse thread"));
@@ -348,11 +354,11 @@ MockFS::MockFS(int max_readahead, bool allow_other, bo
 
 MockFS::~MockFS() {
 	kill_daemon();
-	::unmount("mountpoint", MNT_FORCE);
 	if (m_daemon_id != NULL) {
 		pthread_join(m_daemon_id, NULL);
 		m_daemon_id = NULL;
 	}
+	::unmount("mountpoint", MNT_FORCE);
 	rmdir("mountpoint");
 }
 
@@ -393,13 +399,14 @@ void MockFS::init(uint32_t flags) {
 }
 
 void MockFS::kill_daemon() {
-	if (m_daemon_id != NULL) {
+	quit = 1;
+	if (m_daemon_id != NULL)
 		pthread_kill(m_daemon_id, SIGUSR1);
-		// Closing the /dev/fuse file descriptor first allows unmount
-		// to succeed even if the daemon doesn't correctly respond to
-		// commands during the unmount sequence.
-		close(m_fuse_fd);
-	}
+	// Closing the /dev/fuse file descriptor first allows unmount to
+	// succeed even if the daemon doesn't correctly respond to commands
+	// during the unmount sequence.
+	close(m_fuse_fd);
+	m_fuse_fd = -1;
 }
 
 void MockFS::loop() {
@@ -462,6 +469,9 @@ bool MockFS::pid_ok(pid_t pid) {
 void MockFS::process_default(const mockfs_buf_in *in,
 		std::vector<mockfs_buf_out*> &out)
 {
+	if (verbosity > 1)
+		printf("%-11s REJECTED (wrong pid %d)\n",
+			opcode2opname(in->header.opcode), in->header.pid);
 	auto out0 = new mockfs_buf_out;
 	out0->header.unique = in->header.unique;
 	out0->header.error = -EOPNOTSUPP;

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