[PATCH] proc: show locks in /proc/pid/fdinfo/X

[PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrey Vagin]

Let's show locks which are associated with a file descriptor in
its fdinfo file.

Currently we don't have a reliable way to determine who holds a lock.
We can find some information in /proc/locks, but PID which is reported
there can be wrong. For example, a process takes a lock, then forks a
child and dies. In this case /proc/locks contains the parent pid, which
can be reused by another process.

$ cat /proc/locks
...
6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
...

$ ps -C rpcbind
  PID TTY          TIME CMD
  332 ?        00:00:00 rpcbind

$ cat /proc/332/fdinfo/4
pos:	0
flags:	0100000
mnt_id:	22
lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF

$ ls -l /proc/332/fd/4
lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock

$ ls -l /proc/324/fd/
total 0
lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0

You can see that the process with the 324 pid doesn't hold the lock.

This information is required for proper dumping and restoring file
locks.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 Documentation/filesystems/proc.txt |  4 ++++
 fs/locks.c                         | 38 ++++++++++++++++++++++++++++++++++++++
 fs/proc/fd.c                       | 27 ++++++++++++++++++---------
 include/linux/fs.h                 |  7 +++++++
 4 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index a07ba61..6331623 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -1704,6 +1704,10 @@ A typical output is
 	flags:	0100002
 	mnt_id:	19
 
+All locks associated with a file descriptor are shown in its fdinfo too.
+
+lock:       1: FLOCK  ADVISORY  WRITE 359 00:13:11691 0 EOF
+
 The files such as eventfd, fsnotify, signalfd, epoll among the regular pos/flags
 pair provide additional information particular to the objects they represent.
 
diff --git a/fs/locks.c b/fs/locks.c
index 365c82e..815f832 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2592,6 +2592,44 @@ static int locks_show(struct seq_file *f, void *v)
 	return 0;
 }
 
+static void __show_fd_locks(struct seq_file *f,
+			struct list_head *head, int *id,
+			struct file *filp, struct files_struct *files)
+{
+	struct file_lock *fl;
+
+	list_for_each_entry(fl, head, fl_list) {
+
+		if (filp != fl->fl_file)
+			continue;
+		if (fl->fl_owner != files &&
+		    fl->fl_owner != filp)
+			continue;
+
+		(*id)++;
+		seq_puts(f, "lock:\t");
+		lock_get_status(f, fl, *id, "");
+	}
+}
+
+void show_fd_locks(struct seq_file *f,
+		  struct file *filp, struct files_struct *files)
+{
+	struct inode *inode = file_inode(filp);
+	struct file_lock_context *ctx;
+	int id = 0;
+
+	ctx = inode->i_flctx;
+	if (!ctx)
+		return;
+
+	spin_lock(&ctx->flc_lock);
+	__show_fd_locks(f, &ctx->flc_flock, &id, filp, files);
+	__show_fd_locks(f, &ctx->flc_posix, &id, filp, files);
+	__show_fd_locks(f, &ctx->flc_lease, &id, filp, files);
+	spin_unlock(&ctx->flc_lock);
+}
+
 static void *locks_start(struct seq_file *f, loff_t *pos)
 	__acquires(&blocked_lock_lock)
 {
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 8e5ad83..f04e13d 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -8,6 +8,7 @@
 #include <linux/security.h>
 #include <linux/file.h>
 #include <linux/seq_file.h>
+#include <linux/fs.h>
 
 #include <linux/proc_fs.h>
 
@@ -48,16 +49,24 @@ static int seq_show(struct seq_file *m, void *v)
 		put_files_struct(files);
 	}
 
-	if (!ret) {
-		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
-			   (long long)file->f_pos, f_flags,
-			   real_mount(file->f_path.mnt)->mnt_id);
-		if (file->f_op->show_fdinfo)
-			file->f_op->show_fdinfo(m, file);
-		ret = seq_has_overflowed(m);
-		fput(file);
-	}
+	if (ret)
+		return ret;
+
+	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
+		   (long long)file->f_pos, f_flags,
+		   real_mount(file->f_path.mnt)->mnt_id);
+
+	show_fd_locks(m, file, files);
+	ret = seq_has_overflowed(m);
+	if (ret)
+		goto out;
+
+	if (file->f_op->show_fdinfo)
+		file->f_op->show_fdinfo(m, file);
+	ret = seq_has_overflowed(m);
 
+out:
+	fput(file);
 	return ret;
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b4d71b5..ba0bd2a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1018,6 +1018,9 @@ extern void lease_get_mtime(struct inode *, struct timespec *time);
 extern int generic_setlease(struct file *, long, struct file_lock **, void **priv);
 extern int vfs_setlease(struct file *, long, struct file_lock **, void **);
 extern int lease_modify(struct file_lock *, int, struct list_head *);
+struct files_struct;
+extern void show_fd_locks(struct seq_file *f,
+			 struct file *filp, struct files_struct *files);
 #else /* !CONFIG_FILE_LOCKING */
 static inline int fcntl_getlk(struct file *file, unsigned int cmd,
 			      struct flock __user *user)
@@ -1154,6 +1157,10 @@ static inline int lease_modify(struct file_lock *fl, int arg,
 {
 	return -EINVAL;
 }
+
+struct files_struct;
+static inline void show_fd_locks(struct seq_file *f,
+			struct file *filp, struct files_struct *files) {}
 #endif /* !CONFIG_FILE_LOCKING */
 
 
-- 
2.1.0

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Jeff Layton]

On Thu,  5 Mar 2015 18:37:18 +0300
Andrey Vagin <avagin@openvz.org> wrote:

> Let's show locks which are associated with a file descriptor in
> its fdinfo file.
> 
> Currently we don't have a reliable way to determine who holds a lock.
> We can find some information in /proc/locks, but PID which is reported
> there can be wrong. For example, a process takes a lock, then forks a
> child and dies. In this case /proc/locks contains the parent pid, which
> can be reused by another process.
> 
> $ cat /proc/locks
> ...
> 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> ...
> 
> $ ps -C rpcbind
>   PID TTY          TIME CMD
>   332 ?        00:00:00 rpcbind
> 
> $ cat /proc/332/fdinfo/4
> pos:	0
> flags:	0100000
> mnt_id:	22
> lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> 
> $ ls -l /proc/332/fd/4
> lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock
> 
> $ ls -l /proc/324/fd/
> total 0
> lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
> lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
> lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0
> 
> You can see that the process with the 324 pid doesn't hold the lock.
> 
> This information is required for proper dumping and restoring file
> locks.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Jeff Layton <jlayton@poochiereds.net>
> Cc: "J. Bruce Fields" <bfields@fieldses.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
>  Documentation/filesystems/proc.txt |  4 ++++
>  fs/locks.c                         | 38 ++++++++++++++++++++++++++++++++++++++
>  fs/proc/fd.c                       | 27 ++++++++++++++++++---------
>  include/linux/fs.h                 |  7 +++++++
>  4 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
> index a07ba61..6331623 100644
> --- a/Documentation/filesystems/proc.txt
> +++ b/Documentation/filesystems/proc.txt
> @@ -1704,6 +1704,10 @@ A typical output is
>  	flags:	0100002
>  	mnt_id:	19
>  
> +All locks associated with a file descriptor are shown in its fdinfo too.
> +
> +lock:       1: FLOCK  ADVISORY  WRITE 359 00:13:11691 0 EOF
> +
>  The files such as eventfd, fsnotify, signalfd, epoll among the regular pos/flags
>  pair provide additional information particular to the objects they represent.
>  
> diff --git a/fs/locks.c b/fs/locks.c
> index 365c82e..815f832 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2592,6 +2592,44 @@ static int locks_show(struct seq_file *f, void *v)
>  	return 0;
>  }
>  
> +static void __show_fd_locks(struct seq_file *f,
> +			struct list_head *head, int *id,
> +			struct file *filp, struct files_struct *files)
> +{
> +	struct file_lock *fl;
> +
> +	list_for_each_entry(fl, head, fl_list) {
> +
> +		if (filp != fl->fl_file)
> +			continue;
> +		if (fl->fl_owner != files &&
> +		    fl->fl_owner != filp)
> +			continue;
> +
> +		(*id)++;
> +		seq_puts(f, "lock:\t");
> +		lock_get_status(f, fl, *id, "");
> +	}
> +}
> +
> +void show_fd_locks(struct seq_file *f,
> +		  struct file *filp, struct files_struct *files)
> +{
> +	struct inode *inode = file_inode(filp);
> +	struct file_lock_context *ctx;
> +	int id = 0;
> +
> +	ctx = inode->i_flctx;
> +	if (!ctx)
> +		return;
> +
> +	spin_lock(&ctx->flc_lock);
> +	__show_fd_locks(f, &ctx->flc_flock, &id, filp, files);
> +	__show_fd_locks(f, &ctx->flc_posix, &id, filp, files);
> +	__show_fd_locks(f, &ctx->flc_lease, &id, filp, files);
> +	spin_unlock(&ctx->flc_lock);
> +}
> +
>  static void *locks_start(struct seq_file *f, loff_t *pos)
>  	__acquires(&blocked_lock_lock)
>  {
> diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> index 8e5ad83..f04e13d 100644
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -8,6 +8,7 @@
>  #include <linux/security.h>
>  #include <linux/file.h>
>  #include <linux/seq_file.h>
> +#include <linux/fs.h>
>  
>  #include <linux/proc_fs.h>
>  
> @@ -48,16 +49,24 @@ static int seq_show(struct seq_file *m, void *v)
>  		put_files_struct(files);
>  	}
>  
> -	if (!ret) {
> -		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> -			   (long long)file->f_pos, f_flags,
> -			   real_mount(file->f_path.mnt)->mnt_id);
> -		if (file->f_op->show_fdinfo)
> -			file->f_op->show_fdinfo(m, file);
> -		ret = seq_has_overflowed(m);
> -		fput(file);
> -	}
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> +		   (long long)file->f_pos, f_flags,
> +		   real_mount(file->f_path.mnt)->mnt_id);
> +
> +	show_fd_locks(m, file, files);
> +	ret = seq_has_overflowed(m);
> +	if (ret)
> +		goto out;
> +
> +	if (file->f_op->show_fdinfo)
> +		file->f_op->show_fdinfo(m, file);
> +	ret = seq_has_overflowed(m);
>  
> +out:
> +	fput(file);
>  	return ret;
>  }
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b4d71b5..ba0bd2a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1018,6 +1018,9 @@ extern void lease_get_mtime(struct inode *, struct timespec *time);
>  extern int generic_setlease(struct file *, long, struct file_lock **, void **priv);
>  extern int vfs_setlease(struct file *, long, struct file_lock **, void **);
>  extern int lease_modify(struct file_lock *, int, struct list_head *);
> +struct files_struct;
> +extern void show_fd_locks(struct seq_file *f,
> +			 struct file *filp, struct files_struct *files);
>  #else /* !CONFIG_FILE_LOCKING */
>  static inline int fcntl_getlk(struct file *file, unsigned int cmd,
>  			      struct flock __user *user)
> @@ -1154,6 +1157,10 @@ static inline int lease_modify(struct file_lock *fl, int arg,
>  {
>  	return -EINVAL;
>  }
> +
> +struct files_struct;
> +static inline void show_fd_locks(struct seq_file *f,
> +			struct file *filp, struct files_struct *files) {}
>  #endif /* !CONFIG_FILE_LOCKING */
>  
>  

Looks reasonably sane to me. Via what tree should this go in?

Acked-by: Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Cyrill Gorcunov]

