[PATCH] locks: restore a warn for leaked locks on close

[PATCH] locks: restore a warn for leaked locks on close

[Benjamin Coddington]

When locks.c moved to using file_lock_context, the check for any locks that
were not released was moved from the __fput() to destroy_inode() path in
commit 8634b51f6ca2 ("locks: convert lease handling to file_lock_context").
This warning has been quite useful for catching bugs, particularly in NFS
where lock handling still sees some churn.

Let's bring back the warning for leaked locks on __fput, as this warning is
much more likely to be seen and reported by users.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/locks.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/fs/locks.c b/fs/locks.c
index af2031a1fcff..0889c28f9868 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -270,6 +270,22 @@ locks_check_ctx_lists(struct inode *inode)
 	}
 }
 
+static void
+locks_check_ctx_file_list(struct file *filp, struct list_head *list,
+				char *list_type)
+{
+	struct file_lock *fl;
+	struct inode *inode = locks_inode(filp);
+
+	list_for_each_entry(fl, list, fl_list)
+		if (fl->fl_file == filp)
+			pr_warn("Leaked %s lock on dev=0x%x:0x%x ino=0x%lx "
+				" fl_owner=%p fl_flags=0x%x fl_type=0x%x fl_pid=%u\n",
+				list_type, MAJOR(inode->i_sb->s_dev),
+				MINOR(inode->i_sb->s_dev), inode->i_ino,
+				fl->fl_owner, fl->fl_flags, fl->fl_type, fl->fl_pid);
+}
+
 void
 locks_free_lock_context(struct inode *inode)
 {
@@ -2562,6 +2578,12 @@ void locks_remove_file(struct file *filp)
 
 	/* remove any leases */
 	locks_remove_lease(filp, ctx);
+
+	spin_lock(&ctx->flc_lock);
+	locks_check_ctx_file_list(filp, &ctx->flc_posix, "POSIX");
+	locks_check_ctx_file_list(filp, &ctx->flc_flock, "FLOCK");
+	locks_check_ctx_file_list(filp, &ctx->flc_lease, "LEASE");
+	spin_unlock(&ctx->flc_lock);
 }
 
 /**
-- 
2.9.3

Re: [PATCH] locks: restore a warn for leaked locks on close

[Jeff Layton]

On Fri, 2017-07-21 at 13:36 -0400, Benjamin Coddington wrote:
> When locks.c moved to using file_lock_context, the check for any locks that
> were not released was moved from the __fput() to destroy_inode() path in
> commit 8634b51f6ca2 ("locks: convert lease handling to file_lock_context").
> This warning has been quite useful for catching bugs, particularly in NFS
> where lock handling still sees some churn.
> 
> Let's bring back the warning for leaked locks on __fput, as this warning is
> much more likely to be seen and reported by users.
> 
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/locks.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index af2031a1fcff..0889c28f9868 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -270,6 +270,22 @@ locks_check_ctx_lists(struct inode *inode)
>  	}
>  }
>  
> +static void
> +locks_check_ctx_file_list(struct file *filp, struct list_head *list,
> +				char *list_type)
> +{
> +	struct file_lock *fl;
> +	struct inode *inode = locks_inode(filp);
> +
> +	list_for_each_entry(fl, list, fl_list)
> +		if (fl->fl_file == filp)
> +			pr_warn("Leaked %s lock on dev=0x%x:0x%x ino=0x%lx "
> +				" fl_owner=%p fl_flags=0x%x fl_type=0x%x fl_pid=%u\n",
> +				list_type, MAJOR(inode->i_sb->s_dev),
> +				MINOR(inode->i_sb->s_dev), inode->i_ino,
> +				fl->fl_owner, fl->fl_flags, fl->fl_type, fl->fl_pid);
> +}
> +
>  void
>  locks_free_lock_context(struct inode *inode)
>  {
> @@ -2562,6 +2578,12 @@ void locks_remove_file(struct file *filp)
>  
>  	/* remove any leases */
>  	locks_remove_lease(filp, ctx);
> +
> +	spin_lock(&ctx->flc_lock);
> +	locks_check_ctx_file_list(filp, &ctx->flc_posix, "POSIX");
> +	locks_check_ctx_file_list(filp, &ctx->flc_flock, "FLOCK");
> +	locks_check_ctx_file_list(filp, &ctx->flc_lease, "LEASE");
> +	spin_unlock(&ctx->flc_lock);
>  }
>  
>  /**

Thanks, looks good. Merged into my linux-next branch.
-- 
Jeff Layton <jlayton@poochiereds.net>