d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fusefs: fix running multiple daemons concurrently

Browse Source 
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
This commit is contained in:
Alan Somers 2019-05-10 15:02:29 +00:00
parent a87257ac25
commit 8b73a4c5ae
2 changed files with 42 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
sys/fs/fuse/fuse_device.c
View File

@ -94,14 +94,12 @@ SDT_PROBE_DEFINE2(fusefs, , device, trace, "int", "char*");
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);
}
@ -144,41 +165,6 @@ fuse_device_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
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
fuse_device_poll(struct cdev *dev, int events, struct thread *td)
{

28
tests/sys/fs/fusefs/mockfs.cc
View File

@ -147,7 +147,7 @@ ReturnImmediate(std::function<void(const struct mockfs_buf_in *in,
}
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, bool default_permissions,
.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, bool default_permissions,
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;