On Thu, Mar 05, 2015 at 06:37:18PM +0300, Andrey Vagin wrote:
> Let's show locks which are associated with a file descriptor in
> its fdinfo file.
> 
> Currently we don't have a reliable way to determine who holds a lock.
> We can find some information in /proc/locks, but PID which is reported
> there can be wrong. For example, a process takes a lock, then forks a
> child and dies. In this case /proc/locks contains the parent pid, which
> can be reused by another process.
> 
> $ cat /proc/locks
> ...
> 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> ...
> 
> $ ps -C rpcbind
>   PID TTY          TIME CMD
>   332 ?        00:00:00 rpcbind
> 
...

Looks reasonable to me, thanks!
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrew Vagin]

On Thu, Mar 05, 2015 at 02:11:40PM -0500, Jeff Layton wrote:
> On Thu,  5 Mar 2015 18:37:18 +0300
> Andrey Vagin <avagin@openvz.org> wrote:
> 
> > Let's show locks which are associated with a file descriptor in
> > its fdinfo file.
> > 
> > Currently we don't have a reliable way to determine who holds a lock.
> > We can find some information in /proc/locks, but PID which is reported
> > there can be wrong. For example, a process takes a lock, then forks a
> > child and dies. In this case /proc/locks contains the parent pid, which
> > can be reused by another process.
> > 
> > $ cat /proc/locks
> > ...
> > 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> > ...
> > 
> > $ ps -C rpcbind
> >   PID TTY          TIME CMD
> >   332 ?        00:00:00 rpcbind
> > 
> > $ cat /proc/332/fdinfo/4
> > pos:	0
> > flags:	0100000
> > mnt_id:	22
> > lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> > 
> > $ ls -l /proc/332/fd/4
> > lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock
> > 
> > $ ls -l /proc/324/fd/
> > total 0
> > lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
> > lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
> > lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0
> > 
> > You can see that the process with the 324 pid doesn't hold the lock.
> > 
> > This information is required for proper dumping and restoring file
> > locks.
> > 
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: Jeff Layton <jlayton@poochiereds.net>
> > Cc: "J. Bruce Fields" <bfields@fieldses.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> > Cc: Pavel Emelyanov <xemul@parallels.com>
> > Signed-off-by: Andrey Vagin <avagin@openvz.org>
> > ---
> >  Documentation/filesystems/proc.txt |  4 ++++
> >  fs/locks.c                         | 38 ++++++++++++++++++++++++++++++++++++++
> >  fs/proc/fd.c                       | 27 ++++++++++++++++++---------
> >  include/linux/fs.h                 |  7 +++++++
> >  4 files changed, 67 insertions(+), 9 deletions(-)
> > 

...

> > +struct files_struct;
> > +static inline void show_fd_locks(struct seq_file *f,
> > +			struct file *filp, struct files_struct *files) {}
> >  #endif /* !CONFIG_FILE_LOCKING */
> >  
> >  
> 
> Looks reasonably sane to me. Via what tree should this go in?

I don't know. I think all previous patches about fdinfo went through
linux-mm. Andrew, what do you think?

> 
> Acked-by: Jeff Layton <jlayton@poochiereds.net>

Thanks,
Andrey

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[J. Bruce Fields]

On Thu, Mar 05, 2015 at 06:37:18PM +0300, Andrey Vagin wrote:
> Let's show locks which are associated with a file descriptor in
> its fdinfo file.
> 
> Currently we don't have a reliable way to determine who holds a lock.
> We can find some information in /proc/locks, but PID which is reported
> there can be wrong. For example, a process takes a lock, then forks a
> child and dies. In this case /proc/locks contains the parent pid, which
> can be reused by another process.
> 
> $ cat /proc/locks
> ...
> 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> ...
> 
> $ ps -C rpcbind
>   PID TTY          TIME CMD
>   332 ?        00:00:00 rpcbind
> 
> $ cat /proc/332/fdinfo/4
> pos:	0
> flags:	0100000
> mnt_id:	22
> lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF

The major:minor part is redundant as long as you have the mnt_id, right?

But I think it makes sense to leave it as you have it, with the same
format as /proc/locks.  We get to share the kernel code, maybe userland
gets to reuse a little code too.

And we should really remove that "ifdef WE_CAN_BREAK_LSLK_NOW" from
fs/locks.c, clearly that's not going to happen.  (And add a comment that
the better solution may be to get the mntid from fdinfo, if that's
true?)

Anyway, that's a digression, ACK to the patch.

--b.

> 
> $ ls -l /proc/332/fd/4
> lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock
> 
> $ ls -l /proc/324/fd/
> total 0
> lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
> lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
> lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0
> 
> You can see that the process with the 324 pid doesn't hold the lock.
> 
> This information is required for proper dumping and restoring file
> locks.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Jeff Layton <jlayton@poochiereds.net>
> Cc: "J. Bruce Fields" <bfields@fieldses.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
>  Documentation/filesystems/proc.txt |  4 ++++
>  fs/locks.c                         | 38 ++++++++++++++++++++++++++++++++++++++
>  fs/proc/fd.c                       | 27 ++++++++++++++++++---------
>  include/linux/fs.h                 |  7 +++++++
>  4 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
> index a07ba61..6331623 100644
> --- a/Documentation/filesystems/proc.txt
> +++ b/Documentation/filesystems/proc.txt
> @@ -1704,6 +1704,10 @@ A typical output is
>  	flags:	0100002
>  	mnt_id:	19
>  
> +All locks associated with a file descriptor are shown in its fdinfo too.
> +
> +lock:       1: FLOCK  ADVISORY  WRITE 359 00:13:11691 0 EOF
> +
>  The files such as eventfd, fsnotify, signalfd, epoll among the regular pos/flags
>  pair provide additional information particular to the objects they represent.
>  
> diff --git a/fs/locks.c b/fs/locks.c
> index 365c82e..815f832 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2592,6 +2592,44 @@ static int locks_show(struct seq_file *f, void *v)
>  	return 0;
>  }
>  
> +static void __show_fd_locks(struct seq_file *f,
> +			struct list_head *head, int *id,
> +			struct file *filp, struct files_struct *files)
> +{
> +	struct file_lock *fl;
> +
> +	list_for_each_entry(fl, head, fl_list) {
> +
> +		if (filp != fl->fl_file)
> +			continue;
> +		if (fl->fl_owner != files &&
> +		    fl->fl_owner != filp)
> +			continue;
> +
> +		(*id)++;
> +		seq_puts(f, "lock:\t");
> +		lock_get_status(f, fl, *id, "");
> +	}
> +}
> +
> +void show_fd_locks(struct seq_file *f,
> +		  struct file *filp, struct files_struct *files)
> +{
> +	struct inode *inode = file_inode(filp);
> +	struct file_lock_context *ctx;
> +	int id = 0;
> +
> +	ctx = inode->i_flctx;
> +	if (!ctx)
> +		return;
> +
> +	spin_lock(&ctx->flc_lock);
> +	__show_fd_locks(f, &ctx->flc_flock, &id, filp, files);
> +	__show_fd_locks(f, &ctx->flc_posix, &id, filp, files);
> +	__show_fd_locks(f, &ctx->flc_lease, &id, filp, files);
> +	spin_unlock(&ctx->flc_lock);
> +}
> +
>  static void *locks_start(struct seq_file *f, loff_t *pos)
>  	__acquires(&blocked_lock_lock)
>  {
> diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> index 8e5ad83..f04e13d 100644
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -8,6 +8,7 @@
>  #include <linux/security.h>
>  #include <linux/file.h>
>  #include <linux/seq_file.h>
> +#include <linux/fs.h>
>  
>  #include <linux/proc_fs.h>
>  
> @@ -48,16 +49,24 @@ static int seq_show(struct seq_file *m, void *v)
>  		put_files_struct(files);
>  	}
>  
> -	if (!ret) {
> -		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> -			   (long long)file->f_pos, f_flags,
> -			   real_mount(file->f_path.mnt)->mnt_id);
> -		if (file->f_op->show_fdinfo)
> -			file->f_op->show_fdinfo(m, file);
> -		ret = seq_has_overflowed(m);
> -		fput(file);
> -	}
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> +		   (long long)file->f_pos, f_flags,
> +		   real_mount(file->f_path.mnt)->mnt_id);
> +
> +	show_fd_locks(m, file, files);
> +	ret = seq_has_overflowed(m);
> +	if (ret)
> +		goto out;
> +
> +	if (file->f_op->show_fdinfo)
> +		file->f_op->show_fdinfo(m, file);
> +	ret = seq_has_overflowed(m);
>  
> +out:
> +	fput(file);
>  	return ret;
>  }
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b4d71b5..ba0bd2a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1018,6 +1018,9 @@ extern void lease_get_mtime(struct inode *, struct timespec *time);
>  extern int generic_setlease(struct file *, long, struct file_lock **, void **priv);
>  extern int vfs_setlease(struct file *, long, struct file_lock **, void **);
>  extern int lease_modify(struct file_lock *, int, struct list_head *);
> +struct files_struct;
> +extern void show_fd_locks(struct seq_file *f,
> +			 struct file *filp, struct files_struct *files);
>  #else /* !CONFIG_FILE_LOCKING */
>  static inline int fcntl_getlk(struct file *file, unsigned int cmd,
>  			      struct flock __user *user)
> @@ -1154,6 +1157,10 @@ static inline int lease_modify(struct file_lock *fl, int arg,
>  {
>  	return -EINVAL;
>  }
> +
> +struct files_struct;
> +static inline void show_fd_locks(struct seq_file *f,
> +			struct file *filp, struct files_struct *files) {}
>  #endif /* !CONFIG_FILE_LOCKING */
>  
>  
> -- 
> 2.1.0

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Jeff Layton]

