diff mbox series

[v2] ceph: fix incorrectly showing the .snap size for stat

Message ID 20220831080257.170065-1-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v2] ceph: fix incorrectly showing the .snap size for stat | expand

Commit Message

Xiubo Li Aug. 31, 2022, 8:02 a.m. UTC 
From: Xiubo Li <xiubli@redhat.com>

We should set the 'stat->size' to the real number of snapshots for
snapdirs.

URL: https://tracker.ceph.com/issues/57342
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/inode.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 66 insertions(+), 2 deletions(-)

Comments

Luis Henriques Sept. 5, 2022, 10:52 a.m. UTC | #1 
On Wed, Aug 31, 2022 at 04:02:57PM +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> We should set the 'stat->size' to the real number of snapshots for
> snapdirs.
> 
> URL: https://tracker.ceph.com/issues/57342
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/inode.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 4db4394912e7..99022bcdde64 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -2705,6 +2705,52 @@ static int statx_to_caps(u32 want, umode_t mode)
>  	return mask;
>  }
>  
> +static struct inode *ceph_get_snap_parent(struct inode *inode)
> +{
> +	struct super_block *sb = inode->i_sb;
> +	struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(sb);
> +	struct ceph_mds_request *req;
> +	struct ceph_vino vino = {
> +		.ino = ceph_ino(inode),
> +		.snap = CEPH_NOSNAP,
> +	};
> +	struct inode *parent;
> +	int mask;
> +	int err;
> +
> +	if (ceph_vino_is_reserved(vino))
> +		return ERR_PTR(-ESTALE);
> +
> +	parent = ceph_find_inode(sb, vino);
> +	if (likely(parent)) {
> +		if (ceph_inode_is_shutdown(parent)) {
> +			iput(parent);
> +			return ERR_PTR(-ESTALE);
> +		}
> +		return parent;
> +	}
> +
> +	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LOOKUPINO,
> +				       USE_ANY_MDS);
> +	if (IS_ERR(req))
> +		return ERR_CAST(req);
> +
> +	mask = CEPH_STAT_CAP_INODE;
> +	if (ceph_security_xattr_wanted(d_inode(sb->s_root)))
> +		mask |= CEPH_CAP_XATTR_SHARED;
> +	req->r_args.lookupino.mask = cpu_to_le32(mask);
> +	req->r_ino1 = vino;
> +	req->r_num_caps = 1;
> +	err = ceph_mdsc_do_request(mdsc, NULL, req);
> +	if (err < 0)
> +		return ERR_PTR(err);
> +	parent = req->r_target_inode;
> +	if (!parent)
> +		return ERR_PTR(-ESTALE);
> +	ihold(parent);
> +	return parent;
> +}

Can't we simply re-use __lookup_inode() instead of duplicating all this
code?

(Also: is there a similar fix for the fuse client?)

Cheers,
--
Luís

