Page MenuHomeFreeBSD
Files F156948363

D32332.id96315.diff
No OneTemporary
Actions

D32332.id96315.diff
View Options

Index: sys/fs/fuse/fuse_internal.h
===================================================================
--- sys/fs/fuse/fuse_internal.h
+++ sys/fs/fuse/fuse_internal.h
@@ -223,7 +223,8 @@
/* attributes */
void fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
- uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap);
+ uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap,
+ bool from_server);
/* fsync */
Index: sys/fs/fuse/fuse_internal.c
===================================================================
--- sys/fs/fuse/fuse_internal.c
+++ sys/fs/fuse/fuse_internal.c
@@ -255,7 +255,8 @@
*/
void
fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
- uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap)
+ uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap,
+ bool from_server)
{
struct mount *mp;
struct fuse_vnode_data *fvdat;
@@ -271,9 +272,54 @@
fuse_validity_2_bintime(attr_valid, attr_valid_nsec,
&fvdat->attr_cache_timeout);
+ if (vnode_isreg(vp) &&
+ fvdat->cached_attrs.va_size != VNOVAL &&
+ attr->size != fvdat->cached_attrs.va_size)
+ {
+ if ( data->cache_mode == FUSE_CACHE_WB &&
+ fvdat->flag & FN_SIZECHANGE)
+ {
+ const char *msg;
+
+ /*
+ * The server changed the file's size even though we're
+ * using writeback cacheing and and we have outstanding
+ * dirty writes! That's a server bug.
+ */
+ if (fuse_libabi_geq(data, 7, 23)) {
+ msg = "writeback cache incoherent!."
+ "To prevent data corruption, disable "
+ "the writeback cache according to your "
+ "FUSE server's documentation.";
+ } else {
+ msg = "writeback cache incoherent!."
+ "To prevent data corruption, disable "
+ "the writeback cache by setting "
+ "vfs.fusefs.data_cache_mode to 0 or 1.";
+ }
+ fuse_warn(data, FSESS_WARN_WB_CACHE_INCOHERENT, msg);
+ }
+ if (fuse_vnode_attr_cache_valid(vp) &&
+ data->cache_mode != FUSE_CACHE_UC)
+ {
+ /*
+ * The server changed the file's size even though we
+ * have it cached and our cache has not yet expired.
+ * That's a bug.
+ */
+ fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT,
+ "cache incoherent! "
+ "To prevent "
+ "data corruption, disable the data cache "
+ "by mounting with -o direct_io, or as "
+ "directed otherwise by your FUSE server's "
+ "documentation.");
+ }
+ }
+
/* Fix our buffers if the filesize changed without us knowing */
if (vnode_isreg(vp) && attr->size != fvdat->cached_attrs.va_size) {
- (void)fuse_vnode_setsize(vp, attr->size);
+ (void)fuse_vnode_setsize(vp, attr->size, from_server);
fvdat->cached_attrs.va_size = attr->size;
}
@@ -806,7 +852,7 @@
fuse_vnode_clear_attr_cache(dvp);
fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
- feo->attr_valid_nsec, NULL);
+ feo->attr_valid_nsec, NULL, true);
return err;
}
@@ -912,26 +958,8 @@
fao->attr.mtime = old_mtime.tv_sec;
fao->attr.mtimensec = old_mtime.tv_nsec;
}
- if (vnode_isreg(vp) &&
- fvdat->cached_attrs.va_size != VNOVAL &&
- fao->attr.size != fvdat->cached_attrs.va_size) {
- /*
- * The server changed the file's size even though we had it
- * cached! That's a server bug.
- */
- struct mount *mp = vnode_mount(vp);
- struct fuse_data *data = fuse_get_mpdata(mp);
-
- fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT,
- "cache incoherent! "
- "To prevent data corruption, disable the data cache "
- "by mounting with -o direct_io, or as directed "
- "otherwise by your FUSE server's documentation.");
- int iosize = fuse_iosize(vp);
- v_inval_buf_range(vp, 0, INT64_MAX, iosize);
- }
fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
- fao->attr_valid_nsec, vap);
+ fao->attr_valid_nsec, vap, true);
if (vtyp != vnode_vtype(vp)) {
fuse_internal_vnode_disappear(vp);
err = ENOENT;
@@ -1231,7 +1259,7 @@
struct fuse_attr_out *fao = (struct fuse_attr_out*)fdi.answ;
fuse_vnode_undirty_cached_timestamps(vp);
fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
- fao->attr_valid_nsec, NULL);
+ fao->attr_valid_nsec, NULL, false);
}
out:
Index: sys/fs/fuse/fuse_io.c
===================================================================
--- sys/fs/fuse/fuse_io.c
+++ sys/fs/fuse/fuse_io.c
@@ -571,7 +571,7 @@
as_written_offset = uio->uio_offset - diff;
if (as_written_offset - diff > filesize)
- fuse_vnode_setsize(vp, as_written_offset);
+ fuse_vnode_setsize(vp, as_written_offset, false);
if (as_written_offset - diff >= filesize)
fvdat->flag &= ~FN_SIZECHANGE;
@@ -715,7 +715,7 @@
* Extend file _after_ locking buffer so we won't race
* with other readers
*/
- err = fuse_vnode_setsize(vp, uio->uio_offset + n);
+ err = fuse_vnode_setsize(vp, uio->uio_offset + n, false);
filesize = uio->uio_offset + n;
fvdat->flag |= FN_SIZECHANGE;
if (err) {
@@ -1007,13 +1007,18 @@
/*
* Setup for actual write
*/
- error = fuse_vnode_size(vp, &filesize, cred, curthread);
- if (error) {
- bp->b_ioflags |= BIO_ERROR;
- bp->b_error = error;
- bufdone(bp);
- return (error);
- }
+ /*
+ * If the file's size is cached, use that value, even if the
+ * cache is expired. At this point we're already committed to
+ * writing something. If the FUSE server has changed the
+ * file's size behind our back, it's too late for us to do
+ * anything about it. In particular, we can't invalidate any
+ * part of the file's buffers because VOP_STRATEGY is called
+ * with them already locked.
+ */
+ filesize = fvdat->cached_attrs.va_size;
+ /* filesize must've been cached by fuse_vnop_open. */
+ KASSERT(filesize != VNOVAL, ("filesize should've been cached"));
if ((off_t)bp->b_lblkno * biosize + bp->b_dirtyend > filesize)
bp->b_dirtyend = filesize -
Index: sys/fs/fuse/fuse_node.h
===================================================================
--- sys/fs/fuse/fuse_node.h
+++ sys/fs/fuse/fuse_node.h
@@ -201,7 +201,7 @@
int fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid);
-int fuse_vnode_setsize(struct vnode *vp, off_t newsize);
+int fuse_vnode_setsize(struct vnode *vp, off_t newsize, bool from_server);
void fuse_vnode_undirty_cached_timestamps(struct vnode *vp);
Index: sys/fs/fuse/fuse_node.c
===================================================================
--- sys/fs/fuse/fuse_node.c
+++ sys/fs/fuse/fuse_node.c
@@ -389,11 +389,14 @@
}
/*
- * Adjust the vnode's size to a new value, such as that provided by
- * FUSE_GETATTR.
+ * Adjust the vnode's size to a new value.
+ *
+ * If the new value came from the server, such as from a FUSE_GETATTR
+ * operation, set `from_server` true. But if it came from a local operation,
+ * such as write(2) or truncate(2), set `from_server` false.
*/
int
-fuse_vnode_setsize(struct vnode *vp, off_t newsize)
+fuse_vnode_setsize(struct vnode *vp, off_t newsize, bool from_server)
{
struct fuse_vnode_data *fvdat = VTOFUD(vp);
struct vattr *attrs;
@@ -434,6 +437,16 @@
MPASS(bp->b_flags & B_VMIO);
vfs_bio_clrbuf(bp);
bp->b_dirtyend = MIN(bp->b_dirtyend, newsize - lbn * iosize);
+ } else if (from_server && newsize > oldsize && oldsize != VNOVAL) {
+ /*
+ * The FUSE server changed the file size behind our back. We
+ * should invalidate the entire cache.
+ */
+ daddr_t left_lbn, end_lbn;
+
+ left_lbn = oldsize / iosize;
+ end_lbn = howmany(newsize, iosize);
+ v_inval_buf_range(vp, 0, end_lbn, iosize);
}
out:
if (bp)
Index: sys/fs/fuse/fuse_vfsops.c
===================================================================
--- sys/fs/fuse/fuse_vfsops.c
+++ sys/fs/fuse/fuse_vfsops.c
@@ -539,7 +539,6 @@
struct fuse_entry_out *feo;
struct fuse_vnode_data *fvdat;
const char dot[] = ".";
- off_t filesize;
enum vtype vtyp;
int error;
@@ -576,47 +575,10 @@
error = fuse_vnode_get(mp, feo, nodeid, NULL, vpp, NULL, vtyp);
if (error)
goto out;
- filesize = feo->attr.size;
-
- /*
- * In the case where we are looking up a FUSE node represented by an
- * existing cached vnode, and the true size reported by FUSE_LOOKUP
- * doesn't match the vnode's cached size, then any cached writes beyond
- * the file's current size are lost.
- *
- * We can get here:
- * * following attribute cache expiration, or
- * * due a bug in the daemon, or
- */
fvdat = VTOFUD(*vpp);
- if (vnode_isreg(*vpp) &&
- filesize != fvdat->cached_attrs.va_size &&
- fvdat->flag & FN_SIZECHANGE) {
- if (data->cache_mode == fuse_data_cache_mode) {
- const char *msg;
-
- if (fuse_libabi_geq(data, 7, 23)) {
- msg = "writeback cache incoherent!."
- "To prevent data corruption, disable "
- "the writeback cache according to your "
- "FUSE server's documentation.";
- } else {
- msg = "writeback cache incoherent!."
- "To prevent data corruption, disable "
- "the writeback cache by setting "
- "vfs.fusefs.data_cache_mode to 0 or 1.";
- }
- fuse_warn(data, FSESS_WARN_WB_CACHE_INCOHERENT, msg);
- } else {
- /* If we get here, it's likely a fusefs kernel bug */
- printf("%s: WB cache incoherent on %s!\n", __func__,
- vnode_mount(*vpp)->mnt_stat.f_mntonname);
- }
- fvdat->flag &= ~FN_SIZECHANGE;
- }
fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
- feo->attr_valid_nsec, NULL);
+ feo->attr_valid_nsec, NULL, true);
fuse_validity_2_bintime(feo->entry_valid, feo->entry_valid_nsec,
&fvdat->entry_cache_timeout);
out:
Index: sys/fs/fuse/fuse_vnops.c
===================================================================
--- sys/fs/fuse/fuse_vnops.c
+++ sys/fs/fuse/fuse_vnops.c
@@ -516,8 +516,9 @@
struct fuse_bmap_in *fbi;
struct fuse_bmap_out *fbo;
struct fuse_data *data;
+ struct fuse_vnode_data *fvdat = VTOFUD(vp);
uint64_t biosize;
- off_t filesize;
+ off_t fsize;
daddr_t lbn = ap->a_bn;
daddr_t *pbn = ap->a_bnp;
int *runp = ap->a_runp;
@@ -550,10 +551,21 @@
*/
if (runb != NULL)
*runb = MIN(lbn, maxrun);
- if (runp != NULL) {
- error = fuse_vnode_size(vp, &filesize, td->td_ucred, td);
+ if (runp != NULL && maxrun == 0)
+ *runp = 0;
+ else if (runp != NULL) {
+ /*
+ * If the file's size is cached, use that value to calculate
+ * runp, even if the cache is expired. runp is only advisory,
+ * and the risk of getting it wrong is not worth the cost of
+ * another upcall.
+ */
+ if (fvdat->cached_attrs.va_size != VNOVAL)
+ fsize = fvdat->cached_attrs.va_size;
+ else
+ error = fuse_vnode_size(vp, &fsize, td->td_ucred, td);
if (error == 0)
- *runp = MIN(MAX(0, filesize / (off_t)biosize - lbn - 1),
+ *runp = MIN(MAX(0, fsize / (off_t)biosize - lbn - 1),
maxrun);
else
*runp = 0;
@@ -895,7 +907,7 @@
}
ASSERT_VOP_ELOCKED(*vpp, "fuse_vnop_create");
fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
- feo->attr_valid_nsec, NULL);
+ feo->attr_valid_nsec, NULL, true);
fuse_filehandle_init(*vpp, FUFH_RDWR, NULL, td, cred, foo);
fuse_vnode_open(*vpp, foo->open_flags, td);
@@ -1151,7 +1163,7 @@
*/
fuse_vnode_clear_attr_cache(tdvp);
fuse_internal_cache_attrs(vp, &feo->attr, feo->attr_valid,
- feo->attr_valid_nsec, NULL);
+ feo->attr_valid_nsec, NULL, true);
}
out:
fdisp_destroy(&fdi);
@@ -1360,52 +1372,17 @@
*vpp = dvp;
} else {
struct fuse_vnode_data *fvdat;
- struct vattr *vap;
err = fuse_vnode_get(vnode_mount(dvp), feo, nid, dvp,
&vp, cnp, vtyp);
if (err)
goto out;
*vpp = vp;
-
- /*
- * In the case where we are looking up a FUSE node
- * represented by an existing cached vnode, and the
- * true size reported by FUSE_LOOKUP doesn't match
- * the vnode's cached size, then any cached writes
- * beyond the file's current size are lost.
- *
- * We can get here:
- * * following attribute cache expiration, or
- * * due a bug in the daemon, or
- */
fvdat = VTOFUD(vp);
- if (vnode_isreg(vp) &&
- ((filesize != fvdat->cached_attrs.va_size &&
- fvdat->flag & FN_SIZECHANGE) ||
- ((vap = VTOVA(vp)) &&
- filesize != vap->va_size)))
- {
- fvdat->flag &= ~FN_SIZECHANGE;
- /*
- * The server changed the file's size even
- * though we had it cached, or had dirty writes
- * in the WB cache!
- */
- fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT,
- "cache incoherent! "
- "To prevent "
- "data corruption, disable the data cache "
- "by mounting with -o direct_io, or as "
- "directed otherwise by your FUSE server's "
- "documentation.");
- int iosize = fuse_iosize(vp);
- v_inval_buf_range(vp, 0, INT64_MAX, iosize);
- }
MPASS(feo != NULL);
fuse_internal_cache_attrs(*vpp, &feo->attr,
- feo->attr_valid, feo->attr_valid_nsec, NULL);
+ feo->attr_valid, feo->attr_valid_nsec, NULL, true);
fuse_validity_2_bintime(feo->entry_valid,
feo->entry_valid_nsec,
&fvdat->entry_cache_timeout);
Index: tests/sys/fs/fusefs/bmap.cc
===================================================================
--- tests/sys/fs/fusefs/bmap.cc
+++ tests/sys/fs/fusefs/bmap.cc
@@ -75,6 +75,8 @@
}
};
+class BmapEof: public Bmap, public WithParamInterface<int> {};
+
/*
* Test FUSE_BMAP
* XXX The FUSE protocol does not include the runp and runb variables, so those
@@ -165,3 +167,87 @@
leak(fd);
}
+
+/*
+ * VOP_BMAP should not query the server for the file's size, even if its cached
+ * attributes have expired.
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256937
+ */
+TEST_P(BmapEof, eof)
+{
+ /*
+ * Outline:
+ * 1) lookup the file, setting attr_valid=0
+ * 2) Read more than one block, causing the kernel to issue VOP_BMAP to
+ * plan readahead.
+ * 3) Nothing should panic
+ * 4) Repeat the tests, truncating the file after different numbers of
+ * GETATTR operations.
+ */
+ Sequence seq;
+ const off_t filesize = 2 * m_maxbcachebuf;
+ const ino_t ino = 42;
+ mode_t mode = S_IFREG | 0644;
+ void *contents, *buf;
+ int fd;
+ int ngetattrs;
+
+ ngetattrs = GetParam();
+ contents = calloc(1, filesize);
+ FuseTest::expect_lookup(RELPATH, ino, mode, filesize, 1, 0);
+ expect_open(ino, 0, 1);
+ // Depending on ngetattrs, FUSE_READ could be called with either
+ // filesize or filesize / 2 .
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in.header.opcode == FUSE_READ &&
+ in.header.nodeid == ino &&
+ in.body.read.offset == 0 &&
+ ( in.body.read.size == filesize ||
+ in.body.read.size == filesize / 2));
+ }, Eq(true)),
+ _)
+ ).WillOnce(Invoke(ReturnImmediate([=](auto in, auto& out) {
+ size_t osize = in.body.read.size;
+ out.header.len = sizeof(struct fuse_out_header) + osize;
+ bzero(out.body.bytes, osize);
+ })));
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([](auto in) {
+ return (in.header.opcode == FUSE_GETATTR &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).Times(Between(ngetattrs - 1, ngetattrs))
+ .InSequence(seq)
+ .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, attr);
+ out.body.attr.attr_valid = 0;
+ out.body.attr.attr.ino = ino;
+ out.body.attr.attr.mode = S_IFREG | 0644;
+ out.body.attr.attr.size = filesize;
+ })));
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([](auto in) {
+ return (in.header.opcode == FUSE_GETATTR &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).InSequence(seq)
+ .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, attr);
+ out.body.attr.attr_valid = 0;
+ out.body.attr.attr.ino = ino;
+ out.body.attr.attr.mode = S_IFREG | 0644;
+ out.body.attr.attr.size = filesize / 2;
+ })));
+
+ buf = calloc(1, filesize);
+ fd = open(FULLPATH, O_RDWR);
+ ASSERT_LE(0, fd) << strerror(errno);
+ read(fd, buf, filesize);
+}
+
+INSTANTIATE_TEST_CASE_P(BE, BmapEof,
+ Values(1, 2, 3)
+);
Index: tests/sys/fs/fusefs/write.cc
===================================================================
--- tests/sys/fs/fusefs/write.cc
+++ tests/sys/fs/fusefs/write.cc
@@ -208,6 +208,9 @@
}
};
+class WriteEofDuringVnopStrategy: public Write, public WithParamInterface<int>
+{};
+
void sigxfsz_handler(int __unused sig) {
Write::s_sigxfsz = 1;
}
@@ -526,6 +529,84 @@
leak(fd);
}
+/*
+ * VOP_STRATEGY should not query the server for the file's size, even if its
+ * cached attributes have expired.
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256937
+ */
+TEST_P(WriteEofDuringVnopStrategy, eof_during_vop_strategy)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ Sequence seq;
+ const off_t filesize = 2 * m_maxbcachebuf;
+ void *contents;
+ uint64_t ino = 42;
+ uint64_t attr_valid = 0;
+ uint64_t attr_valid_nsec = 0;
+ mode_t mode = S_IFREG | 0644;
+ int fd;
+ int ngetattrs;
+
+ ngetattrs = GetParam();
+ contents = calloc(1, filesize);
+
+ EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+ .WillRepeatedly(Invoke(
+ ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.attr.mode = mode;
+ out.body.entry.nodeid = ino;
+ out.body.entry.attr.nlink = 1;
+ out.body.entry.attr.size = filesize;
+ out.body.entry.attr_valid = attr_valid;
+ out.body.entry.attr_valid_nsec = attr_valid_nsec;
+ })));
+ 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)),
+ _)
+ ).Times(Between(ngetattrs - 1, ngetattrs))
+ .InSequence(seq)
+ .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_valid = attr_valid;
+ out.body.attr.attr_valid_nsec = attr_valid_nsec;
+ out.body.attr.attr.size = filesize;
+ })));
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in.header.opcode == FUSE_GETATTR &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).InSequence(seq)
+ .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_valid = attr_valid;
+ out.body.attr.attr_valid_nsec = attr_valid_nsec;
+ out.body.attr.attr.size = filesize / 2;
+ })));
+ expect_write(ino, 0, filesize / 2, filesize / 2, contents);
+
+ fd = open(FULLPATH, O_RDWR);
+ ASSERT_LE(0, fd) << strerror(errno);
+ ASSERT_EQ(filesize / 2, write(fd, contents, filesize / 2))
+ << strerror(errno);
+
+}
+
+INSTANTIATE_TEST_CASE_P(W, WriteEofDuringVnopStrategy,
+ Values(1, 2, 3)
+);
+
/*
* If the kernel cannot be sure which uid, gid, or pid was responsible for a
* write, then it must set the FUSE_WRITE_CACHE bit

File Metadata

Mime Type
text/plain
Expires
Mon, May 18, 2:06 PM (1 h, 36 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
33251384
Default Alt Text
D32332.id96315.diff (18 KB)

Event Timeline