On Fri, 6 Mar 2015 09:41:00 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Thu, Mar 05, 2015 at 06:37:18PM +0300, Andrey Vagin wrote:
> > Let's show locks which are associated with a file descriptor in
> > its fdinfo file.
> > 
> > Currently we don't have a reliable way to determine who holds a lock.
> > We can find some information in /proc/locks, but PID which is reported
> > there can be wrong. For example, a process takes a lock, then forks a
> > child and dies. In this case /proc/locks contains the parent pid, which
> > can be reused by another process.
> > 
> > $ cat /proc/locks
> > ...
> > 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> > ...
> > 
> > $ ps -C rpcbind
> >   PID TTY          TIME CMD
> >   332 ?        00:00:00 rpcbind
> > 
> > $ cat /proc/332/fdinfo/4
> > pos:	0
> > flags:	0100000
> > mnt_id:	22
> > lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> 
> The major:minor part is redundant as long as you have the mnt_id, right?
> 
> But I think it makes sense to leave it as you have it, with the same
> format as /proc/locks.  We get to share the kernel code, maybe userland
> gets to reuse a little code too.
> 
> And we should really remove that "ifdef WE_CAN_BREAK_LSLK_NOW" from
> fs/locks.c, clearly that's not going to happen.  (And add a comment that
> the better solution may be to get the mntid from fdinfo, if that's
> true?)
> 

Yeah, I don't see a good reason to keep that in place either. It's been
there since the git history started. I'll queue up a patch to remove it
in v4.1.

Thanks!

> Anyway, that's a digression, ACK to the patch.
> 
> --b.
> 
> > 
> > $ ls -l /proc/332/fd/4
> > lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock
> > 
> > $ ls -l /proc/324/fd/
> > total 0
> > lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
> > lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
> > lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0
> > 
> > You can see that the process with the 324 pid doesn't hold the lock.
> > 
> > This information is required for proper dumping and restoring file
> > locks.
> > 
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: Jeff Layton <jlayton@poochiereds.net>
> > Cc: "J. Bruce Fields" <bfields@fieldses.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> > Cc: Pavel Emelyanov <xemul@parallels.com>
> > Signed-off-by: Andrey Vagin <avagin@openvz.org>
> > ---
> >  Documentation/filesystems/proc.txt |  4 ++++
> >  fs/locks.c                         | 38 ++++++++++++++++++++++++++++++++++++++
> >  fs/proc/fd.c                       | 27 ++++++++++++++++++---------
> >  include/linux/fs.h                 |  7 +++++++
> >  4 files changed, 67 insertions(+), 9 deletions(-)
> > 
> > diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
> > index a07ba61..6331623 100644
> > --- a/Documentation/filesystems/proc.txt
> > +++ b/Documentation/filesystems/proc.txt
> > @@ -1704,6 +1704,10 @@ A typical output is
> >  	flags:	0100002
> >  	mnt_id:	19
> >  
> > +All locks associated with a file descriptor are shown in its fdinfo too.
> > +
> > +lock:       1: FLOCK  ADVISORY  WRITE 359 00:13:11691 0 EOF
> > +
> >  The files such as eventfd, fsnotify, signalfd, epoll among the regular pos/flags
> >  pair provide additional information particular to the objects they represent.
> >  
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 365c82e..815f832 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -2592,6 +2592,44 @@ static int locks_show(struct seq_file *f, void *v)
> >  	return 0;
> >  }
> >  
> > +static void __show_fd_locks(struct seq_file *f,
> > +			struct list_head *head, int *id,
> > +			struct file *filp, struct files_struct *files)
> > +{
> > +	struct file_lock *fl;
> > +
> > +	list_for_each_entry(fl, head, fl_list) {
> > +
> > +		if (filp != fl->fl_file)
> > +			continue;
> > +		if (fl->fl_owner != files &&
> > +		    fl->fl_owner != filp)
> > +			continue;
> > +
> > +		(*id)++;
> > +		seq_puts(f, "lock:\t");
> > +		lock_get_status(f, fl, *id, "");
> > +	}
> > +}
> > +
> > +void show_fd_locks(struct seq_file *f,
> > +		  struct file *filp, struct files_struct *files)
> > +{
> > +	struct inode *inode = file_inode(filp);
> > +	struct file_lock_context *ctx;
> > +	int id = 0;
> > +
> > +	ctx = inode->i_flctx;
> > +	if (!ctx)
> > +		return;
> > +
> > +	spin_lock(&ctx->flc_lock);
> > +	__show_fd_locks(f, &ctx->flc_flock, &id, filp, files);
> > +	__show_fd_locks(f, &ctx->flc_posix, &id, filp, files);
> > +	__show_fd_locks(f, &ctx->flc_lease, &id, filp, files);
> > +	spin_unlock(&ctx->flc_lock);
> > +}
> > +
> >  static void *locks_start(struct seq_file *f, loff_t *pos)
> >  	__acquires(&blocked_lock_lock)
> >  {
> > diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> > index 8e5ad83..f04e13d 100644
> > --- a/fs/proc/fd.c
> > +++ b/fs/proc/fd.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/security.h>
> >  #include <linux/file.h>
> >  #include <linux/seq_file.h>
> > +#include <linux/fs.h>
> >  
> >  #include <linux/proc_fs.h>
> >  
> > @@ -48,16 +49,24 @@ static int seq_show(struct seq_file *m, void *v)
> >  		put_files_struct(files);
> >  	}
> >  
> > -	if (!ret) {
> > -		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> > -			   (long long)file->f_pos, f_flags,
> > -			   real_mount(file->f_path.mnt)->mnt_id);
> > -		if (file->f_op->show_fdinfo)
> > -			file->f_op->show_fdinfo(m, file);
> > -		ret = seq_has_overflowed(m);
> > -		fput(file);
> > -	}
> > +	if (ret)
> > +		return ret;
> > +
> > +	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> > +		   (long long)file->f_pos, f_flags,
> > +		   real_mount(file->f_path.mnt)->mnt_id);
> > +
> > +	show_fd_locks(m, file, files);
> > +	ret = seq_has_overflowed(m);
> > +	if (ret)
> > +		goto out;
> > +
> > +	if (file->f_op->show_fdinfo)
> > +		file->f_op->show_fdinfo(m, file);
> > +	ret = seq_has_overflowed(m);
> >  
> > +out:
> > +	fput(file);
> >  	return ret;
> >  }
> >  
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index b4d71b5..ba0bd2a 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1018,6 +1018,9 @@ extern void lease_get_mtime(struct inode *, struct timespec *time);
> >  extern int generic_setlease(struct file *, long, struct file_lock **, void **priv);
> >  extern int vfs_setlease(struct file *, long, struct file_lock **, void **);
> >  extern int lease_modify(struct file_lock *, int, struct list_head *);
> > +struct files_struct;
> > +extern void show_fd_locks(struct seq_file *f,
> > +			 struct file *filp, struct files_struct *files);
> >  #else /* !CONFIG_FILE_LOCKING */
> >  static inline int fcntl_getlk(struct file *file, unsigned int cmd,
> >  			      struct flock __user *user)
> > @@ -1154,6 +1157,10 @@ static inline int lease_modify(struct file_lock *fl, int arg,
> >  {
> >  	return -EINVAL;
> >  }
> > +
> > +struct files_struct;
> > +static inline void show_fd_locks(struct seq_file *f,
> > +			struct file *filp, struct files_struct *files) {}
> >  #endif /* !CONFIG_FILE_LOCKING */
> >  
> >  
> > -- 
> > 2.1.0


-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrew Morton]

