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

[Andrew Vagin]

 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@xxxxxxxxxx> 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;
> >  }
>