From 102c7ac0837e0314342073339b4315b4c6e7bfa9 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 24 Apr 2019 17:30:50 +0000 Subject: [PATCH] fusefs: handle ENOSYS for FUSE_INTERRUPT Though it's not documented, Linux will interpret a FUSE_INTERRUPT response of ENOSYS as "the file system does not support FUSE_INTERRUPT". Subsequently it will never send FUSE_INTERRUPT again to the same mount point. This change matches Linux's behavior. PR: 346357 Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_ipc.c | 17 +++- tests/sys/fs/fusefs/interrupt.cc | 161 ++++++++++++++++++++++--------- 2 files changed, 125 insertions(+), 53 deletions(-) diff --git a/sys/fs/fuse/fuse_ipc.c b/sys/fs/fuse/fuse_ipc.c index 76686f40c9e5..531538f2e708 100644 --- a/sys/fs/fuse/fuse_ipc.c +++ b/sys/fs/fuse/fuse_ipc.c @@ -130,16 +130,13 @@ static uma_zone_t ticket_zone; /* * TODO: figure out how to timeout INTERRUPT requests, because the daemon may * leagally never respond - * - * TODO: remove an INTERRUPT request if the daemon responds to the original */ static int fuse_interrupt_callback(struct fuse_ticket *tick, struct uio *uio) { struct fuse_ticket *otick, *x_tick; struct fuse_interrupt_in *fii; - struct fuse_data *data; - data = tick->tk_data; + struct fuse_data *data = tick->tk_data; bool found = false; fii = (struct fuse_interrupt_in*)((char*)tick->tk_ms_fiov.base + @@ -162,7 +159,10 @@ fuse_interrupt_callback(struct fuse_ticket *tick, struct uio *uio) /* Clear the original ticket's interrupt association */ otick->irq_unique = 0; - if (tick->tk_aw_ohead.error == EAGAIN) { + if (tick->tk_aw_ohead.error == ENOSYS) { + fsess_set_notimpl(data->mp, FUSE_INTERRUPT); + return 0; + } else if (tick->tk_aw_ohead.error == EAGAIN) { /* * There are two reasons we might get this: * 1) the daemon received the INTERRUPT request before the @@ -220,6 +220,13 @@ fuse_interrupt_send(struct fuse_ticket *otick, int err) } fuse_lck_mtx_unlock(data->ms_mtx); + /* + * If the fuse daemon doesn't support interrupts, then there's + * nothing more that we can do + */ + if (!fsess_isimpl(data->mp, FUSE_INTERRUPT)) + return; + /* * If the fuse daemon has already received otick, then we must * send FUSE_INTERRUPT. diff --git a/tests/sys/fs/fusefs/interrupt.cc b/tests/sys/fs/fusefs/interrupt.cc index 52ece4589c7f..5b255c67ab8d 100644 --- a/tests/sys/fs/fusefs/interrupt.cc +++ b/tests/sys/fs/fusefs/interrupt.cc @@ -49,6 +49,8 @@ const off_t FILESIZE = 1000; const mode_t MODE = 0755; const char FULLDIRPATH0[] = "mountpoint/some_dir"; const char RELDIRPATH0[] = "some_dir"; +const char FULLDIRPATH1[] = "mountpoint/other_dir"; +const char RELDIRPATH1[] = "other_dir"; static sem_t *signaled_semaphore; @@ -163,6 +165,29 @@ void TearDown() { } }; +static void* mkdir0(void* arg __unused) { + ssize_t r; + + r = mkdir(FULLDIRPATH0, MODE); + if (r >= 0) + return 0; + else + return (void*)(intptr_t)errno; +} + +static void* read1(void* arg) { + const size_t bufsize = FILESIZE; + char buf[bufsize]; + int fd = (int)(intptr_t)arg; + ssize_t r; + + r = read(fd, buf, bufsize); + if (r >= 0) + return 0; + else + return (void*)(intptr_t)errno; +} + /* * An interrupt operation that gets received after the original command is * complete should generate an EAGAIN response. @@ -205,6 +230,89 @@ TEST_F(Interrupt, already_complete) EXPECT_EQ(0, mkdir(FULLDIRPATH0, MODE)) << strerror(errno); } +/* + * If a FUSE file system returns ENOSYS for a FUSE_INTERRUPT operation, the + * kernel should not attempt to interrupt any other operations on that mount + * point. + */ +TEST_F(Interrupt, enosys) +{ + uint64_t ino0 = 42, ino1 = 43;; + uint64_t mkdir_unique; + pthread_t self, th0; + sem_t sem0, sem1; + void *thr0_value; + Sequence seq; + + self = pthread_self(); + ASSERT_EQ(0, sem_init(&sem0, 0, 0)) << strerror(errno); + ASSERT_EQ(0, sem_init(&sem1, 0, 0)) << strerror(errno); + + EXPECT_LOOKUP(1, RELDIRPATH1).WillOnce(Invoke(ReturnErrno(ENOENT))); + EXPECT_LOOKUP(1, RELDIRPATH0).WillOnce(Invoke(ReturnErrno(ENOENT))); + expect_mkdir(&mkdir_unique); + EXPECT_CALL(*m_mock, process( + ResultOf([&](auto in) { + return (in->header.opcode == FUSE_INTERRUPT && + in->body.interrupt.unique == mkdir_unique); + }, Eq(true)), + _) + ).InSequence(seq) + .WillOnce(Invoke([&](auto in, auto &out) { + // reject FUSE_INTERRUPT and respond to the FUSE_WRITE + auto out0 = new mockfs_buf_out; + auto out1 = new mockfs_buf_out; + + out0->header.unique = in->header.unique; + out0->header.error = -ENOSYS; + out0->header.len = sizeof(out0->header); + out.push_back(out0); + + SET_OUT_HEADER_LEN(out1, entry); + out1->body.create.entry.attr.mode = S_IFDIR | MODE; + out1->body.create.entry.nodeid = ino1; + out1->header.unique = mkdir_unique; + out.push_back(out1); + })); + EXPECT_CALL(*m_mock, process( + ResultOf([&](auto in) { + return (in->header.opcode == FUSE_MKDIR); + }, Eq(true)), + _) + ).InSequence(seq) + .WillOnce(Invoke([&](auto in, auto &out) { + auto out0 = new mockfs_buf_out; + + sem_post(&sem0); + sem_wait(&sem1); + + SET_OUT_HEADER_LEN(out0, entry); + out0->body.create.entry.attr.mode = S_IFDIR | MODE; + out0->body.create.entry.nodeid = ino0; + out0->header.unique = in->header.unique; + out.push_back(out0); + })); + + setup_interruptor(self); + /* First mkdir operation should finish synchronously */ + ASSERT_EQ(0, mkdir(FULLDIRPATH1, MODE)) << strerror(errno); + + ASSERT_EQ(0, pthread_create(&th0, NULL, mkdir0, NULL)) + << strerror(errno); + + sem_wait(&sem0); + /* + * th0 should be blocked waiting for the fuse daemon thread. + * Signal it. No FUSE_INTERRUPT should result + */ + pthread_kill(th0, SIGUSR1); + /* Allow the daemon thread to proceed */ + sem_post(&sem1); + pthread_join(th0, &thr0_value); + /* Second mkdir should've finished without error */ + EXPECT_EQ(0, (intptr_t)thr0_value); +} + /* * Upon receipt of a fatal signal, fusefs should return ASAP after sending * FUSE_INTERRUPT. @@ -279,7 +387,7 @@ TEST_F(Interrupt, ignore) }, Eq(true)), _) ).WillOnce(Invoke([&](auto in __unused, auto &out) { - // Ignore FUSE_INTERRUPT; respond to the FUSE_WRITE + // Ignore FUSE_INTERRUPT; respond to the FUSE_MKDIR auto out0 = new mockfs_buf_out; out0->header.unique = mkdir_unique; SET_OUT_HEADER_LEN(out0, entry); @@ -292,42 +400,6 @@ TEST_F(Interrupt, ignore) ASSERT_EQ(0, mkdir(FULLDIRPATH0, MODE)) << strerror(errno); } -void* mkdir0(void* arg __unused) { - ssize_t r; - - r = mkdir(FULLDIRPATH0, MODE); - if (r >= 0) - return 0; - else - return (void*)(intptr_t)errno; -} - -void* setxattr0(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; -} - -void* read1(void* arg) { - const size_t bufsize = FILESIZE; - char buf[bufsize]; - int fd = (int)(intptr_t)arg; - ssize_t r; - - r = read(fd, buf, bufsize); - if (r >= 0) - return 0; - else - return (void*)(intptr_t)errno; -} - /* * A restartable operation (basically, anything except write or setextattr) * that hasn't yet been sent to userland can be interrupted without sending @@ -536,15 +608,11 @@ TEST_F(Interrupt, in_progress_read) setup_interruptor(self); ASSERT_EQ(-1, read(fd, buf, bufsize)); EXPECT_EQ(EINTR, errno); - - /* Deliberately leak fd. close(2) will be tested in release.cc */ } /* FUSE_INTERRUPT operations should take priority over other pending ops */ TEST_F(Interrupt, priority) { - const char FULLPATH1[] = "mountpoint/other_dir"; - const char RELPATH1[] = "other_dir"; Sequence seq; uint64_t ino1 = 43; uint64_t mkdir_unique; @@ -556,8 +624,7 @@ TEST_F(Interrupt, priority) self = pthread_self(); EXPECT_LOOKUP(1, RELDIRPATH0).WillOnce(Invoke(ReturnErrno(ENOENT))); - EXPECT_LOOKUP(1, RELPATH1).WillOnce(Invoke(ReturnErrno(ENOENT))); - //expect_mkdir(&mkdir_unique); + EXPECT_LOOKUP(1, RELDIRPATH1).WillOnce(Invoke(ReturnErrno(ENOENT))); EXPECT_CALL(*m_mock, process( ResultOf([=](auto in) { return (in->header.opcode == FUSE_MKDIR); @@ -579,8 +646,8 @@ TEST_F(Interrupt, priority) out->header.len = sizeof(out->header); }))); /* - * FUSE_INTERRUPT should be received before the second FUSE_MKDIR, even - * though it was generated later + * FUSE_INTERRUPT should be received before the second FUSE_MKDIR, + * even though it was generated later */ EXPECT_CALL(*m_mock, process( ResultOf([&](auto in) { @@ -610,7 +677,7 @@ TEST_F(Interrupt, priority) sem_wait(&sem1); /* Sequence the two mkdirs */ setup_interruptor(th0); - ASSERT_EQ(0, mkdir(FULLPATH1, MODE)) << strerror(errno); + ASSERT_EQ(0, mkdir(FULLDIRPATH1, MODE)) << strerror(errno); /* Wait awhile to make sure the signal generates no FUSE_INTERRUPT */ usleep(250'000); @@ -669,8 +736,6 @@ TEST_F(Interrupt, too_soon) setup_interruptor(self); ASSERT_EQ(-1, mkdir(FULLDIRPATH0, MODE)); EXPECT_EQ(EINTR, errno); - - /* Deliberately leak fd. close(2) will be tested in release.cc */ }