On Thu,  5 Mar 2015 18:37:18 +0300 Andrey Vagin <avagin@openvz.org> wrote:

> Let's show locks which are associated with a file descriptor in
> its fdinfo file.
> 
> Currently we don't have a reliable way to determine who holds a lock.
> We can find some information in /proc/locks, but PID which is reported
> there can be wrong. For example, a process takes a lock, then forks a
> child and dies. In this case /proc/locks contains the parent pid, which
> can be reused by another process.
> 
> $ cat /proc/locks
> ...
> 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> ...
> 
> $ ps -C rpcbind
>   PID TTY          TIME CMD
>   332 ?        00:00:00 rpcbind
> 
> $ cat /proc/332/fdinfo/4
> pos:	0
> flags:	0100000
> mnt_id:	22
> lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> 
> $ ls -l /proc/332/fd/4
> lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock
> 
> $ ls -l /proc/324/fd/
> total 0
> lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
> lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
> lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0
> 
> You can see that the process with the 324 pid doesn't hold the lock.
> 
> This information is required for proper dumping and restoring file
> locks.
> 
> ...
>
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -8,6 +8,7 @@
>  #include <linux/security.h>
>  #include <linux/file.h>
>  #include <linux/seq_file.h>
> +#include <linux/fs.h>
>  
>  #include <linux/proc_fs.h>
>  
> @@ -48,16 +49,24 @@ static int seq_show(struct seq_file *m, void *v)
>  		put_files_struct(files);
>  	}
>  
> -	if (!ret) {
> -		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> -			   (long long)file->f_pos, f_flags,
> -			   real_mount(file->f_path.mnt)->mnt_id);
> -		if (file->f_op->show_fdinfo)
> -			file->f_op->show_fdinfo(m, file);
> -		ret = seq_has_overflowed(m);
> -		fput(file);
> -	}
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> +		   (long long)file->f_pos, f_flags,
> +		   real_mount(file->f_path.mnt)->mnt_id);
> +
> +	show_fd_locks(m, file, files);
> +	ret = seq_has_overflowed(m);
> +	if (ret)
> +		goto out;

seq_has_overflowed() returns a boolean, but fs/seq_file.c:traverse() is
looking for a -ve errno from ->show().

Documentation/filesystems/seq_file.txt says

If all is well, the show() function should return zero.  A negative error
code in the usual manner indicates that something went wrong; it will be
passed back to user space.  This function can also return SEQ_SKIP, which
causes the current item to be skipped; if the show() function has already
generated output before returning SEQ_SKIP, that output will be dropped.


> +	if (file->f_op->show_fdinfo)
> +		file->f_op->show_fdinfo(m, file);
> +	ret = seq_has_overflowed(m);
>  
> +out:
> +	fput(file);
>  	return ret;
>  }

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrew Vagin]

[-- Attachment #1: Type: text/plain, Size: 3200 bytes --]

On Wed, Mar 11, 2015 at 03:08:53PM -0700, Andrew Morton wrote:
> On Thu,  5 Mar 2015 18:37:18 +0300 Andrey Vagin <avagin@openvz.org> wrote:
> 
> > Let's show locks which are associated with a file descriptor in
> > its fdinfo file.
> > 
> > Currently we don't have a reliable way to determine who holds a lock.
> > We can find some information in /proc/locks, but PID which is reported
> > there can be wrong. For example, a process takes a lock, then forks a
> > child and dies. In this case /proc/locks contains the parent pid, which
> > can be reused by another process.
> > 
> > $ cat /proc/locks
> > ...
> > 6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> > ...
> > 
> > $ ps -C rpcbind
> >   PID TTY          TIME CMD
> >   332 ?        00:00:00 rpcbind
> > 
> > $ cat /proc/332/fdinfo/4
> > pos:	0
> > flags:	0100000
> > mnt_id:	22
> > lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
> > 
> > $ ls -l /proc/332/fd/4
> > lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock
> > 
> > $ ls -l /proc/324/fd/
> > total 0
> > lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
> > lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
> > lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0
> > 
> > You can see that the process with the 324 pid doesn't hold the lock.
> > 
> > This information is required for proper dumping and restoring file
> > locks.
> > 
> > ...
> >
> > --- a/fs/proc/fd.c
> > +++ b/fs/proc/fd.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/security.h>
> >  #include <linux/file.h>
> >  #include <linux/seq_file.h>
> > +#include <linux/fs.h>
> >  
> >  #include <linux/proc_fs.h>
> >  
> > @@ -48,16 +49,24 @@ static int seq_show(struct seq_file *m, void *v)
> >  		put_files_struct(files);
> >  	}
> >  
> > -	if (!ret) {
> > -		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> > -			   (long long)file->f_pos, f_flags,
> > -			   real_mount(file->f_path.mnt)->mnt_id);
> > -		if (file->f_op->show_fdinfo)
> > -			file->f_op->show_fdinfo(m, file);
> > -		ret = seq_has_overflowed(m);
> > -		fput(file);
> > -	}
> > +	if (ret)
> > +		return ret;
> > +
> > +	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
> > +		   (long long)file->f_pos, f_flags,
> > +		   real_mount(file->f_path.mnt)->mnt_id);
> > +
> > +	show_fd_locks(m, file, files);
> > +	ret = seq_has_overflowed(m);
> > +	if (ret)
> > +		goto out;
> 
> seq_has_overflowed() returns a boolean, but fs/seq_file.c:traverse() is
> looking for a -ve errno from ->show().
> 
> Documentation/filesystems/seq_file.txt says
> 
> If all is well, the show() function should return zero.  A negative error
> code in the usual manner indicates that something went wrong; it will be
> passed back to user space.  This function can also return SEQ_SKIP, which
> causes the current item to be skipped; if the show() function has already
> generated output before returning SEQ_SKIP, that output will be dropped.

You are right. The updated version of this patch is attached. Thank you
for the review.

> 
> 
> > +	if (file->f_op->show_fdinfo)
> > +		file->f_op->show_fdinfo(m, file);
> > +	ret = seq_has_overflowed(m);
> >  
> > +out:
> > +	fput(file);
> >  	return ret;
> >  }
> 

[-- Attachment #2: 0001-proc-show-locks-in-proc-pid-fdinfo-X-v2.patch --]
[-- Type: text/plain, Size: 5864 bytes --]

>From 29fcabc8a9d9c8bea4a7ec6202a7c0f6e98a518e Mon Sep 17 00:00:00 2001
From: Andrey Vagin <avagin@openvz.org>
Date: Thu, 26 Feb 2015 15:46:02 +0300
Subject: [PATCH] proc: show locks in /proc/pid/fdinfo/X (v2)

Let's show locks which are associated with a file descriptor in
its fdinfo file.

Currently we don't have a reliable way to determine who holds a lock.
We can find some information in /proc/locks, but PID which is reported
there can be wrong. For example, a process takes a lock, then forks a
child and dies. In this case /proc/locks contains the parent pid, which
can be reused by another process.

$ cat /proc/locks
...
6: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF
...

$ ps -C rpcbind
  PID TTY          TIME CMD
  332 ?        00:00:00 rpcbind

$ cat /proc/332/fdinfo/4
pos:	0
flags:	0100000
mnt_id:	22
lock:	1: FLOCK  ADVISORY  WRITE 324 00:13:13431 0 EOF

$ ls -l /proc/332/fd/4
lr-x------ 1 root root 64 Mar  5 14:43 /proc/332/fd/4 -> /run/rpcbind.lock

$ ls -l /proc/324/fd/
total 0
lrwx------ 1 root root 64 Feb 27 14:50 0 -> /dev/pts/0
lrwx------ 1 root root 64 Feb 27 14:50 1 -> /dev/pts/0
lrwx------ 1 root root 64 Feb 27 14:49 2 -> /dev/pts/0

You can see that the process with the 324 pid doesn't hold the lock.

This information is required for proper dumping and restoring file
locks.

v1:
Acked-by: Jeff Layton <jlayton@poochiereds.net>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Acked-by: "J. Bruce Fields" <bfields@fieldses.org>

v2: use seq_has_overflowed() properly

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 Documentation/filesystems/proc.txt |  4 ++++
 fs/locks.c                         | 38 ++++++++++++++++++++++++++++++++++++++
 fs/proc/fd.c                       | 27 +++++++++++++++++----------
 include/linux/fs.h                 |  7 +++++++
 4 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index a07ba61..6331623 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -1704,6 +1704,10 @@ A typical output is
 	flags:	0100002
 	mnt_id:	19
 
+All locks associated with a file descriptor are shown in its fdinfo too.
+
+lock:       1: FLOCK  ADVISORY  WRITE 359 00:13:11691 0 EOF
+
 The files such as eventfd, fsnotify, signalfd, epoll among the regular pos/flags
 pair provide additional information particular to the objects they represent.
 
diff --git a/fs/locks.c b/fs/locks.c
index f1bad68..89a0540 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2593,6 +2593,44 @@ static int locks_show(struct seq_file *f, void *v)
 	return 0;
 }
 
