From 3d070fdc76030f23eb3be8bf2ffa9614100cf5ea Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 19 Apr 2019 15:05:32 +0000 Subject: [PATCH] fusefs: don't send FUSE_INTERRUPT for ops that are still in-kernel If a pending FUSE operation hasn't yet been sent to the daemon, then there's no reason to inform the daemon that it's been interrupted. Instead, simply remove it from the fuse message queue and set its status to EINTR or ERESTART as appropriate. PR: 346357 Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_ipc.c | 52 ++++++++++++++++---- tests/sys/fs/fusefs/interrupt.cc | 83 ++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 9 deletions(-) diff --git a/sys/fs/fuse/fuse_ipc.c b/sys/fs/fuse/fuse_ipc.c index 10363a4a0372..da71496f35c6 100644 --- a/sys/fs/fuse/fuse_ipc.c +++ b/sys/fs/fuse/fuse_ipc.c @@ -95,7 +95,7 @@ SDT_PROBE_DEFINE2(fuse, , ipc, trace, "int", "char*"); static void fdisp_make_pid(struct fuse_dispatcher *fdip, enum fuse_opcode op, struct fuse_data *data, uint64_t nid, pid_t pid, struct ucred *cred); static void fiov_clear(struct fuse_iov *fiov); -static void fuse_interrupt_send(struct fuse_ticket *otick); +static void fuse_interrupt_send(struct fuse_ticket *otick, int err); static struct fuse_ticket *fticket_alloc(struct fuse_data *data); static void fticket_refresh(struct fuse_ticket *ftick); static void fticket_destroy(struct fuse_ticket *ftick); @@ -175,7 +175,7 @@ fuse_interrupt_callback(struct fuse_ticket *tick, struct uio *uio) * second, we should ignore it. */ /* Resend */ - fuse_interrupt_send(otick); + fuse_interrupt_send(otick, EINTR); return 0; } else { /* Illegal FUSE_INTERRUPT response */ @@ -183,16 +183,49 @@ fuse_interrupt_callback(struct fuse_ticket *tick, struct uio *uio) } } +/* Interrupt the operation otick. Return err as its error code */ void -fuse_interrupt_send(struct fuse_ticket *otick) +fuse_interrupt_send(struct fuse_ticket *otick, int err) { struct fuse_dispatcher fdi; struct fuse_interrupt_in *fii; struct fuse_in_header *ftick_hdr; struct fuse_data *data = otick->tk_data; + struct fuse_ticket *tick, *xtick; struct ucred reused_creds; if (otick->irq_unique == 0) { + /* + * If the daemon hasn't yet received otick, then we can answer + * it ourselves and return. + */ + fuse_lck_mtx_lock(data->ms_mtx); + STAILQ_FOREACH_SAFE(tick, &otick->tk_data->ms_head, tk_ms_link, + xtick) { + if (tick == otick) { + STAILQ_REMOVE(&otick->tk_data->ms_head, tick, + fuse_ticket, tk_ms_link); + otick->tk_ms_link.stqe_next = NULL; + fuse_lck_mtx_unlock(data->ms_mtx); + + fuse_lck_mtx_lock(otick->tk_aw_mtx); + if (!fticket_answered(otick)) { + fticket_set_answered(otick); + otick->tk_aw_errno = err; + wakeup(otick); + } + fuse_lck_mtx_unlock(otick->tk_aw_mtx); + + fuse_ticket_drop(tick); + return; + } + } + fuse_lck_mtx_unlock(data->ms_mtx); + + /* + * If the fuse daemon has already received otick, then we must + * send FUSE_INTERRUPT. + */ ftick_hdr = fticket_in_header(otick); reused_creds.cr_uid = ftick_hdr->uid; reused_creds.cr_rgid = ftick_hdr->gid; @@ -400,11 +433,12 @@ fticket_wait_answer(struct fuse_ticket *ftick) sigset_t blockedset, oldset; int err = 0; struct fuse_data *data; + SIGEMPTYSET(blockedset); + + kern_sigprocmask(td, SIG_BLOCK, NULL, &oldset, 0); fuse_lck_mtx_lock(ftick->tk_aw_mtx); - SIGEMPTYSET(blockedset); - kern_sigprocmask(td, SIG_BLOCK, &blockedset, &oldset, 0); retry: if (fticket_answered(ftick)) { goto out; @@ -416,6 +450,7 @@ fticket_wait_answer(struct fuse_ticket *ftick) fticket_set_answered(ftick); goto out; } + kern_sigprocmask(td, SIG_BLOCK, &blockedset, NULL, 0); err = msleep(ftick, &ftick->tk_aw_mtx, PCATCH, "fu_ans", data->daemon_timeout * hz); kern_sigprocmask(td, SIG_SETMASK, &oldset, NULL, 0); @@ -446,22 +481,21 @@ fticket_wait_answer(struct fuse_ticket *ftick) SDT_PROBE2(fuse, , ipc, trace, 4, "fticket_wait_answer: interrupt"); fuse_lck_mtx_unlock(ftick->tk_aw_mtx); - fuse_interrupt_send(ftick); - fuse_lck_mtx_lock(ftick->tk_aw_mtx); + fuse_interrupt_send(ftick, err); PROC_LOCK(td->td_proc); mtx_lock(&td->td_proc->p_sigacts->ps_mtx); sig = cursig(td); mtx_unlock(&td->td_proc->p_sigacts->ps_mtx); PROC_UNLOCK(td->td_proc); + + fuse_lck_mtx_lock(ftick->tk_aw_mtx); if (!sig_isfatal(td->td_proc, sig)) { /* * Block the just-delivered signal while we wait for an * interrupt response */ SIGADDSET(blockedset, sig); - kern_sigprocmask(curthread, SIG_BLOCK, &blockedset, - NULL, 0); goto retry; } else { /* Return immediately for fatal signals */ diff --git a/tests/sys/fs/fusefs/interrupt.cc b/tests/sys/fs/fusefs/interrupt.cc index b6620cd47c5b..30960add7e64 100644 --- a/tests/sys/fs/fusefs/interrupt.cc +++ b/tests/sys/fs/fusefs/interrupt.cc @@ -32,6 +32,7 @@ extern "C" { #include #include #include +#include #include } @@ -292,6 +293,88 @@ TEST_F(Interrupt, ignore) /* Deliberately leak fd. close(2) will be tested in release.cc */ } +void* write0(void* arg) { + const char *CONTENTS = "abcdefgh"; + ssize_t bufsize = strlen(CONTENTS); + int fd = (int)(intptr_t)arg; + ssize_t r; + + r = write(fd, CONTENTS, bufsize); + if (r >= 0) + return 0; + else + return (void*)(intptr_t)errno; +} + +/* + * An operation that hasn't yet been sent to userland can be interrupted + * without sending FUSE_INTERRUPT + */ +TEST_F(Interrupt, in_kernel) +{ + const char FULLPATH0[] = "mountpoint/some_file.txt"; + const char RELPATH0[] = "some_file.txt"; + const char FULLPATH1[] = "mountpoint/other_file.txt"; + const char RELPATH1[] = "other_file.txt"; + const char *CONTENTS = "ijklmnop"; + ssize_t bufsize = strlen(CONTENTS); + uint64_t ino0 = 42, ino1 = 43; + int fd0, fd1; + pthread_t self, th0; + sem_t sem0, sem1; + void *thr0_value; + + ASSERT_EQ(0, sem_init(&sem0, 0, 0)) << strerror(errno); + ASSERT_EQ(0, sem_init(&sem1, 0, 0)) << strerror(errno); + self = pthread_self(); + + expect_lookup(RELPATH0, ino0); + expect_open(ino0, 0, 1); + expect_lookup(RELPATH1, ino1); + expect_open(ino1, 0, 1); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_WRITE && + in->header.nodeid == ino0); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([&](auto in, auto out) { + /* Let the next write proceed */ + sem_post(&sem1); + /* Pause the daemon thread so it won't read the next op */ + sem_wait(&sem0); + + SET_OUT_HEADER_LEN(out, write); + out->body.write.size = in->body.write.size; + }))); + + fd0 = open(FULLPATH0, O_WRONLY); + ASSERT_LE(0, fd0) << strerror(errno); + fd1 = open(FULLPATH1, O_WRONLY); + ASSERT_LE(0, fd1) << strerror(errno); + + /* Use a separate thread for the first write */ + ASSERT_EQ(0, pthread_create(&th0, NULL, write0, (void*)(intptr_t)fd0)) + << strerror(errno); + + setup_interruptor(self); + + sem_wait(&sem1); /* Sequence the two writes */ + ASSERT_EQ(-1, write(fd1, CONTENTS, bufsize)); + EXPECT_EQ(EINTR, errno); + + /* Unstick the daemon */ + ASSERT_EQ(0, sem_post(&sem0)) << strerror(errno); + + /* Wait awhile to make sure the signal generates no FUSE_INTERRUPT */ + usleep(250'000); + + pthread_join(th0, &thr0_value); + EXPECT_EQ(0, (intptr_t)thr0_value); + sem_destroy(&sem1); + sem_destroy(&sem0); +} + /* * A syscall that gets interrupted while blocking on FUSE I/O should send a * FUSE_INTERRUPT command to the fuse filesystem, which should then send EINTR