Re: [PATCH v2] ceph: properly handle statfs on multifs setups

[Xiubo Li <xiubli@redhat.com>]

On 11/3/21 4:45 AM, Jeff Layton wrote:
> ceph_statfs currently stuffs the cluster fsid into the f_fsid field.
> This was fine when we only had a single filesystem per cluster, but now
> that we have multiples we need to use something that will vary between
> them.
>
> Change ceph_statfs to xor each 32-bit chunk of the fsid (aka cluster id)
> into the lower bits of the statfs->f_fsid. Change the lower bits to hold
> the fscid (filesystem ID within the cluster).
>
> That should give us a value that is guaranteed to be unique between
> filesystems within a cluster, and should minimize the chance of
> collisions between mounts of different clusters.
>
> URL: https://tracker.ceph.com/issues/52812
> Reported-by: Sachin Prabhu <sprabhu@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/ceph/super.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> While looking at making an equivalent change to the userland libraries,
> it occurred to me that the earlier patch's method for computing this
> was overly-complex. This makes it a bit simpler, and avoids the
> intermediate step of setting up a u64.
>
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index 9bb88423417e..e7b839aa08f6 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -52,8 +52,7 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	struct ceph_fs_client *fsc = ceph_inode_to_client(d_inode(dentry));
>   	struct ceph_mon_client *monc = &fsc->client->monc;
>   	struct ceph_statfs st;
> -	u64 fsid;
> -	int err;
> +	int i, err;
>   	u64 data_pool;
>   
>   	if (fsc->mdsc->mdsmap->m_num_data_pg_pools == 1) {
> @@ -99,12 +98,14 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	buf->f_namelen = NAME_MAX;
>   
>   	/* Must convert the fsid, for consistent values across arches */
> +	buf->f_fsid.val[0] = 0;
>   	mutex_lock(&monc->mutex);
> -	fsid = le64_to_cpu(*(__le64 *)(&monc->monmap->fsid)) ^
> -	       le64_to_cpu(*((__le64 *)&monc->monmap->fsid + 1));
> +	for (i = 0; i < 4; ++i)
> +		buf->f_fsid.val[0] ^= le32_to_cpu(((__le32 *)&monc->monmap->fsid)[i]);
>   	mutex_unlock(&monc->mutex);
>   
> -	buf->f_fsid = u64_to_fsid(fsid);
> +	/* fold the fs_cluster_id into the upper bits */
> +	buf->f_fsid.val[1] = monc->fs_cluster_id;
>   
>   	return 0;
>   }

This version looks better.

Reviewed-by: Xiubo Li <xiubli@redhat.com>