+static void __show_fd_locks(struct seq_file *f,
+			struct list_head *head, int *id,
+			struct file *filp, struct files_struct *files)
+{
+	struct file_lock *fl;
+
+	list_for_each_entry(fl, head, fl_list) {
+
+		if (filp != fl->fl_file)
+			continue;
+		if (fl->fl_owner != files &&
+		    fl->fl_owner != filp)
+			continue;
+
+		(*id)++;
+		seq_puts(f, "lock:\t");
+		lock_get_status(f, fl, *id, "");
+	}
+}
+
+void show_fd_locks(struct seq_file *f,
+		  struct file *filp, struct files_struct *files)
+{
+	struct inode *inode = file_inode(filp);
+	struct file_lock_context *ctx;
+	int id = 0;
+
+	ctx = inode->i_flctx;
+	if (!ctx)
+		return;
+
+	spin_lock(&ctx->flc_lock);
+	__show_fd_locks(f, &ctx->flc_flock, &id, filp, files);
+	__show_fd_locks(f, &ctx->flc_posix, &id, filp, files);
+	__show_fd_locks(f, &ctx->flc_lease, &id, filp, files);
+	spin_unlock(&ctx->flc_lock);
+}
+
 static void *locks_start(struct seq_file *f, loff_t *pos)
 	__acquires(&blocked_lock_lock)
 {
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 8e5ad83..af84ad0 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -8,6 +8,7 @@
 #include <linux/security.h>
 #include <linux/file.h>
 #include <linux/seq_file.h>
+#include <linux/fs.h>
 
 #include <linux/proc_fs.h>
 
@@ -48,17 +49,23 @@ static int seq_show(struct seq_file *m, void *v)
 		put_files_struct(files);
 	}
 
-	if (!ret) {
-		seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
-			   (long long)file->f_pos, f_flags,
-			   real_mount(file->f_path.mnt)->mnt_id);
-		if (file->f_op->show_fdinfo)
-			file->f_op->show_fdinfo(m, file);
-		ret = seq_has_overflowed(m);
-		fput(file);
-	}
+	if (ret)
+		return ret;
 
-	return ret;
+	seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
+		   (long long)file->f_pos, f_flags,
+		   real_mount(file->f_path.mnt)->mnt_id);
+
+	show_fd_locks(m, file, files);
+	if (seq_has_overflowed(m))
+		goto out;
+
+	if (file->f_op->show_fdinfo)
+		file->f_op->show_fdinfo(m, file);
+
+out:
+	fput(file);
+	return 0;
 }
 
 static int seq_fdinfo_open(struct inode *inode, struct file *file)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b4d71b5..ba0bd2a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1018,6 +1018,9 @@ extern void lease_get_mtime(struct inode *, struct timespec *time);
 extern int generic_setlease(struct file *, long, struct file_lock **, void **priv);
 extern int vfs_setlease(struct file *, long, struct file_lock **, void **);
 extern int lease_modify(struct file_lock *, int, struct list_head *);
+struct files_struct;
+extern void show_fd_locks(struct seq_file *f,
+			 struct file *filp, struct files_struct *files);
 #else /* !CONFIG_FILE_LOCKING */
 static inline int fcntl_getlk(struct file *file, unsigned int cmd,
 			      struct flock __user *user)
@@ -1154,6 +1157,10 @@ static inline int lease_modify(struct file_lock *fl, int arg,
 {
 	return -EINVAL;
 }
+
+struct files_struct;
+static inline void show_fd_locks(struct seq_file *f,
+			struct file *filp, struct files_struct *files) {}
 #endif /* !CONFIG_FILE_LOCKING */
 
 
-- 
2.1.0

[PATCH] selftest: add a test case to check how locks are shown in fdinfo

[Andrey Vagin]

The main idea of this test is to check that locks are shown correctly
when they can't be placed in a default seq_file buffer due to its size.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 tools/testing/selftests/Makefile        |   1 +
 tools/testing/selftests/fdinfo/Makefile |  11 +++
 tools/testing/selftests/fdinfo/locks.c  | 119 ++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/fdinfo/Makefile
 create mode 100644 tools/testing/selftests/fdinfo/locks.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 4e51122..8cd57f6 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -17,6 +17,7 @@ TARGETS += sysctl
 TARGETS += timers
 TARGETS += user
 TARGETS += vm
+TARGETS += fdinfo
 #Please keep the TARGETS list alphabetically sorted
 
 TARGETS_HOTPLUG = cpu-hotplug
diff --git a/tools/testing/selftests/fdinfo/Makefile b/tools/testing/selftests/fdinfo/Makefile
new file mode 100644
index 0000000..83f34ef
--- /dev/null
+++ b/tools/testing/selftests/fdinfo/Makefile
@@ -0,0 +1,11 @@
+CFLAGS += -Wall
+
+all: locks
+
+run_tests: all
+	@./locks || echo "locks: [FAIL]"
+
+locks: locks.c
+
+clean:
+	rm -f locks
diff --git a/tools/testing/selftests/fdinfo/locks.c b/tools/testing/selftests/fdinfo/locks.c
new file mode 100644
index 0000000..93f25c6
--- /dev/null
+++ b/tools/testing/selftests/fdinfo/locks.c
@@ -0,0 +1,119 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#define pr_perror(fmt, ...) fprintf(stderr, "%s:%d: " fmt ": %m\n", \
+					__FILE__, __LINE__, ##__VA_ARGS__)
+
+#define FILE_SIZE 4096
+
+int main(int argc, char **argv)
+{
+	int fd, fdinfo, i, ret, size, bsize = 4096;
+	char *buf, *p, fdinfo_path[] = "/proc/self/fdinfo/XXXXXXXXXX";
+
+	fd = open("test_file", O_RDWR | O_CREAT, 0666);
+	if (fd == -1) {
+		pr_perror("Unable to open test_file");
+		return 1;
+	}
+	unlink("test_file");
+	if (ftruncate(fd, FILE_SIZE) == -1) {
+		pr_perror("Unable to truncate test_file");
+		return 1;
+	}
+
+	/*
+	 * Generate FILE_SIZE locks. We are going to exceed the default
+	 * size of seq buffer
+	 */
+	for (i = 0; i < FILE_SIZE; i++) {
+		struct flock lock;
+
+		if (i % 2)
+			lock.l_type = F_WRLCK;
+		else
+			lock.l_type = F_RDLCK;
+		lock.l_whence = SEEK_SET;
+		lock.l_start  = i;
+		lock.l_len    = 1;
+		lock.l_pid    = -1;
+		if (fcntl(fd, F_SETLK, &lock)) {
+			pr_perror("Unable to set lock %d\n", i);
+			return 1;
+		}
+	}
+
+	snprintf(fdinfo_path, sizeof(fdinfo_path), "/proc/self/fdinfo/%d", fd);
+	fdinfo = open(fdinfo_path, O_RDONLY);
+	if (fdinfo < 0) {
+		pr_perror("Unable to open %s", fdinfo_path);
+		return 1;
+	}
+
+	buf = malloc(bsize);
+	if (buf == NULL) {
+		pr_perror("Unable to allocate a buffer");
+		return 1;
+	}
+	size = 0;
+	while (1) {
+		ret = read(fdinfo, buf + size, bsize - 1 - size);
+		if (ret == 0)
+			break;
+		if (ret == -1) {
+			pr_perror("Unable to read %s", fdinfo_path);
+			return 1;
+		}
+		size += ret;
+		if (bsize - size < 4096)
+			bsize += 4096;
+		buf = realloc(buf, bsize);
+		if (buf == NULL) {
+			pr_perror("Unable to allocate a buffer");
+			return 1;
+		}
+	}
+	buf[size] = 0;
+
+	i = 0;
+	for (p = buf - 1; p != NULL; p = strchr(p, '\n')) {
+		char fl_flag[10], fl_type[15], fl_option[10], end[32];
+		int fl_id, fl_owner, maj, min;
+		unsigned long ino;
+		unsigned long long start;
+
+		p++;
+
+		if (strncmp(p, "lock:", 5))
+			continue;
+		ret = sscanf(p, "lock:\t%d:%s %s %s %d %x:%x:%ld %lld %s",
+				&fl_id, fl_flag, fl_type, fl_option,
+				&fl_owner, &maj, &min, &ino,
+				&start, end);
+		if (ret != 10) {
+			pr_perror("Unable to parse");
+			fprintf(stderr, "%s\n", buf);
+			return 1;
+		}
+		i++;
+	}
+
+	close(fdinfo);
+	close(fd);
+
+	if (i == FILE_SIZE)
+		printf("PASS\n");
+	else {
+		fprintf(stderr, "%s\n", buf);
+		return 1;
+	}
+
+	free(buf);
+
+	return 0;
+}
-- 
2.1.0

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrew Morton]

On Thu, 12 Mar 2015 18:54:42 +0300 Andrew Vagin <avagin@parallels.com> wrote:

> v2: use seq_has_overflowed() properly

--- a/fs/proc/fd.c~proc-show-locks-in-proc-pid-fdinfo-x-v2
+++ a/fs/proc/fd.c
@@ -57,17 +57,15 @@ static int seq_show(struct seq_file *m,
 		   real_mount(file->f_path.mnt)->mnt_id);
 
 	show_fd_locks(m, file, files);
-	ret = seq_has_overflowed(m);
-	if (ret)
+	if (seq_has_overflowed(m))
 		goto out;
 
 	if (file->f_op->show_fdinfo)
 		file->f_op->show_fdinfo(m, file);
-	ret = seq_has_overflowed(m);
 
 out:
 	fput(file);
-	return ret;
+	return 0;
 }
 
 static int seq_fdinfo_open(struct inode *inode, struct file *file)


So it returns "success" when the output has overflowed?  Why this,
rather than returning an error?

