diff mbox series

ceph: fix stale xattr when using read() on dir with '-o dirstat'

Message ID 20240516170021.3738-1-t.fuchs@thofu.net (mailing list archive)
State New, archived
Headers show
Series ceph: fix stale xattr when using read() on dir with '-o dirstat' | expand

Commit Message

Thorsten Fuchs May 16, 2024, 5 p.m. UTC
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@thofu.net>
---
 fs/ceph/dir.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Xiubo Li May 17, 2024, 12:32 a.m. UTC | #1
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@thofu.net>
> ---
>   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)
Thorsten Fuchs May 17, 2024, 4:47 a.m. UTC | #2
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
```

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@redhat.com> 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@thofu.net>
> > ---
> >   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)
diff mbox series

Patch

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)