On Tue, Apr 6, 2021 at 4:07 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > In fuse when a direct/write-through write happens we invalidate attrs because > that might have updated mtime/ctime on server and cached mtime/ctime > will be stale. > > What about page writeback path. Looks like we don't invalidate attrs there. > To be consistent, invalidate attrs in writeback path as well. Only exception > is when writeback_cache is enabled. In that case we strust local mtime/ctime > and there is no need to invalidate attrs. > > Recently users started experiencing failure of xfstests generic/080, > geneirc/215 and generic/614 on virtiofs. This happened only newer > "stat" utility and not older one. This patch fixes the issue. > > So what's the root cause of the issue. Here is detailed explanation. > > generic/080 test does mmap write to a file, closes the file and then > checks if mtime has been updated or not. When file is closed, it > leads to flushing of dirty pages (and that should update mtime/ctime > on server). But we did not explicitly invalidate attrs after writeback > finished. Still generic/080 passed so far and reason being that we > invalidated atime in fuse_readpages_end(). This is called in fuse_readahead() > path and always seems to trigger before mmaped write. > > So after mmaped write when lstat() is called, it sees that atleast one > of the fields being asked for is invalid (atime) and that results in > generating GETATTR to server and mtime/ctime also get updated and test > passes. > > But newer /usr/bin/stat seems to have moved to using statx() syscall now > (instead of using lstat()). And statx() allows it to query only ctime > or mtime (and not rest of the basic stat fields). That means when > querying for mtime, fuse_update_get_attr() sees that mtime is not > invalid (only atime is invalid). So it does not generate a new GETATTR > and fill stat with cached mtime/ctime. And that means updated mtime > is not seen by xfstest and tests start failing. > > Invalidating attrs after writeback completion should solve this problem > in a generic manner. > > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx> Thanks, applied. Miklos