Re: [PATCH] selftest: add a test case to check how locks are shown in fdinfo

[Shuah Khan]

Hi Andrey,

Looks good in general. Couple of comments.

On 03/12/2015 10:30 AM, Andrey Vagin wrote:
> The main idea of this test is to check that locks are shown correctly
> when they can't be placed in a default seq_file buffer due to its size.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
>  tools/testing/selftests/Makefile        |   1 +
>  tools/testing/selftests/fdinfo/Makefile |  11 +++
>  tools/testing/selftests/fdinfo/locks.c  | 119 ++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
>  create mode 100644 tools/testing/selftests/fdinfo/Makefile
>  create mode 100644 tools/testing/selftests/fdinfo/locks.c
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 4e51122..8cd57f6 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -17,6 +17,7 @@ TARGETS += sysctl
>  TARGETS += timers
>  TARGETS += user
>  TARGETS += vm
> +TARGETS += fdinfo
>  #Please keep the TARGETS list alphabetically sorted

Please move the new target up to keep the TARGETS list
alphabetically sorted. This helps avoid conflicts as new
tests get added.

>  
>  TARGETS_HOTPLUG = cpu-hotplug
> diff --git a/tools/testing/selftests/fdinfo/Makefile b/tools/testing/selftests/fdinfo/Makefile
> new file mode 100644
> index 0000000..83f34ef
> --- /dev/null
> +++ b/tools/testing/selftests/fdinfo/Makefile
> @@ -0,0 +1,11 @@
> +CFLAGS += -Wall
> +
> +all: locks
> +
> +run_tests: all
> +	@./locks || echo "locks: [FAIL]"
> +
> +locks: locks.c
> +
> +clean:
> +	rm -f locks
> diff --git a/tools/testing/selftests/fdinfo/locks.c b/tools/testing/selftests/fdinfo/locks.c
> new file mode 100644
> index 0000000..93f25c6
> --- /dev/null
> +++ b/tools/testing/selftests/fdinfo/locks.c
> @@ -0,0 +1,119 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#define pr_perror(fmt, ...) fprintf(stderr, "%s:%d: " fmt ": %m\n", \
> +					__FILE__, __LINE__, ##__VA_ARGS__)
> +
> +#define FILE_SIZE 4096
> +
> +int main(int argc, char **argv)
> +{
> +	int fd, fdinfo, i, ret, size, bsize = 4096;
> +	char *buf, *p, fdinfo_path[] = "/proc/self/fdinfo/XXXXXXXXXX";
> +
> +	fd = open("test_file", O_RDWR | O_CREAT, 0666);
> +	if (fd == -1) {
> +		pr_perror("Unable to open test_file");
> +		return 1;
> +	}
> +	unlink("test_file");
> +	if (ftruncate(fd, FILE_SIZE) == -1) {
> +		pr_perror("Unable to truncate test_file");
> +		return 1;
> +	}
> +
> +	/*
> +	 * Generate FILE_SIZE locks. We are going to exceed the default
> +	 * size of seq buffer
> +	 */
> +	for (i = 0; i < FILE_SIZE; i++) {
> +		struct flock lock;
> +
> +		if (i % 2)
> +			lock.l_type = F_WRLCK;
> +		else
> +			lock.l_type = F_RDLCK;
> +		lock.l_whence = SEEK_SET;
> +		lock.l_start  = i;
> +		lock.l_len    = 1;
> +		lock.l_pid    = -1;
> +		if (fcntl(fd, F_SETLK, &lock)) {
> +			pr_perror("Unable to set lock %d\n", i);
> +			return 1;
> +		}
> +	}
> +
> +	snprintf(fdinfo_path, sizeof(fdinfo_path), "/proc/self/fdinfo/%d", fd);
> +	fdinfo = open(fdinfo_path, O_RDONLY);
> +	if (fdinfo < 0) {
> +		pr_perror("Unable to open %s", fdinfo_path);
> +		return 1;
> +	}
> +
> +	buf = malloc(bsize);
> +	if (buf == NULL) {
> +		pr_perror("Unable to allocate a buffer");
> +		return 1;
> +	}
> +	size = 0;
> +	while (1) {
> +		ret = read(fdinfo, buf + size, bsize - 1 - size);
> +		if (ret == 0)
> +			break;
> +		if (ret == -1) {
> +			pr_perror("Unable to read %s", fdinfo_path);
> +			return 1;
> +		}
> +		size += ret;
> +		if (bsize - size < 4096)
> +			bsize += 4096;
> +		buf = realloc(buf, bsize);
> +		if (buf == NULL) {
> +			pr_perror("Unable to allocate a buffer");
> +			return 1;
> +		}
> +	}
> +	buf[size] = 0;
> +
> +	i = 0;
> +	for (p = buf - 1; p != NULL; p = strchr(p, '\n')) {
> +		char fl_flag[10], fl_type[15], fl_option[10], end[32];
> +		int fl_id, fl_owner, maj, min;
> +		unsigned long ino;
> +		unsigned long long start;
> +
> +		p++;
> +
> +		if (strncmp(p, "lock:", 5))
> +			continue;
> +		ret = sscanf(p, "lock:\t%d:%s %s %s %d %x:%x:%ld %lld %s",
> +				&fl_id, fl_flag, fl_type, fl_option,
> +				&fl_owner, &maj, &min, &ino,
> +				&start, end);
> +		if (ret != 10) {
> +			pr_perror("Unable to parse");
> +			fprintf(stderr, "%s\n", buf);
> +			return 1;
> +		}
> +		i++;
> +	}
> +
> +	close(fdinfo);
> +	close(fd);
> +
> +	if (i == FILE_SIZE)
> +		printf("PASS\n");

Look into using ksft framework for reporting pass/fail conditions.
Please see kselftest.h for the framework.

> +	else {
> +		fprintf(stderr, "%s\n", buf);
> +		return 1;
> +	}
> +
> +	free(buf);
> +
> +	return 0;
> +}
> 

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: [PATCH] proc: show locks in /proc/pid/fdinfo/X

[Andrey Wagin]

2015-03-12 22:23 GMT+03:00 Andrew Morton <akpm@linux-foundation.org>:
> On Thu, 12 Mar 2015 18:54:42 +0300 Andrew Vagin <avagin@parallels.com> wrote:
>
>> v2: use seq_has_overflowed() properly
>
> --- a/fs/proc/fd.c~proc-show-locks-in-proc-pid-fdinfo-x-v2
> +++ a/fs/proc/fd.c
> @@ -57,17 +57,15 @@ static int seq_show(struct seq_file *m,
>                    real_mount(file->f_path.mnt)->mnt_id);
>
>         show_fd_locks(m, file, files);
> -       ret = seq_has_overflowed(m);
> -       if (ret)
> +       if (seq_has_overflowed(m))
>                 goto out;
>
>         if (file->f_op->show_fdinfo)
>                 file->f_op->show_fdinfo(m, file);
> -       ret = seq_has_overflowed(m);
>
>  out:
>         fput(file);
> -       return ret;
> +       return 0;
>  }
>
>  static int seq_fdinfo_open(struct inode *inode, struct file *file)
>
>
> So it returns "success" when the output has overflowed?  Why this,
> rather than returning an error?

I have read fs/seq_file.c and looks like it's the right way.
seq_has_overflowed() is used to avoid useless work. If we call it or
don't call it, the result must be the same in both cases. So from this
point of view it looks logically correct too.

There are two interesting places from seq_file.c
                error = m->op->show(m, p);
                if (error < 0)
                        break;
                if (unlikely(error)) {
                        error = 0;
                        m->count = 0;
                }
                if (seq_has_overflowed(m))
                        goto Eoverflow;

...
                err = m->op->show(m, p);
                if (err < 0)
                        break;
                if (unlikely(err))
                        m->count = 0;
                if (unlikely(!m->count)) {
                        p = m->op->next(m, p, &pos);
                        m->index = pos;
                        continue;
                }
                if (m->count < m->size)
                        goto Fill;
                m->op->stop(m, p);
                kvfree(m->buf);
                m->count = 0;
                m->buf = seq_buf_alloc(m->size <<= 1);

Thanks,
Andrey

Re: [PATCH] selftest: add a test case to check how locks are shown in fdinfo

[Andrew Vagin]

