fuse: Do not unconditionally overwrite local vnode size with remote

[emil_etsalapatis.com]

Summary

The code in fuse_internal_cache_attrs() assumes that the size reported
by a FUSE_GETATTR request is always canonical. That is not accurate -
it is possible the file has had its FN_SIZECHANGE set during the
getattr call due to a concurrent write. However, the code currently
still considers the server's response canonical and overwrite the size
of the file with the one reported by the server, most likely erasing
data due to file truncation. Adjust the code to consider whether the
file has its FN_SIZECHANGE flag set, and if so use the local cached
size instead.

[asomers]

Interesting find. I'll review this next week, after I'm home from vacation.

[emil_etsalapatis.com]

Thank you! For context, this diff along with the elock issue we discussed on D55046 are the main blockers for getting virtiofs working (D46295 + D46296). This issue from this diff specifically causes constant truncations when using metadata caching from the virtiofs (and assume FUSE) server. This looks like corruption from a user's point of view. I actually think that it'd be better if we removed the warnings altogether because they trigger spuriously and so are not helpful as diagnostics.

[asomers]

This looks like a real problem. But how did you find it? Was it just virtio-fs, or did you find it with a regular fuse file system too? If either case, do you know if FUSE_ASYNC_READ was set? And do you know what values are typically used for the entry and attribute cache?

[asomers]

The odd thing here is that VOP_WRITE should always be called with the vnode exclusively locked. So it shouldn't be possible for it to race with a separate operation, like VOP_READ, that might call fuse_internal_cache_attrs . Do you have a stack trace for either thread involved with this race?

[emil_etsalapatis.com]

In D55047#1263024, @asomers wrote:

This looks like a real problem. But how did you find it? Was it just virtio-fs, or did you find it with a regular fuse file system too? If either case, do you know if FUSE_ASYNC_READ was set? And do you know what values are typically used for the entry and attribute cache?

This was with virtiofs, yes. The feature that seems to trigger this is metadata caching: I can reliably get it to happen when I turn it on. The default values are 86400 for those, but I don't think the number matters, the race seems pretty fast (see below).

I can reliably trigger this by cloning a repo into the mount (Redis , but any repo will probably do it). What seems to happen is that a getattr() call returning from the host is stale and causes fuse_vnode_setsize to truncate writes that happened in blocks past the reported value. I don't turn on FSESS_ASYNC_READ for virtiofs, so it's not used.

In D55047#1263025, @asomers wrote:

The odd thing here is that VOP_WRITE should always be called with the vnode exclusively locked. So it shouldn't be possible for it to race with a separate operation, like VOP_READ, that might call fuse_internal_cache_attrs . Do you have a stack trace for either thread involved with this race?

I can try getting one, I'll post it here. I can also get a trace of the FUSE tickets from the server. Anecdotally, when the bug triggered for me I would always see a getattr with the stale size from the server side.