On Sat, Jan 18, 2025 at 2:53 PM Liang Jie <buaajxlj@xxxxxxx> wrote: > > On Wed, 15 Jan 2025 21:15:11 +0100, Ilya Dryomov <idryomov@xxxxxxxxx> wrote: > > On Fri, Jan 10, 2025 at 8:25=E2=80=AFPM Viacheslav Dubeyko > > <Slava.Dubeyko@xxxxxxx> wrote: > > > > > > On Fri, 2025-01-10 at 18:05 +0800, Liang Jie wrote: > > > > From: Liang Jie <liangjie@xxxxxxxxxxx> > > > > > > > > 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@xxxxxxxxxxx> > > > > --- > > > > 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@xxxxxxx> > > > > Applied. > > > > Thanks, > > > > Ilya > > Hi Ilya, > > I have recently received a warning from the kernel test robot about an indentation > issue adjacent to the changes made in my this patch. > > The warning is as follows: > (link: https://lore.kernel.org/oe-kbuild-all/202501172005.IoKVy2Op-lkp@xxxxxxxxx/) > > > New smatch warnings: > > fs/ceph/mds_client.c:3298 __prepare_send_request() warn: inconsistent indenting > > > > vim +3298 fs/ceph/mds_client.c > > > > 2f2dc053404feb Sage Weil 2009-10-06 3272 > > ... > > ce0d5bd3a6c176 Xiubo Li 2023-07-25 3295 if (req->r_attempts) { > > 164b631927701b Liang Jie 2025-01-10 3296 old_max_retry = sizeof_field(struct ceph_mds_request_head, > > ce0d5bd3a6c176 Xiubo Li 2023-07-25 3297 num_retry); > > ce0d5bd3a6c176 Xiubo Li 2023-07-25 @3298 old_max_retry = 1 << (old_max_retry * BITS_PER_BYTE); > > ce0d5bd3a6c176 Xiubo Li 2023-07-25 3299 if ((old_version && req->r_attempts >= old_max_retry) || > > ce0d5bd3a6c176 Xiubo Li 2023-07-25 3300 ((uint32_t)req->r_attempts >= U32_MAX)) { > > 38d46409c4639a Xiubo Li 2023-06-12 3301 pr_warn_ratelimited_client(cl, "request tid %llu seq overflow\n", > > 38d46409c4639a Xiubo Li 2023-06-12 3302 req->r_tid); > > 546a5d6122faae Xiubo Li 2022-03-30 3303 return -EMULTIHOP; > > 546a5d6122faae Xiubo Li 2022-03-30 3304 } > > ce0d5bd3a6c176 Xiubo Li 2023-07-25 3305 } > > The warning indicates suspect code indent on line 3298, existing before my > proposed changes were made. After investigating the issue, it has become clear > that the warning stems from a pre-existing code block that uses 15 spaces for > indentation instead of conforming to the standard 16-space (two tabs) indentation. > > I am writing to seek your advice on how best to proceed. Would you recommend that > I adjust the indentation within the scope of my current patch, or would it be more > appropriate to address this as a separate style fix, given that the indentation > issue is not directly introduced by my changes? Hi Liang, I noticed the indentation mismatch myself and, when applying, edited your patch to be consistent with the pre-existing block (it's actually a tab + 7 spaces, no idea why): https://github.com/ceph/ceph-client/commit/929fba81687e4ba6ed9af18fc1f34e76ac90772a It looks like this warning was generated based on the patch as posted, not as applied, so it can be ignored. > > I am happy to make the necessary adjustments to ensure the code base adheres to the > kernel's coding standards. However, I also want to respect the best practices of > contributing to the project and maintain the clarity and focus of my patch. > > Kindly advise on the preferred way forward. > > Thank you for your time and guidance. Thank you for being thorough! Ilya