[-- Attachment #1: Type: text/plain, Size: 5445 bytes --]

On Thu, Mar 12, 2015 at 02:43:19PM -0600, Shuah Khan wrote:
> Hi Andrey,
> 
> Looks good in general. Couple of comments.

Thanks. The updated version is attached.

> 
> On 03/12/2015 10:30 AM, Andrey Vagin wrote:
> > The main idea of this test is to check that locks are shown correctly
> > when they can't be placed in a default seq_file buffer due to its size.
> > 
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Shuah Khan <shuahkh@osg.samsung.com>
> > Signed-off-by: Andrey Vagin <avagin@openvz.org>
> > ---
> >  tools/testing/selftests/Makefile        |   1 +
> >  tools/testing/selftests/fdinfo/Makefile |  11 +++
> >  tools/testing/selftests/fdinfo/locks.c  | 119 ++++++++++++++++++++++++++++++++
> >  3 files changed, 131 insertions(+)
> >  create mode 100644 tools/testing/selftests/fdinfo/Makefile
> >  create mode 100644 tools/testing/selftests/fdinfo/locks.c
> > 
> > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> > index 4e51122..8cd57f6 100644
> > --- a/tools/testing/selftests/Makefile
> > +++ b/tools/testing/selftests/Makefile
> > @@ -17,6 +17,7 @@ TARGETS += sysctl
> >  TARGETS += timers
> >  TARGETS += user
> >  TARGETS += vm
> > +TARGETS += fdinfo
> >  #Please keep the TARGETS list alphabetically sorted
> 
> Please move the new target up to keep the TARGETS list
> alphabetically sorted. This helps avoid conflicts as new
> tests get added.
> 
> >  
> >  TARGETS_HOTPLUG = cpu-hotplug
> > diff --git a/tools/testing/selftests/fdinfo/Makefile b/tools/testing/selftests/fdinfo/Makefile
> > new file mode 100644
> > index 0000000..83f34ef
> > --- /dev/null
> > +++ b/tools/testing/selftests/fdinfo/Makefile
> > @@ -0,0 +1,11 @@
> > +CFLAGS += -Wall
> > +
> > +all: locks
> > +
> > +run_tests: all
> > +	@./locks || echo "locks: [FAIL]"
> > +
> > +locks: locks.c
> > +
> > +clean:
> > +	rm -f locks
> > diff --git a/tools/testing/selftests/fdinfo/locks.c b/tools/testing/selftests/fdinfo/locks.c
> > new file mode 100644
> > index 0000000..93f25c6
> > --- /dev/null
> > +++ b/tools/testing/selftests/fdinfo/locks.c
> > @@ -0,0 +1,119 @@
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#define pr_perror(fmt, ...) fprintf(stderr, "%s:%d: " fmt ": %m\n", \
> > +					__FILE__, __LINE__, ##__VA_ARGS__)
> > +
> > +#define FILE_SIZE 4096
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	int fd, fdinfo, i, ret, size, bsize = 4096;
> > +	char *buf, *p, fdinfo_path[] = "/proc/self/fdinfo/XXXXXXXXXX";
> > +
> > +	fd = open("test_file", O_RDWR | O_CREAT, 0666);
> > +	if (fd == -1) {
> > +		pr_perror("Unable to open test_file");
> > +		return 1;
> > +	}
> > +	unlink("test_file");
> > +	if (ftruncate(fd, FILE_SIZE) == -1) {
> > +		pr_perror("Unable to truncate test_file");
> > +		return 1;
> > +	}
> > +
> > +	/*
> > +	 * Generate FILE_SIZE locks. We are going to exceed the default
> > +	 * size of seq buffer
> > +	 */
> > +	for (i = 0; i < FILE_SIZE; i++) {
> > +		struct flock lock;
> > +
> > +		if (i % 2)
> > +			lock.l_type = F_WRLCK;
> > +		else
> > +			lock.l_type = F_RDLCK;
> > +		lock.l_whence = SEEK_SET;
> > +		lock.l_start  = i;
> > +		lock.l_len    = 1;
> > +		lock.l_pid    = -1;
> > +		if (fcntl(fd, F_SETLK, &lock)) {
> > +			pr_perror("Unable to set lock %d\n", i);
> > +			return 1;
> > +		}
> > +	}
> > +
> > +	snprintf(fdinfo_path, sizeof(fdinfo_path), "/proc/self/fdinfo/%d", fd);
> > +	fdinfo = open(fdinfo_path, O_RDONLY);
> > +	if (fdinfo < 0) {
> > +		pr_perror("Unable to open %s", fdinfo_path);
> > +		return 1;
> > +	}
> > +
> > +	buf = malloc(bsize);
> > +	if (buf == NULL) {
> > +		pr_perror("Unable to allocate a buffer");
> > +		return 1;
> > +	}
> > +	size = 0;
> > +	while (1) {
> > +		ret = read(fdinfo, buf + size, bsize - 1 - size);
> > +		if (ret == 0)
> > +			break;
> > +		if (ret == -1) {
> > +			pr_perror("Unable to read %s", fdinfo_path);
> > +			return 1;
> > +		}
> > +		size += ret;
> > +		if (bsize - size < 4096)
> > +			bsize += 4096;
> > +		buf = realloc(buf, bsize);
> > +		if (buf == NULL) {
> > +			pr_perror("Unable to allocate a buffer");
> > +			return 1;
> > +		}
> > +	}
> > +	buf[size] = 0;
> > +
> > +	i = 0;
> > +	for (p = buf - 1; p != NULL; p = strchr(p, '\n')) {
> > +		char fl_flag[10], fl_type[15], fl_option[10], end[32];
> > +		int fl_id, fl_owner, maj, min;
> > +		unsigned long ino;
> > +		unsigned long long start;
> > +
> > +		p++;
> > +
> > +		if (strncmp(p, "lock:", 5))
> > +			continue;
> > +		ret = sscanf(p, "lock:\t%d:%s %s %s %d %x:%x:%ld %lld %s",
> > +				&fl_id, fl_flag, fl_type, fl_option,
> > +				&fl_owner, &maj, &min, &ino,
> > +				&start, end);
> > +		if (ret != 10) {
> > +			pr_perror("Unable to parse");
> > +			fprintf(stderr, "%s\n", buf);
> > +			return 1;
> > +		}
> > +		i++;
> > +	}
> > +
> > +	close(fdinfo);
> > +	close(fd);
> > +
> > +	if (i == FILE_SIZE)
> > +		printf("PASS\n");
> 
> Look into using ksft framework for reporting pass/fail conditions.
> Please see kselftest.h for the framework.
> 
> > +	else {
> > +		fprintf(stderr, "%s\n", buf);
> > +		return 1;
> > +	}
> > +
> > +	free(buf);
> > +
> > +	return 0;
> > +}
> > 
> 
> thanks,
> -- Shuah
> 
> -- 
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978

