Xiubo Li <xiubli@xxxxxxxxxx> hat am 21.05.2024 15:31 CEST geschrieben:
On 5/21/24 14:13, Xiubo Li wrote:
Hi Thorsten
On 5/17/24 12:47, t.fuchs@xxxxxxxxx wrote:
Hi Xiubo,
Thanks for the response.
These are the steps I used to test the issue on a cephfs mount with the dirstat option enabled:
```
mkdir -p dir1/dir2
touch dir1/dir2/file
cat dir1
```
Output without patch applied:
```
entries: 1
files: 0
subdirs: 1
rentries: 2
rfiles: 0
rsubdirs: 2
rbytes: 0
rctime: 1715919375.649629819
```
Output with patch applied:
```
entries: 1
files: 0
subdirs: 1
rentries: 3
rfiles: 1
rsubdirs: 2
rbytes: 0
rctime: 1715919859.046790099
```
Unfortunately, it doesn't work in case of:
$ mkdir -p dir1/dir2; touch dir1/dir2/file
$ cat dir1
It seems in this case the mds still will send the stale metadatas
back for some reasons.
It's because in MDS it will defer propagating rstat if multiple
mkdir/create requests come and are handled in
'mds_dirstat_min_interval' seconds.
In this case as a workaround we need to wait around 5 seconds, which
is 'mds_tick_interval' and the scatter_tick() is fired.
Maybe this should be fixed in both kclient and mds.
- Xiubo
I improved my test environment and did some more checks. Seems I made a
mistake
in the assumption that `cat` or `getfattr` would return the correct
value immediately after
creating the file; it indeed takes a few seconds to get propagated.
This behavior is well explained by your description.
So the initial patch only fixes the behavior of `cat` to be equivalent
to `getfattr` and return
correct values after a few seconds instead of keeping stale values for a
longer timespan.
It's not immediate.
And also I think you means 's/CEPH_STAT_CAP_XATTR/CEPH_STAT_CAP_RSTAT/' ?
Thanks
- Xiubo
Did you mean CEPH_STAT_RSTAT? I couldn't find the symbol
CEPH_STAT_CAP_RSTAT.
I tested this with `err = ceph_do_getattr(inode, CEPH_STAT_RSTAT,
false);` which works fine.
Unlike CEPH_STAT_CAP_XATTR it does not require the force flag to be set
to true.
- Thorsten
The unpatched code does not show the new file in the recursive
attributes.
Accessing the directory with e.g. `ls dir1` seems to update the
attributes and `cat` will return the correct information
afterwards.
Interestingly the call `mkdir -p dir1/dir2/dir3 && cat dir1` will
not count dir3 initially even with the patch but fixes itself after
a few seconds with the patch.
This behavior is the same for `getfattr` though; hence I did not
investigate more.
Best regards,
Thorsten
Xiubo Li<xiubli@xxxxxxxxxx> hat am 17.05.2024 02:32 CEST geschrieben:
Hi Thorsten,
Thanks for your patch.
BTW, could share the steps to reproduce this issue you are trying to fix ?
Maybe this worth to add a test case in ceph qa suite.
Thanks
- Xiubo
On 5/17/24 01:00, Thorsten Fuchs wrote:
Fixes stale recursive stats (rbytes, rentries, ...) being returned for
a directory after creating/deleting entries in subdirectories.
Now `getfattr` and `cat` return the same values for the attributes.
Signed-off-by: Thorsten Fuchs<t.fuchs@xxxxxxxxx>
---
fs/ceph/dir.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 0e9f56eaba1e..e3cf76660305 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -2116,12 +2116,16 @@ static ssize_t ceph_read_dir(struct file *file, char __user *buf, size_t size,
struct ceph_dir_file_info *dfi = file->private_data;
struct inode *inode = file_inode(file);
struct ceph_inode_info *ci = ceph_inode(inode);
- int left;
+ int left, err;
const int bufsize = 1024;
if (!ceph_test_mount_opt(ceph_sb_to_fs_client(inode->i_sb), DIRSTAT))
return -EISDIR;
+ err = ceph_do_getattr(inode, CEPH_STAT_CAP_XATTR, true);
+ if (err)
+ return err;
+
if (!dfi->dir_info) {
dfi->dir_info = kmalloc(bufsize, GFP_KERNEL);
if (!dfi->dir_info)
