fusefs: Fix a bug during VOP_STRATEGY when the server changes file size
If the FUSE server tells the kernel that a file's size has changed, then the kernel must invalidate any portion of that file in cache. But the kernel can't do that during VOP_STRATEGY, because the file's buffers are already locked. Instead, proceed with the write. PR: 256937 Reported by: Agata <chogata@moosefs.pro> Tested by: Agata <chogata@moosefs.pro> MFC after: 2 weeks Reviewed by: pfg Differential Revision: https://reviews.freebsd.org/D32332
This commit is contained in:
parent
7430017b99
commit
032a5bd55b
@ -1007,13 +1007,18 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
|
||||
/*
|
||||
* 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 -
|
||||
|
@ -208,6 +208,9 @@ virtual void SetUp() {
|
||||
}
|
||||
};
|
||||
|
||||
class WriteEofDuringVnopStrategy: public Write, public WithParamInterface<int>
|
||||
{};
|
||||
|
||||
void sigxfsz_handler(int __unused sig) {
|
||||
Write::s_sigxfsz = 1;
|
||||
}
|
||||
@ -526,6 +529,84 @@ TEST_F(Write, eof_during_rmw)
|
||||
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
|
||||
|
Loading…
x
Reference in New Issue
Block a user