[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c5276b40b56a092ea1ec8d161eaa42926018bf5f.camel@ibm.com>
Date: Fri, 10 Jan 2025 19:25:35 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: Xiubo Li <xiubli@...hat.com>, "buaajxlj@....com" <buaajxlj@....com>
CC: "fanggeng@...iang.com" <fanggeng@...iang.com>,
"yangchen11@...iang.com"
<yangchen11@...iang.com>,
"idryomov@...il.com" <idryomov@...il.com>,
"ceph-devel@...r.kernel.org" <ceph-devel@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"liangjie@...iang.com" <liangjie@...iang.com>
Subject: Re: [PATCH] ceph: streamline request head structures in MDS client
On Fri, 2025-01-10 at 18:05 +0800, Liang Jie wrote:
> From: Liang Jie <liangjie@...iang.com>
>
> The existence of the ceph_mds_request_head_old structure in the MDS
> client
> code is no longer required due to improvements in handling different
> MDS
> request header versions. This patch removes the now redundant
> ceph_mds_request_head_old structure and replaces its usage with the
> flexible and extensible ceph_mds_request_head structure.
>
> Changes include:
> - Modification of find_legacy_request_head to directly cast the
> pointer to
> ceph_mds_request_head_legacy without going through the old
> structure.
> - Update sizeof calculations in create_request_message to use
> offsetofend
> for consistency and future-proofing, rather than referencing the
> old
> structure.
> - Use of the structured ceph_mds_request_head directly instead of the
> old
> one.
>
> Additionally, this consolidation normalizes the handling of
> request_head_version v1 to align with versions v2 and v3, leading to
> a
> more consistent and maintainable codebase.
>
> These changes simplify the codebase and reduce potential confusion
> stemming
> from the existence of an obsolete structure.
>
> Signed-off-by: Liang Jie <liangjie@...iang.com>
> ---
> fs/ceph/mds_client.c | 16 ++++++++--------
> include/linux/ceph/ceph_fs.h | 14 --------------
> 2 files changed, 8 insertions(+), 22 deletions(-)
>
Looks good to me. Nice cleanup.
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@....com>
Thanks,
Slava.
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 785fe489ef4b..2196e404318c 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2945,12 +2945,12 @@ static struct ceph_mds_request_head_legacy *
> find_legacy_request_head(void *p, u64 features)
> {
> bool legacy = !(features & CEPH_FEATURE_FS_BTIME);
> - struct ceph_mds_request_head_old *ohead;
> + struct ceph_mds_request_head *head;
>
> if (legacy)
> return (struct ceph_mds_request_head_legacy *)p;
> - ohead = (struct ceph_mds_request_head_old *)p;
> - return (struct ceph_mds_request_head_legacy *)&ohead-
> >oldest_client_tid;
> + head = (struct ceph_mds_request_head *)p;
> + return (struct ceph_mds_request_head_legacy *)&head-
> >oldest_client_tid;
> }
>
> /*
> @@ -3020,7 +3020,7 @@ static struct ceph_msg
> *create_request_message(struct ceph_mds_session *session,
> if (legacy)
> len = sizeof(struct ceph_mds_request_head_legacy);
> else if (request_head_version == 1)
> - len = sizeof(struct ceph_mds_request_head_old);
> + len = offsetofend(struct ceph_mds_request_head,
> args);
> else if (request_head_version == 2)
> len = offsetofend(struct ceph_mds_request_head,
> ext_num_fwd);
> else
> @@ -3104,11 +3104,11 @@ static struct ceph_msg
> *create_request_message(struct ceph_mds_session *session,
> msg->hdr.version = cpu_to_le16(3);
> p = msg->front.iov_base + sizeof(*lhead);
> } else if (request_head_version == 1) {
> - struct ceph_mds_request_head_old *ohead = msg-
> >front.iov_base;
> + struct ceph_mds_request_head *nhead = msg-
> >front.iov_base;
>
> msg->hdr.version = cpu_to_le16(4);
> - ohead->version = cpu_to_le16(1);
> - p = msg->front.iov_base + sizeof(*ohead);
> + nhead->version = cpu_to_le16(1);
> + p = msg->front.iov_base + offsetofend(struct
> ceph_mds_request_head, args);
> } else if (request_head_version == 2) {
> struct ceph_mds_request_head *nhead = msg-
> >front.iov_base;
>
> @@ -3265,7 +3265,7 @@ static int __prepare_send_request(struct
> ceph_mds_session *session,
> * so we limit to retry at most 256 times.
> */
> if (req->r_attempts) {
> - old_max_retry = sizeof_field(struct
> ceph_mds_request_head_old,
> + old_max_retry = sizeof_field(struct
> ceph_mds_request_head,
> num_retry);
> old_max_retry = 1 << (old_max_retry * BITS_PER_BYTE);
> if ((old_version && req->r_attempts >= old_max_retry)
> ||
> diff --git a/include/linux/ceph/ceph_fs.h
> b/include/linux/ceph/ceph_fs.h
> index 2d7d86f0290d..c7f2c63b3bc3 100644
> --- a/include/linux/ceph/ceph_fs.h
> +++ b/include/linux/ceph/ceph_fs.h
> @@ -504,20 +504,6 @@ struct ceph_mds_request_head_legacy {
>
> #define CEPH_MDS_REQUEST_HEAD_VERSION 3
>
> -struct ceph_mds_request_head_old {
> - __le16 version; /* struct version */
> - __le64 oldest_client_tid;
> - __le32 mdsmap_epoch; /* on client */
> - __le32 flags; /* CEPH_MDS_FLAG_* */
> - __u8 num_retry, num_fwd; /* count retry, fwd attempts
> */
> - __le16 num_releases; /* # include cap/lease
> release records */
> - __le32 op; /* mds op code */
> - __le32 caller_uid, caller_gid;
> - __le64 ino; /* use this ino for openc,
> mkdir, mknod,
> - etc. (if replaying) */
> - union ceph_mds_request_args_ext args;
> -} __attribute__ ((packed));
> -
> struct ceph_mds_request_head {
> __le16 version; /* struct version */
> __le64 oldest_client_tid;
Powered by blists - more mailing lists