Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Revert "fs/9p: Refresh metadata in d_revalidate for uncached mode too"

This reverts commit 290434474c332a2ba9c8499fe699c7f2e1153280.

That commit broke cache=mmap, a mode that doesn't cache metadata,
but still has writeback cache.

In commit 290434474c33 ("fs/9p: Refresh metadata in d_revalidate
for uncached mode too") we considered metadata cache to be enough to
not look at the server, but in writeback cache too looking at the server
size would make the vfs consider the file has been truncated before the
data has been flushed out, making the following repro fail (nothing is
ever read back, the resulting file ends up with no data written)
```
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

char buf[4096];

int main(int argc, char *argv[])
{
int ret, i;
int fdw, fdr;

if (argc < 2)
return 1;

fdw = openat(AT_FDCWD, argv[1], O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600);
if (fdw < 0) {
fprintf(stderr, "cannot open fdw\n");
return 1;
}
write(fdw, buf, sizeof(buf));

fdr = openat(AT_FDCWD, argv[1], O_RDONLY|O_CLOEXEC);

if (fdr < 0) {
fprintf(stderr, "cannot open fdr\n");
close(fdw);
return 1;
}

for (i = 0; i < 10; i++) {
ret = read(fdr, buf, sizeof(buf));
fprintf(stderr, "i: %d, read returns %d\n", i, ret);
}

close(fdr);
close(fdw);
return 0;
}
```

There is a fix for this particular reproducer but it looks like there
are other problems around metadata refresh (e.g. around file rename), so
revert this to avoid d_revalidate in uncached mode for now.

Reported-by: Song Liu <song@kernel.org>
Link: https://lkml.kernel.org/r/CAHzjS_u_SYdt5=2gYO_dxzMKXzGMt-TfdE_ueowg-Hq5tRCAiw@mail.gmail.com
Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Link: https://lore.kernel.org/bpf/CAEf4BzZbCE4tLoDZyUf_aASpgAGFj75QMfSXX4a4dLYixnOiLg@mail.gmail.com/
Fixes: 290434474c33 ("fs/9p: Refresh metadata in d_revalidate for uncached mode too")
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>

+4 -22
+2 -8
fs/9p/vfs_dentry.c
··· 66 66 struct p9_fid *fid; 67 67 struct inode *inode; 68 68 struct v9fs_inode *v9inode; 69 - unsigned int cached; 70 69 71 70 if (flags & LOOKUP_RCU) 72 71 return -ECHILD; ··· 75 76 goto out_valid; 76 77 77 78 v9inode = V9FS_I(inode); 78 - struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode); 79 - 80 - cached = v9ses->cache & (CACHE_META | CACHE_LOOSE); 81 - 82 - if (!cached || v9inode->cache_validity & V9FS_INO_INVALID_ATTR) { 79 + if (v9inode->cache_validity & V9FS_INO_INVALID_ATTR) { 83 80 int retval; 84 81 struct v9fs_session_info *v9ses; 85 82 ··· 109 114 p9_debug(P9_DEBUG_VFS, 110 115 "refresh inode: dentry = %pd (%p), got error %pe\n", 111 116 dentry, dentry, ERR_PTR(retval)); 117 + if (retval < 0) 112 118 return retval; 113 119 } 114 120 } ··· 146 150 }; 147 151 148 152 const struct dentry_operations v9fs_dentry_operations = { 149 - .d_revalidate = v9fs_lookup_revalidate, 150 - .d_weak_revalidate = __v9fs_lookup_revalidate, 151 153 .d_release = v9fs_dentry_release, 152 154 .d_unalias_trylock = v9fs_dentry_unalias_trylock, 153 155 .d_unalias_unlock = v9fs_dentry_unalias_unlock,
+1 -7
fs/9p/vfs_inode.c
··· 1339 1339 * Don't update inode if the file type is different 1340 1340 */ 1341 1341 umode = p9mode2unixmode(v9ses, st, &rdev); 1342 - if (inode_wrong_type(inode, umode)) { 1343 - /* 1344 - * Do this as a way of letting the caller know the inode should not 1345 - * be reused 1346 - */ 1347 - v9fs_invalidate_inode_attr(inode); 1342 + if (inode_wrong_type(inode, umode)) 1348 1343 goto out; 1349 - } 1350 1344 1351 1345 /* 1352 1346 * We don't want to refresh inode->i_size,
+1 -7
fs/9p/vfs_inode_dotl.c
··· 897 897 /* 898 898 * Don't update inode if the file type is different 899 899 */ 900 - if (inode_wrong_type(inode, st->st_mode)) { 901 - /* 902 - * Do this as a way of letting the caller know the inode should not 903 - * be reused 904 - */ 905 - v9fs_invalidate_inode_attr(inode); 900 + if (inode_wrong_type(inode, st->st_mode)) 906 901 goto out; 907 - } 908 902 909 903 /* 910 904 * We don't want to refresh inode->i_size,