linux-kernel - Re: [PATCH] ceph: mark lock variable __maybe_unused in ceph_count_file

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] [day] [month] [year] [list]

Date: Tue, 06 Feb 2024 10:08:56 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Christian Brauner <brauner@...nel.org>, Xiubo Li <xiubli@...hat.com>, 
	Ilya Dryomov <idryomov@...il.com>
Cc: ceph-devel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH] ceph: mark lock variable __maybe_unused in
 ceph_count_file_locks

On Tue, 2024-02-06 at 09:52 -0500, Jeff Layton wrote:
> The "lock" variable won't be used if CONFIG_FILE_LOCKING=n. We can't
> remove it altogether though, since we do need it for the
> for_each_file_lock loops. Reduce its scope and mark it __maybe_unused.
> 
> Fixes: 3956f35fbd36 ("ceph: adapt to breakup of struct file_lock")
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202402062210.3YyBVGF1-lkp@intel.com/
> Signed-off-by: Jeff Layton <jlayton@...nel.org>
> ---
> This warning is fallout from the big file_lock re-org, so this should
> probably go in via Christian's tree.
> ---
>  fs/ceph/locks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index ebf4ac0055dd..edfbf94f0d14 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -377,7 +377,6 @@ int ceph_flock(struct file *file, int cmd, struct file_lock *fl)
>  void ceph_count_locks(struct inode *inode, int *fcntl_count, int *flock_count)
>  {
>  	struct ceph_client *cl = ceph_inode_to_client(inode);
> -	struct file_lock *lock;
>  	struct file_lock_context *ctx;
>  
>  	*fcntl_count = 0;
> @@ -385,6 +384,8 @@ void ceph_count_locks(struct inode *inode, int *fcntl_count, int *flock_count)
>  
>  	ctx = locks_inode_context(inode);
>  	if (ctx) {
> +		struct file_lock __maybe_unused *lock;
> +
>  		spin_lock(&ctx->flc_lock);
>  		for_each_file_lock(lock, &ctx->flc_posix)
>  			++(*fcntl_count);
> 
> ---
> base-commit: 77f8316a9199a752ffcd136bd01d0566f54e0ea9
> change-id: 20240206-flsplit-d4f427d9e0ad
> 
> Best regards,

On second thought...the simpler fix would probably be to just eliminate
the version of for_each_file_lock when CONFIG_FILE_LOCKING=n, and
unconditionally define it.

Should we do this instead? It also seems to fix the warning:


diff --git a/include/linux/filelock.h b/include/linux/filelock.h
index 553d65a88048..7d819a760c8f 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -180,9 +180,6 @@ static inline void locks_wake_up(struct file_lock *fl)
        wake_up(&fl->c.flc_wait);
 }
 
-/* for walking lists of file_locks linked by fl_list */
-#define for_each_file_lock(_fl, _head) list_for_each_entry(_fl, _head, c.flc_list)
-
 /* fs/locks.c */
 void locks_free_lock_context(struct inode *inode);
 void locks_free_lock(struct file_lock *fl);
@@ -282,8 +279,6 @@ static inline void locks_wake_up(struct file_lock *fl)
 {
 }
 
-#define for_each_file_lock(_fl, _head) while(false)
-
 static inline void
 locks_free_lock_context(struct inode *inode)
 {
@@ -407,6 +402,9 @@ locks_inode_context(const struct inode *inode)
 
 #endif /* !CONFIG_FILE_LOCKING */
 
+/* for walking lists of file_locks linked by fl_list */
+#define for_each_file_lock(_fl, _head) list_for_each_entry(_fl, _head, c.flc_list)
+
 static inline int locks_lock_file_wait(struct file *filp, struct file_lock *fl)
 {
        return locks_lock_inode_wait(file_inode(filp), fl);