On Wed, 2021-11-03 at 14:56 +0800, Xiubo Li wrote: > 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@xxxxxxxxxx> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxx> > > Thanks. I think I'm going to make one more small change in there and express the loop conditional as: i < sizeof(monc->monmap->fsid) / sizeof(__le32) That should work out to be '4', but should be safer in case the size of fsid ever changes. I'm not going to bother to re-post for that though. -- Jeff Layton <jlayton@xxxxxxxxxx>