> +
>  /*
>   * Get all the attributes. If we have sufficient caps for the requested attrs,
>   * then we can avoid talking to the MDS at all.
> @@ -2748,10 +2794,28 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
>  
>  	if (S_ISDIR(inode->i_mode)) {
>  		if (ceph_test_mount_opt(ceph_sb_to_client(inode->i_sb),
> -					RBYTES))
> +					RBYTES)) {
>  			stat->size = ci->i_rbytes;
> -		else
> +		} else if (ceph_snap(inode) == CEPH_SNAPDIR) {
> +			struct inode *parent = ceph_get_snap_parent(inode);
> +			struct ceph_inode_info *pci;
> +			struct ceph_snap_realm *realm;
> +
> +			if (!parent)
> +				return PTR_ERR(parent);
> +
> +			pci = ceph_inode(parent);
> +			spin_lock(&pci->i_ceph_lock);
> +			realm = pci->i_snap_realm;
> +			if (realm)
> +				stat->size = realm->num_snaps;
> +			else
> +				stat->size = 0;
> +			spin_unlock(&pci->i_ceph_lock);
> +			iput(parent);
> +		} else {
>  			stat->size = ci->i_files + ci->i_subdirs;
> +		}
>  		stat->blocks = 0;
>  		stat->blksize = 65536;
>  		/*
> -- 
> 2.36.0.rc1
>
Xiubo Li Sept. 5, 2022, 12:45 p.m. UTC | #2 
On 05/09/2022 18:52, Luís Henriques wrote:
> On Wed, Aug 31, 2022 at 04:02:57PM +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> We should set the 'stat->size' to the real number of snapshots for
>> snapdirs.
>>
>> URL: https://tracker.ceph.com/issues/57342
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>   fs/ceph/inode.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 66 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
>> index 4db4394912e7..99022bcdde64 100644
>> --- a/fs/ceph/inode.c
>> +++ b/fs/ceph/inode.c
>> @@ -2705,6 +2705,52 @@ static int statx_to_caps(u32 want, umode_t mode)
>>   	return mask;
>>   }
>>   
>> +static struct inode *ceph_get_snap_parent(struct inode *inode)
>> +{
>> +	struct super_block *sb = inode->i_sb;
>> +	struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(sb);
>> +	struct ceph_mds_request *req;
>> +	struct ceph_vino vino = {
>> +		.ino = ceph_ino(inode),
>> +		.snap = CEPH_NOSNAP,
>> +	};
>> +	struct inode *parent;
>> +	int mask;
>> +	int err;
>> +
>> +	if (ceph_vino_is_reserved(vino))
>> +		return ERR_PTR(-ESTALE);
>> +
>> +	parent = ceph_find_inode(sb, vino);
>> +	if (likely(parent)) {
>> +		if (ceph_inode_is_shutdown(parent)) {
>> +			iput(parent);
>> +			return ERR_PTR(-ESTALE);
>> +		}
>> +		return parent;
>> +	}
>> +
>> +	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LOOKUPINO,
>> +				       USE_ANY_MDS);
>> +	if (IS_ERR(req))
>> +		return ERR_CAST(req);
>> +
>> +	mask = CEPH_STAT_CAP_INODE;
>> +	if (ceph_security_xattr_wanted(d_inode(sb->s_root)))
>> +		mask |= CEPH_CAP_XATTR_SHARED;
>> +	req->r_args.lookupino.mask = cpu_to_le32(mask);
>> +	req->r_ino1 = vino;
>> +	req->r_num_caps = 1;
>> +	err = ceph_mdsc_do_request(mdsc, NULL, req);
>> +	if (err < 0)
>> +		return ERR_PTR(err);
>> +	parent = req->r_target_inode;
>> +	if (!parent)
>> +		return ERR_PTR(-ESTALE);
>> +	ihold(parent);
>> +	return parent;
>> +}
> Can't we simply re-use __lookup_inode() instead of duplicating all this
> code?

It seems working.

Let me check more about that.

> (Also: is there a similar fix for the fuse client?)

Yeah, I have raised one PR for that already.

Thanks Luis!


> Cheers,
> --
> Luís
>
>> +
>>   /*
>>    * Get all the attributes. If we have sufficient caps for the requested attrs,
>>    * then we can avoid talking to the MDS at all.
>> @@ -2748,10 +2794,28 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
>>   
>>   	if (S_ISDIR(inode->i_mode)) {
>>   		if (ceph_test_mount_opt(ceph_sb_to_client(inode->i_sb),
>> -					RBYTES))
>> +					RBYTES)) {
>>   			stat->size = ci->i_rbytes;
>> -		else
>> +		} else if (ceph_snap(inode) == CEPH_SNAPDIR) {
>> +			struct inode *parent = ceph_get_snap_parent(inode);
>> +			struct ceph_inode_info *pci;
>> +			struct ceph_snap_realm *realm;
>> +
>> +			if (!parent)
>> +				return PTR_ERR(parent);
>> +
>> +			pci = ceph_inode(parent);
>> +			spin_lock(&pci->i_ceph_lock);
>> +			realm = pci->i_snap_realm;
>> +			if (realm)
>> +				stat->size = realm->num_snaps;
>> +			else
>> +				stat->size = 0;
>> +			spin_unlock(&pci->i_ceph_lock);
>> +			iput(parent);
>> +		} else {
>>   			stat->size = ci->i_files + ci->i_subdirs;
>> +		}
>>   		stat->blocks = 0;
>>   		stat->blksize = 65536;
>>   		/*
>> -- 
>> 2.36.0.rc1
>>
diff mbox series

Patch

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 4db4394912e7..99022bcdde64 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2705,6 +2705,52 @@  static int statx_to_caps(u32 want, umode_t mode)
 	return mask;
 }
 
+static struct inode *ceph_get_snap_parent(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(sb);
+	struct ceph_mds_request *req;
+	struct ceph_vino vino = {
+		.ino = ceph_ino(inode),
+		.snap = CEPH_NOSNAP,
+	};
+	struct inode *parent;
+	int mask;
+	int err;
+
+	if (ceph_vino_is_reserved(vino))
+		return ERR_PTR(-ESTALE);
+
+	parent = ceph_find_inode(sb, vino);
+	if (likely(parent)) {
+		if (ceph_inode_is_shutdown(parent)) {
+			iput(parent);
+			return ERR_PTR(-ESTALE);
+		}
+		return parent;
+	}
+
+	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LOOKUPINO,
+				       USE_ANY_MDS);
+	if (IS_ERR(req))
+		return ERR_CAST(req);
+
+	mask = CEPH_STAT_CAP_INODE;
+	if (ceph_security_xattr_wanted(d_inode(sb->s_root)))
+		mask |= CEPH_CAP_XATTR_SHARED;
+	req->r_args.lookupino.mask = cpu_to_le32(mask);
+	req->r_ino1 = vino;
+	req->r_num_caps = 1;
+	err = ceph_mdsc_do_request(mdsc, NULL, req);
+	if (err < 0)
+		return ERR_PTR(err);
+	parent = req->r_target_inode;
+	if (!parent)
+		return ERR_PTR(-ESTALE);
+	ihold(parent);
+	return parent;
+}
+
 /*
  * Get all the attributes. If we have sufficient caps for the requested attrs,
  * then we can avoid talking to the MDS at all.
@@ -2748,10 +2794,28 @@  int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
 
 	if (S_ISDIR(inode->i_mode)) {
 		if (ceph_test_mount_opt(ceph_sb_to_client(inode->i_sb),
-					RBYTES))
+					RBYTES)) {
 			stat->size = ci->i_rbytes;
-		else
+		} else if (ceph_snap(inode) == CEPH_SNAPDIR) {
+			struct inode *parent = ceph_get_snap_parent(inode);
+			struct ceph_inode_info *pci;
+			struct ceph_snap_realm *realm;
+
+			if (!parent)
+				return PTR_ERR(parent);
+
+			pci = ceph_inode(parent);
+			spin_lock(&pci->i_ceph_lock);
+			realm = pci->i_snap_realm;
+			if (realm)
+				stat->size = realm->num_snaps;
+			else
+				stat->size = 0;
+			spin_unlock(&pci->i_ceph_lock);
+			iput(parent);
+		} else {
 			stat->size = ci->i_files + ci->i_subdirs;
+		}
 		stat->blocks = 0;
 		stat->blksize = 65536;
 		/*