Re: [PATCH 03/12] include/ceph/ceph_fs.h: convert `pool_id` to u32

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Fri, 2026-06-12 at 18:51 +0200, Max Kellermann wrote:
> While the Ceph OSD protocol transmits OSD pool ids as 64 bit
> integers,
> the MDS protocol is limited to 32 bits.  This is a protocol
> limitation
> we cannot fix, but it gives us the chance to reduce the struct sizes
> by only using the integer size we really need.

We have tendency of growing capacity of file system volumes. I would
prefer to increase this value to u64 instead of decreasing to u32.

> 
> There is one caveat: previously, -1 was used to indicate "invalid
> pool
> id".  This patch changes this magic value to 0 (i.e. removes the
> special-case code from ceph_file_layout_{from,to}_legacy()).  This is
> fine because on the wire, this magic value is 0, too.

Potentially, we can use U64_MAX as invalid pool id.

Thanks,
Slava.

> 
> This reduces the size of `struct ceph_inode_info` by 16 bytes
> (because
> it contains `struct ceph_file_layout` twice, and that struct shrinks
> by 8 bytes; the small pool_id now fits in the existing padding hole).
> 
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
>  fs/ceph/addr.c               | 16 ++++++++--------
>  fs/ceph/caps.c               |  2 +-
>  fs/ceph/inode.c              |  2 +-
>  fs/ceph/mds_client.h         |  2 +-
>  fs/ceph/util.c               |  8 +-------
>  fs/ceph/xattr.c              | 10 +++++-----
>  include/linux/ceph/ceph_fs.h |  2 +-
>  7 files changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 0a86f672cc09..a11b5c633358 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -2381,7 +2381,7 @@ enum {
>  };
>  
>  static int __ceph_pool_perm_get(struct ceph_inode_info *ci,
> -				s64 pool, struct ceph_string
> *pool_ns)
> +				u32 pool, struct ceph_string
> *pool_ns)
>  {
>  	struct ceph_fs_client *fsc = ceph_inode_to_fs_client(&ci-
> >netfs.inode);
>  	struct ceph_mds_client *mdsc = fsc->mdsc;
> @@ -2420,10 +2420,10 @@ static int __ceph_pool_perm_get(struct
> ceph_inode_info *ci,
>  		goto out;
>  
>  	if (pool_ns)
> -		doutc(cl, "pool %lld ns %.*s no perm cached\n",
> pool,
> +		doutc(cl, "pool %u ns %.*s no perm cached\n", pool,
>  		      (int)pool_ns->len, pool_ns->str);
>  	else
> -		doutc(cl, "pool %lld no perm cached\n", pool);
> +		doutc(cl, "pool %u no perm cached\n", pool);
>  
>  	down_write(&mdsc->pool_perm_rwsem);
>  	p = &mdsc->pool_perm_tree.rb_node;
> @@ -2548,10 +2548,10 @@ static int __ceph_pool_perm_get(struct
> ceph_inode_info *ci,
>  	if (!err)
>  		err = have;
>  	if (pool_ns)
> -		doutc(cl, "pool %lld ns %.*s result = %d\n", pool,
> +		doutc(cl, "pool %u ns %.*s result = %d\n", pool,
>  		      (int)pool_ns->len, pool_ns->str, err);
>  	else
> -		doutc(cl, "pool %lld result = %d\n", pool, err);
> +		doutc(cl, "pool %u result = %d\n", pool, err);
>  	return err;
>  }
>  
> @@ -2560,7 +2560,7 @@ int ceph_pool_perm_check(struct inode *inode,
> int need)
>  	struct ceph_client *cl = ceph_inode_to_client(inode);
>  	struct ceph_inode_info *ci = ceph_inode(inode);
>  	struct ceph_string *pool_ns;
> -	s64 pool;
> +	u32 pool;
>  	int ret, flags;
>  
>  	/* Only need to do this for regular files */
> @@ -2587,11 +2587,11 @@ int ceph_pool_perm_check(struct inode *inode,
> int need)
>  check:
>  	if (flags & CEPH_I_POOL_PERM) {
>  		if ((need & CEPH_CAP_FILE_RD) && !(flags &
> CEPH_I_POOL_RD)) {
> -			doutc(cl, "pool %lld no read perm\n", pool);
> +			doutc(cl, "pool %u no read perm\n", pool);
>  			return -EPERM;
>  		}
>  		if ((need & CEPH_CAP_FILE_WR) && !(flags &
> CEPH_I_POOL_WR)) {
> -			doutc(cl, "pool %lld no write perm\n",
> pool);
> +			doutc(cl, "pool %u no write perm\n", pool);
>  			return -EPERM;
>  		}
>  		return 0;
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index d51454e995a8..cf9916608dc2 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3638,7 +3638,7 @@ static void handle_cap_grant(struct inode
> *inode,
>  
>  	if (newcaps & (CEPH_CAP_ANY_FILE_RD | CEPH_CAP_ANY_FILE_WR))
> {
>  		/* file layout may have changed */
> -		s64 old_pool = ci->i_layout.pool_id;
> +		u32 old_pool = ci->i_layout.pool_id;
>  		struct ceph_string *old_ns;
>  
>  		ceph_file_layout_from_legacy(&ci->i_layout, &grant-
> >layout);
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 1504b889622c..c26217ed2034 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1175,7 +1175,7 @@ int ceph_fill_inode(struct inode *inode, struct
> page *locked_page,
>  	if (new_version ||
>  	    (new_issued & (CEPH_CAP_ANY_FILE_RD |
> CEPH_CAP_ANY_FILE_WR))) {
>  		u64 size = le64_to_cpu(info->size);
> -		s64 old_pool = ci->i_layout.pool_id;
> +		u32 old_pool = ci->i_layout.pool_id;
>  		struct ceph_string *old_ns;
>  
>  		ceph_file_layout_from_legacy(&ci->i_layout, &info-
> >layout);
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index 4e6c87f8414c..7b848fdd6f70 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -396,7 +396,7 @@ struct ceph_mds_request {
>  struct ceph_pool_perm {
>  	struct rb_node node;
>  	int perm;
> -	s64 pool;
> +	u32 pool;
>  	size_t pool_ns_len;
>  	char pool_ns[];
>  };
> diff --git a/fs/ceph/util.c b/fs/ceph/util.c
> index 2c34875675bf..5bb40af08b1c 100644
> --- a/fs/ceph/util.c
> +++ b/fs/ceph/util.c
> @@ -35,9 +35,6 @@ void ceph_file_layout_from_legacy(struct
> ceph_file_layout *fl,
>  	fl->stripe_count = le32_to_cpu(legacy->fl_stripe_count);
>  	fl->object_size = le32_to_cpu(legacy->fl_object_size);
>  	fl->pool_id = le32_to_cpu(legacy->fl_pg_pool);
> -	if (fl->pool_id == 0 && fl->stripe_unit == 0 &&
> -	    fl->stripe_count == 0 && fl->object_size == 0)
> -		fl->pool_id = -1;
>  }
>  
>  void ceph_file_layout_to_legacy(struct ceph_file_layout *fl,
> @@ -46,10 +43,7 @@ void ceph_file_layout_to_legacy(struct
> ceph_file_layout *fl,
>  	legacy->fl_stripe_unit = cpu_to_le32(fl->stripe_unit);
>  	legacy->fl_stripe_count = cpu_to_le32(fl->stripe_count);
>  	legacy->fl_object_size = cpu_to_le32(fl->object_size);
> -	if (fl->pool_id >= 0)
> -		legacy->fl_pg_pool = cpu_to_le32(fl->pool_id);
> -	else
> -		legacy->fl_pg_pool = 0;
> +	legacy->fl_pg_pool = cpu_to_le32(fl->pool_id);
>  }
>  
>  int ceph_flags_to_mode(int flags)
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index e773be07f767..9808eb10625d 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -50,7 +50,7 @@ static bool ceph_vxattrcb_layout_exists(struct
> ceph_inode_info *ci)
>  {
>  	struct ceph_file_layout *fl = &ci->i_layout;
>  	return (fl->stripe_unit > 0 || fl->stripe_count > 0 ||
> -		fl->object_size > 0 || fl->pool_id >= 0 ||
> +		fl->object_size > 0 || fl->pool_id > 0 ||
>  		rcu_dereference_raw(fl->pool_ns) != NULL);
>  }
>  
> @@ -61,7 +61,7 @@ static ssize_t ceph_vxattrcb_layout(struct
> ceph_inode_info *ci, char *val,
>  	struct ceph_client *cl = fsc->client;
>  	struct ceph_osd_client *osdc = &fsc->client->osdc;
>  	struct ceph_string *pool_ns;
> -	s64 pool = ci->i_layout.pool_id;
> +	u32 pool = ci->i_layout.pool_id;
>  	const char *pool_name;
>  	const char *ns_field = " pool_namespace=";
>  	char buf[128];
> @@ -81,7 +81,7 @@ static ssize_t ceph_vxattrcb_layout(struct
> ceph_inode_info *ci, char *val,
>  		total_len = len + strlen(pool_name);
>  	} else {
>  		len = snprintf(buf, sizeof(buf),
> -		"stripe_unit=%u stripe_count=%u object_size=%u
> pool=%lld",
> +		"stripe_unit=%u stripe_count=%u object_size=%u
> pool=%u",
>  		ci->i_layout.stripe_unit, ci->i_layout.stripe_count,
>  		ci->i_layout.object_size, pool);
>  		total_len = len;
> @@ -164,7 +164,7 @@ static ssize_t ceph_vxattrcb_layout_pool(struct
> ceph_inode_info *ci,
>  	ssize_t ret;
>  	struct ceph_fs_client *fsc = ceph_sb_to_fs_client(ci-
> >netfs.inode.i_sb);
>  	struct ceph_osd_client *osdc = &fsc->client->osdc;
> -	s64 pool = ci->i_layout.pool_id;
> +	u32 pool = ci->i_layout.pool_id;
>  	const char *pool_name;
>  
>  	down_read(&osdc->lock);
> @@ -174,7 +174,7 @@ static ssize_t ceph_vxattrcb_layout_pool(struct
> ceph_inode_info *ci,
>  		if (ret <= size)
>  			memcpy(val, pool_name, ret);
>  	} else {
> -		ret = ceph_fmt_xattr(val, size, "%lld", pool);
> +		ret = ceph_fmt_xattr(val, size, "%u", pool);
>  	}
>  	up_read(&osdc->lock);
>  	return ret;
> diff --git a/include/linux/ceph/ceph_fs.h
> b/include/linux/ceph/ceph_fs.h
> index 69ac3e55a3fe..7faa18493a07 100644
> --- a/include/linux/ceph/ceph_fs.h
> +++ b/include/linux/ceph/ceph_fs.h
> @@ -69,7 +69,7 @@ struct ceph_file_layout {
>  	u32 stripe_unit;   /* stripe unit, in bytes */
>  	u32 stripe_count;  /* over this many objects */
>  	u32 object_size;   /* until objects are this big */
> -	s64 pool_id;        /* rados pool id */
> +	u32 pool_id;        /* rados pool id */
>  	struct ceph_string __rcu *pool_ns; /* rados pool namespace
> */
>  };
>