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

From: Viacheslav Dubeyko

Date: Tue Jun 30 2026 - 12:13:55 EST

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@xxxxxxxxx>
> ---
> 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
> */
> };
>