Page MenuHomeFreeBSD
Files F142859831

D54850.id170296.diff
No OneTemporary
Actions

D54850.id170296.diff
View Options

diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c
--- a/sys/fs/fuse/fuse_internal.c
+++ b/sys/fs/fuse/fuse_internal.c
@@ -245,8 +245,9 @@
* 'vp'. Optionally, if argument 'vap' is not NULL, store a copy of the
* converted attributes there as well.
*
- * If the nominal attribute cache TTL is zero, do not cache on the 'vp' (but do
- * return the result to the caller).
+ * If the nominal attribute cache TTL is zero, or if we cannot acquire an
+ * exclusive vnode lock, then do not cache on the 'vp' (but do return the
+ * result to the caller).
*/
void
fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
@@ -257,15 +258,34 @@
struct fuse_vnode_data *fvdat;
struct fuse_data *data;
struct vattr *vp_cache_at;
+ bool ex, downgrade;
mp = vnode_mount(vp);
fvdat = VTOFUD(vp);
data = fuse_get_mpdata(mp);
- ASSERT_VOP_ELOCKED(vp, "fuse_internal_cache_attrs");
+ ASSERT_VOP_LOCKED(vp, "fuse_internal_cache_attrs");
+ if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE) {
+ downgrade = false;
+ ex = true;
+ } else if (0 == vn_lock(vp, LK_TRYUPGRADE)) {
+ downgrade = true;
+ ex = true;
+ } else {
+ /*
+ * We can't acquire an exclusive lock. Proceed anyway, but
+ * _don't_ update the vnode cache. The cache will still be
+ * stale, so some other operation will soon fetch the
+ * attributes again. With luck, it will get an exclusive lock
+ * and be able to update the vnode cache.
+ */
+ downgrade = false;
+ ex = false;
+ }
- fuse_validity_2_bintime(attr_valid, attr_valid_nsec,
- &fvdat->attr_cache_timeout);
+ if (ex)
+ fuse_validity_2_bintime(attr_valid, attr_valid_nsec,
+ &fvdat->attr_cache_timeout);
if (vnode_isreg(vp) &&
fvdat->cached_attrs.va_size != VNOVAL &&
@@ -313,17 +333,18 @@
}
/* Fix our buffers if the filesize changed without us knowing */
- if (vnode_isreg(vp) && attr->size != fvdat->cached_attrs.va_size) {
+ if (vnode_isreg(vp) && attr->size != fvdat->cached_attrs.va_size && ex)
+ {
(void)fuse_vnode_setsize(vp, attr->size, from_server);
fvdat->cached_attrs.va_size = attr->size;
}
- if (attr_valid > 0 || attr_valid_nsec > 0)
+ if (ex && (attr_valid > 0 || attr_valid_nsec > 0))
vp_cache_at = &(fvdat->cached_attrs);
else if (vap != NULL)
vp_cache_at = vap;
else
- return;
+ goto out;
vp_cache_at->va_fsid = mp->mnt_stat.f_fsid.val[0];
vp_cache_at->va_fileid = attr->ino;
@@ -350,6 +371,10 @@
if (vap != vp_cache_at && vap != NULL)
memcpy(vap, vp_cache_at, sizeof(*vap));
+
+out:
+ if (downgrade)
+ MPASS(0 == VOP_LOCK(vp, LK_DOWNGRADE));
}
/* fsync */
diff --git a/tests/sys/fs/fusefs/read.cc b/tests/sys/fs/fusefs/read.cc
--- a/tests/sys/fs/fusefs/read.cc
+++ b/tests/sys/fs/fusefs/read.cc
@@ -95,6 +95,19 @@
}
};
+class AsyncReadNoAttrCache: public Read {
+virtual void SetUp() {
+ m_init_flags = FUSE_ASYNC_READ;
+ Read::SetUp();
+}
+public:
+void expect_lookup(const char *relpath, uint64_t ino)
+{
+ // Don't return size, and set attr_valid=0
+ FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, 1, 0);
+}
+};
+
class ReadAhead: public Read,
public WithParamInterface<tuple<bool, int>>
{
@@ -351,6 +364,52 @@
leak(fd);
}
+/*
+ * If the daemon disables the attribute cache (or if it has expired), then the
+ * kernel must fetch attributes during VOP_READ. If async reads are enabled,
+ * then fuse_internal_cache_attrs will be called without the vnode exclusively
+ * locked. Regression test for
+ * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=291064
+ */
+TEST_F(AsyncReadNoAttrCache, read)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ const char *CONTENTS = "abcdefgh";
+ uint64_t ino = 42;
+ mode_t mode = S_IFREG | 0644;
+ int fd;
+ ssize_t bufsize = strlen(CONTENTS);
+ uint8_t buf[bufsize];
+ Sequence seq;
+
+ expect_lookup(RELPATH, ino);
+ expect_open(ino, 0, 1);
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in.header.opcode == FUSE_GETATTR &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto& out)
+ {
+ SET_OUT_HEADER_LEN(out, attr);
+ out.body.attr.attr.ino = ino;
+ out.body.attr.attr.mode = mode;
+ out.body.attr.attr.size = bufsize;
+ out.body.attr.attr_valid = 0;
+ })));
+ expect_read(ino, 0, bufsize, bufsize, CONTENTS);
+
+ fd = open(FULLPATH, O_RDONLY);
+ ASSERT_LE(0, fd) << strerror(errno);
+
+ ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno);
+ ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize));
+
+ leak(fd);
+}
+
/* The kernel should update the cached atime attribute during a read */
TEST_F(Read, atime)
{

File Metadata

Mime Type
text/plain
Expires
Sun, Jan 25, 6:01 AM (19 h, 18 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
27928818
Default Alt Text
D54850.id170296.diff (4 KB)

Event Timeline