Date: Tue, 03 Sep 2019 14:07:09 -0000 From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r346387 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201904191505.x3JF5Xrf054574@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Fri Apr 19 15:05:32 2019 New Revision: 346387 URL: https://svnweb.freebsd.org/changeset/base/346387 Log: 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 Modified: projects/fuse2/sys/fs/fuse/fuse_ipc.c projects/fuse2/tests/sys/fs/fusefs/interrupt.cc Modified: projects/fuse2/sys/fs/fuse/fuse_ipc.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_ipc.c Fri Apr 19 13:43:33 2019 (r346386) +++ projects/fuse2/sys/fs/fuse/fuse_ipc.c Fri Apr 19 15:05:32 2019 (r346387) @@ -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, stru * 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, stru } } +/* 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 @@ retry: 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 @@ retry: 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 */ Modified: projects/fuse2/tests/sys/fs/fusefs/interrupt.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/interrupt.cc Fri Apr 19 13:43:33 2019 (r346386) +++ projects/fuse2/tests/sys/fs/fusefs/interrupt.cc Fri Apr 19 15:05:32 2019 (r346387) @@ -32,6 +32,7 @@ extern "C" { #include <sys/wait.h> #include <fcntl.h> #include <pthread.h> +#include <semaphore.h> #include <signal.h> } @@ -290,6 +291,88 @@ TEST_F(Interrupt, ignore) ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); /* 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); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201904191505.x3JF5Xrf054574>