[-- Attachment #2: 0001-selftest-add-a-test-case-to-check-how-locks-are-show.patch --]
[-- Type: text/plain, Size: 4519 bytes --]

>From 2b8180d6ef2be57c0c303ef422fbca40833142f3 Mon Sep 17 00:00:00 2001
From: Andrey Vagin <avagin@openvz.org>
Date: Thu, 12 Mar 2015 15:28:55 +0300
Subject: [PATCH] selftest: add a test case to check how locks are shown in
 fdinfo (v2)

The main idea of this test is to check that locks are shown correctly
when they can't be placed in a default seq_file buffer due to its size.

V2: use the kselftest framework

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 tools/testing/selftests/Makefile        |   1 +
 tools/testing/selftests/fdinfo/Makefile |  11 +++
 tools/testing/selftests/fdinfo/locks.c  | 119 ++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/fdinfo/Makefile
 create mode 100644 tools/testing/selftests/fdinfo/locks.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 4e51122..061a507 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -2,6 +2,7 @@ TARGETS = breakpoints
 TARGETS += cpu-hotplug
 TARGETS += efivarfs
 TARGETS += exec
+TARGETS += fdinfo
 TARGETS += firmware
 TARGETS += ftrace
 TARGETS += kcmp
diff --git a/tools/testing/selftests/fdinfo/Makefile b/tools/testing/selftests/fdinfo/Makefile
new file mode 100644
index 0000000..83f34ef
--- /dev/null
+++ b/tools/testing/selftests/fdinfo/Makefile
@@ -0,0 +1,11 @@
+CFLAGS += -Wall
+
+all: locks
+
+run_tests: all
+	@./locks || echo "locks: [FAIL]"
+
+locks: locks.c
+
+clean:
+	rm -f locks
diff --git a/tools/testing/selftests/fdinfo/locks.c b/tools/testing/selftests/fdinfo/locks.c
new file mode 100644
index 0000000..71fc237
--- /dev/null
+++ b/tools/testing/selftests/fdinfo/locks.c
@@ -0,0 +1,119 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include "../kselftest.h"
+
+#define pr_perror(fmt, ...) fprintf(stderr, "%s:%d: " fmt ": %m\n", \
+					__FILE__, __LINE__, ##__VA_ARGS__)
+
+#define FILE_SIZE 4096
+
+int main(int argc, char **argv)
+{
+	int fd, fdinfo, i, ret, size, bsize = 4096;
+	char *buf, *p, fdinfo_path[] = "/proc/self/fdinfo/XXXXXXXXXX";
+
+	fd = open("test_file", O_RDWR | O_CREAT, 0666);
+	if (fd == -1) {
+		pr_perror("Unable to open test_file");
+		return ksft_exit_fail();
+	}
+	unlink("test_file");
+	if (ftruncate(fd, FILE_SIZE) == -1) {
+		pr_perror("Unable to truncate test_file");
+		return ksft_exit_fail();
+	}
+
+	/*
+	 * Generate FILE_SIZE locks. We are going to exceed the default
+	 * size of seq buffer
+	 */
+	for (i = 0; i < FILE_SIZE; i++) {
+		struct flock lock;
+
+		if (i % 2)
+			lock.l_type = F_WRLCK;
+		else
+			lock.l_type = F_RDLCK;
+		lock.l_whence = SEEK_SET;
+		lock.l_start  = i;
+		lock.l_len    = 1;
+		lock.l_pid    = -1;
+		if (fcntl(fd, F_SETLK, &lock)) {
+			pr_perror("Unable to set lock %d\n", i);
+			return ksft_exit_fail();
+		}
+	}
+
+	snprintf(fdinfo_path, sizeof(fdinfo_path), "/proc/self/fdinfo/%d", fd);
+	fdinfo = open(fdinfo_path, O_RDONLY);
+	if (fdinfo < 0) {
+		pr_perror("Unable to open %s", fdinfo_path);
+		return ksft_exit_fail();
+	}
+
+	buf = malloc(bsize);
+	if (buf == NULL) {
+		pr_perror("Unable to allocate a buffer");
+		return ksft_exit_fail();
+	}
+	size = 0;
+	while (1) {
+		ret = read(fdinfo, buf + size, bsize - 1 - size);
+		if (ret == 0)
+			break;
+		if (ret == -1) {
+			pr_perror("Unable to read %s", fdinfo_path);
+			return ksft_exit_fail();
+		}
+		size += ret;
+		if (bsize - size < 4096)
+			bsize += 4096;
+		buf = realloc(buf, bsize);
+		if (buf == NULL) {
+			pr_perror("Unable to allocate a buffer");
+			return ksft_exit_fail();
+		}
+	}
+	buf[size] = 0;
+
+	i = 0;
+	for (p = buf - 1; p != NULL; p = strchr(p, '\n')) {
+		char fl_flag[10], fl_type[15], fl_option[10], end[32];
+		int fl_id, fl_owner, maj, min;
+		unsigned long ino;
+		unsigned long long start;
+
+		p++;
+
+		if (strncmp(p, "lock:", 5))
+			continue;
+		ret = sscanf(p, "lock:\t%d:%s %s %s %d %x:%x:%ld %lld %s",
+				&fl_id, fl_flag, fl_type, fl_option,
+				&fl_owner, &maj, &min, &ino,
+				&start, end);
+		if (ret != 10) {
+			pr_perror("Unable to parse");
+			fprintf(stderr, "%s\n", buf);
+			return ksft_exit_fail();
+		}
+		i++;
+	}
+
+	close(fdinfo);
+	close(fd);
+
+	if (i != FILE_SIZE) {
+		fprintf(stderr, "%s\n", buf);
+		return ksft_exit_fail();
+	}
+
+	free(buf);
+
+	return ksft_exit_pass();
+}
-- 
2.1.0

Re: [PATCH] selftest: add a test case to check how locks are shown in fdinfo

[Shuah Khan]

On 03/13/2015 03:34 AM, Andrew Vagin wrote:
> On Thu, Mar 12, 2015 at 02:43:19PM -0600, Shuah Khan wrote:
>> Hi Andrey,
>>
>> Looks good in general. Couple of comments.
> 
> Thanks. The updated version is attached.

Please send a properly marked patch v2. Please don't
send patches as attachments.

thanks,
-- Shuah
> 
>>
>> On 03/12/2015 10:30 AM, Andrey Vagin wrote:
>>> The main idea of this test is to check that locks are shown correctly
>>> when they can't be placed in a default seq_file buffer due to its size.
>>>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>>> Signed-off-by: Andrey Vagin <avagin@openvz.org>
>>> ---
>>>  tools/testing/selftests/Makefile        |   1 +
>>>  tools/testing/selftests/fdinfo/Makefile |  11 +++
>>>  tools/testing/selftests/fdinfo/locks.c  | 119 ++++++++++++++++++++++++++++++++
>>>  3 files changed, 131 insertions(+)
>>>  create mode 100644 tools/testing/selftests/fdinfo/Makefile
>>>  create mode 100644 tools/testing/selftests/fdinfo/locks.c
>>>
>>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>>> index 4e51122..8cd57f6 100644
>>> --- a/tools/testing/selftests/Makefile
>>> +++ b/tools/testing/selftests/Makefile
>>> @@ -17,6 +17,7 @@ TARGETS += sysctl
>>>  TARGETS += timers
>>>  TARGETS += user
>>>  TARGETS += vm
>>> +TARGETS += fdinfo
>>>  #Please keep the TARGETS list alphabetically sorted
>>
>> Please move the new target up to keep the TARGETS list
>> alphabetically sorted. This helps avoid conflicts as new
>> tests get added.
>>
>>>  
>>>  TARGETS_HOTPLUG = cpu-hotplug
>>> diff --git a/tools/testing/selftests/fdinfo/Makefile b/tools/testing/selftests/fdinfo/Makefile
>>> new file mode 100644
>>> index 0000000..83f34ef
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/fdinfo/Makefile
>>> @@ -0,0 +1,11 @@
>>> +CFLAGS += -Wall
>>> +
>>> +all: locks
>>> +
>>> +run_tests: all
>>> +	@./locks || echo "locks: [FAIL]"
>>> +
>>> +locks: locks.c
>>> +
>>> +clean:
>>> +	rm -f locks
>>> diff --git a/tools/testing/selftests/fdinfo/locks.c b/tools/testing/selftests/fdinfo/locks.c
>>> new file mode 100644
>>> index 0000000..93f25c6
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/fdinfo/locks.c
>>> @@ -0,0 +1,119 @@
>>> +#include <stdlib.h>
>>> +#include <stdio.h>
>>> +#include <unistd.h>
>>> +#include <string.h>
>>> +#include <sys/types.h>
>>> +#include <sys/stat.h>
>>> +#include <fcntl.h>
>>> +
>>> +#define pr_perror(fmt, ...) fprintf(stderr, "%s:%d: " fmt ": %m\n", \
>>> +					__FILE__, __LINE__, ##__VA_ARGS__)
>>> +
>>> +#define FILE_SIZE 4096
>>> +
>>> +int main(int argc, char **argv)
>>> +{
>>> +	int fd, fdinfo, i, ret, size, bsize = 4096;
>>> +	char *buf, *p, fdinfo_path[] = "/proc/self/fdinfo/XXXXXXXXXX";
>>> +
>>> +	fd = open("test_file", O_RDWR | O_CREAT, 0666);
>>> +	if (fd == -1) {
>>> +		pr_perror("Unable to open test_file");
>>> +		return 1;
>>> +	}
>>> +	unlink("test_file");
>>> +	if (ftruncate(fd, FILE_SIZE) == -1) {
>>> +		pr_perror("Unable to truncate test_file");
>>> +		return 1;
>>> +	}
>>> +
>>> +	/*
>>> +	 * Generate FILE_SIZE locks. We are going to exceed the default
>>> +	 * size of seq buffer
>>> +	 */
>>> +	for (i = 0; i < FILE_SIZE; i++) {
>>> +		struct flock lock;
>>> +
>>> +		if (i % 2)
>>> +			lock.l_type = F_WRLCK;
>>> +		else
>>> +			lock.l_type = F_RDLCK;
>>> +		lock.l_whence = SEEK_SET;
>>> +		lock.l_start  = i;
>>> +		lock.l_len    = 1;
>>> +		lock.l_pid    = -1;
>>> +		if (fcntl(fd, F_SETLK, &lock)) {
>>> +			pr_perror("Unable to set lock %d\n", i);
>>> +			return 1;
>>> +		}
>>> +	}
>>> +
>>> +	snprintf(fdinfo_path, sizeof(fdinfo_path), "/proc/self/fdinfo/%d", fd);
>>> +	fdinfo = open(fdinfo_path, O_RDONLY);
>>> +	if (fdinfo < 0) {
>>> +		pr_perror("Unable to open %s", fdinfo_path);
>>> +		return 1;
>>> +	}
>>> +
>>> +	buf = malloc(bsize);
>>> +	if (buf == NULL) {
>>> +		pr_perror("Unable to allocate a buffer");
>>> +		return 1;
>>> +	}
>>> +	size = 0;
>>> +	while (1) {
>>> +		ret = read(fdinfo, buf + size, bsize - 1 - size);
>>> +		if (ret == 0)
>>> +			break;
>>> +		if (ret == -1) {
>>> +			pr_perror("Unable to read %s", fdinfo_path);
>>> +			return 1;
>>> +		}
>>> +		size += ret;
>>> +		if (bsize - size < 4096)
>>> +			bsize += 4096;
>>> +		buf = realloc(buf, bsize);
>>> +		if (buf == NULL) {
>>> +			pr_perror("Unable to allocate a buffer");
>>> +			return 1;
>>> +		}
>>> +	}
>>> +	buf[size] = 0;
>>> +
>>> +	i = 0;
>>> +	for (p = buf - 1; p != NULL; p = strchr(p, '\n')) {
>>> +		char fl_flag[10], fl_type[15], fl_option[10], end[32];
>>> +		int fl_id, fl_owner, maj, min;
>>> +		unsigned long ino;
>>> +		unsigned long long start;
>>> +
>>> +		p++;
>>> +
>>> +		if (strncmp(p, "lock:", 5))
>>> +			continue;
>>> +		ret = sscanf(p, "lock:\t%d:%s %s %s %d %x:%x:%ld %lld %s",
>>> +				&fl_id, fl_flag, fl_type, fl_option,
>>> +				&fl_owner, &maj, &min, &ino,
>>> +				&start, end);
>>> +		if (ret != 10) {
>>> +			pr_perror("Unable to parse");
>>> +			fprintf(stderr, "%s\n", buf);
>>> +			return 1;
>>> +		}
>>> +		i++;
>>> +	}
>>> +
>>> +	close(fdinfo);
>>> +	close(fd);
>>> +
>>> +	if (i == FILE_SIZE)
>>> +		printf("PASS\n");
>>
>> Look into using ksft framework for reporting pass/fail conditions.
>> Please see kselftest.h for the framework.
>>
>>> +	else {
>>> +		fprintf(stderr, "%s\n", buf);
>>> +		return 1;
>>> +	}
>>> +
>>> +	free(buf);
>>> +
>>> +	return 0;
>>> +}
>>>
>>
>> thanks,
>> -- Shuah
>>
>> -- 
>> Shuah Khan
>> Sr. Linux Kernel Developer
>> Open Source Innovation Group
>> Samsung Research America (Silicon Valley)
>> shuahkh@osg.samsung.com | (970) 217-8978


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978