linux-kernel - Re: [PATCH] ceph: streamline request head structures in MDS client

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250118135243.3560118-1-buaajxlj@163.com>
Date: Sat, 18 Jan 2025 21:52:43 +0800
From: Liang Jie <buaajxlj@....com>
To: idryomov@...il.com
Cc: Slava.Dubeyko@....com,
	buaajxlj@....com,
	ceph-devel@...r.kernel.org,
	fanggeng@...iang.com,
	liangjie@...iang.com,
	linux-kernel@...r.kernel.org,
	xiubli@...hat.com,
	yangchen11@...iang.com
Subject: Re: [PATCH] ceph: streamline request head structures in MDS client

On Wed, 15 Jan 2025 21:15:11 +0100, Ilya Dryomov <idryomov@...il.com> wrote:
> On Fri, Jan 10, 2025 at 8:25=E2=80=AFPM Viacheslav Dubeyko
> <Slava.Dubeyko@....com> wrote:
> >
> > 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>
> 
> 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@intel.com/)

> 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?

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.

Best regards,
Liang Jie