+ add-file-position-info-to-proc.patch added to -mm tree

+ add-file-position-info-to-proc.patch added to -mm tree

[akpm]

The patch titled
     add file position info to proc
has been added to the -mm tree.  Its filename is
     add-file-position-info-to-proc.patch

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this

------------------------------------------------------
Subject: add file position info to proc
From: Miklos Szeredi <mszeredi@suse.cz>

Add support for finding out the current file position, open flags and
possibly other info in the future.

These new entries are added:

  /proc/PID/fdinfo/FD
  /proc/PID/task/TID/fdinfo/FD

For each fd the information is provided in the following format:

pos:	1234
flags:	0100002

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Cc: Alexey Dobriyan <adobriyan@sw.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/proc/base.c |  132 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 114 insertions(+), 18 deletions(-)

diff -puN fs/proc/base.c~add-file-position-info-to-proc fs/proc/base.c
--- a/fs/proc/base.c~add-file-position-info-to-proc
+++ a/fs/proc/base.c
@@ -1231,7 +1231,10 @@ out:
 	return ~0U;
 }
 
-static int proc_fd_link(struct inode *inode, struct dentry **dentry, struct vfsmount **mnt)
+#define PROC_FDINFO_MAX 64
+
+static int proc_fd_info(struct inode *inode, struct dentry **dentry,
+			struct vfsmount **mnt, char *info)
 {
 	struct task_struct *task = get_proc_task(inode);
 	struct files_struct *files = NULL;
@@ -1250,8 +1253,16 @@ static int proc_fd_link(struct inode *in
 		spin_lock(&files->file_lock);
 		file = fcheck_files(files, fd);
 		if (file) {
-			*mnt = mntget(file->f_path.mnt);
-			*dentry = dget(file->f_path.dentry);
+			if (mnt)
+				*mnt = mntget(file->f_path.mnt);
+			if (dentry)
+				*dentry = dget(file->f_path.dentry);
+			if (info)
+				snprintf(info, PROC_FDINFO_MAX,
+					 "pos:\t%lli\n"
+					 "flags:\t0%o\n",
+					 (long long) file->f_pos,
+					 file->f_flags);
 			spin_unlock(&files->file_lock);
 			put_files_struct(files);
 			return 0;
@@ -1262,6 +1273,12 @@ static int proc_fd_link(struct inode *in
 	return -ENOENT;
 }
 
+static int proc_fd_link(struct inode *inode, struct dentry **dentry,
+			struct vfsmount **mnt)
+{
+	return proc_fd_info(inode, dentry, mnt, NULL);
+}
+
 static int tid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
 	struct inode *inode = dentry->d_inode;
@@ -1357,7 +1374,9 @@ out_iput:
 	goto out;
 }
 
-static struct dentry *proc_lookupfd(struct inode * dir, struct dentry * dentry, struct nameidata *nd)
+static struct dentry *proc_lookupfd_common(struct inode *dir,
+					   struct dentry *dentry,
+					   instantiate_t instantiate)
 {
 	struct task_struct *task = get_proc_task(dir);
 	unsigned fd = name_to_int(dentry);
@@ -1368,23 +1387,15 @@ static struct dentry *proc_lookupfd(stru
 	if (fd == ~0U)
 		goto out;
 
-	result = proc_fd_instantiate(dir, dentry, task, &fd);
+	result = instantiate(dir, dentry, task, &fd);
 out:
 	put_task_struct(task);
 out_no_task:
 	return result;
 }
 
-static int proc_fd_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
-	struct task_struct *task, int fd)
-{
-	char name[PROC_NUMBUF];
-	int len = snprintf(name, sizeof(name), "%d", fd);
-	return proc_fill_cache(filp, dirent, filldir, name, len,
-				proc_fd_instantiate, task, &fd);
-}
-
-static int proc_readfd(struct file * filp, void * dirent, filldir_t filldir)
+static int proc_readfd_common(struct file * filp, void * dirent,
+			      filldir_t filldir, instantiate_t instantiate)
 {
 	struct dentry *dentry = filp->f_path.dentry;
 	struct inode *inode = dentry->d_inode;
@@ -1420,12 +1431,17 @@ static int proc_readfd(struct file * fil
 			for (fd = filp->f_pos-2;
 			     fd < fdt->max_fds;
 			     fd++, filp->f_pos++) {
+				char name[PROC_NUMBUF];
+				int len;
 
 				if (!fcheck_files(files, fd))
 					continue;
 				rcu_read_unlock();
 
-				if (proc_fd_fill_cache(filp, dirent, filldir, p, fd) < 0) {
+				len = snprintf(name, sizeof(name), "%d", fd);
+				if (proc_fill_cache(filp, dirent, filldir,
+						    name, len, instantiate,
+						    p, &fd) < 0) {
 					rcu_read_lock();
 					break;
 				}
@@ -1440,6 +1456,32 @@ out_no_task:
 	return retval;
 }
 
+static struct dentry *proc_lookupfd(struct inode *dir, struct dentry *dentry,
+				    struct nameidata *nd)
+{
+	return proc_lookupfd_common(dir, dentry, proc_fd_instantiate);
+}
+
+static int proc_readfd(struct file *filp, void *dirent, filldir_t filldir)
+{
+	return proc_readfd_common(filp, dirent, filldir, proc_fd_instantiate);
+}
+
+static ssize_t proc_fdinfo_read(struct file *file, char __user *buf,
+				      size_t len, loff_t *ppos)
+{
+	char tmp[PROC_FDINFO_MAX];
+	int err = proc_fd_info(file->f_path.dentry->d_inode, NULL, NULL, tmp);
+	if (!err)
+		err = simple_read_from_buffer(buf, len, ppos, tmp, strlen(tmp));
+	return err;
+}
+
+const struct file_operations proc_fdinfo_file_operations = {
+	.open		= nonseekable_open,
+	.read		= proc_fdinfo_read,
+};
+
 static const struct file_operations proc_fd_operations = {
 	.read		= generic_read_dir,
 	.readdir	= proc_readfd,
@@ -1471,6 +1513,58 @@ static const struct inode_operations pro
 	.setattr	= proc_setattr,
 };
 
+static struct dentry *proc_fdinfo_instantiate(struct inode *dir,
+	struct dentry *dentry, struct task_struct *task, const void *ptr)
+{
+	unsigned fd = *(unsigned *)ptr;
+ 	struct inode *inode;
+ 	struct proc_inode *ei;
+	struct dentry *error = ERR_PTR(-ENOENT);
+
+	inode = proc_pid_make_inode(dir->i_sb, task);
+	if (!inode)
+		goto out;
+	ei = PROC_I(inode);
+	ei->fd = fd;
+	inode->i_mode = S_IFREG | S_IRUSR;
+	inode->i_fop = &proc_fdinfo_file_operations;
+	dentry->d_op = &tid_fd_dentry_operations;
+	d_add(dentry, inode);
+	/* Close the race of the process dying before we return the dentry */
+	if (tid_fd_revalidate(dentry, NULL))
+		error = NULL;
+
+ out:
+	return error;
+}
+
+static struct dentry *proc_lookupfdinfo(struct inode *dir,
+					struct dentry *dentry,
+					struct nameidata *nd)
+{
+	return proc_lookupfd_common(dir, dentry, proc_fdinfo_instantiate);
+}
+
+static int proc_readfdinfo(struct file *filp, void *dirent, filldir_t filldir)
+{
+	return proc_readfd_common(filp, dirent, filldir,
+				  proc_fdinfo_instantiate);
+}
+
+static const struct file_operations proc_fdinfo_operations = {
+	.read		= generic_read_dir,
+	.readdir	= proc_readfdinfo,
+};
+
+/*
+ * proc directories can do almost nothing..
+ */
+static const struct inode_operations proc_fdinfo_inode_operations = {
+	.lookup		= proc_lookupfdinfo,
+	.setattr	= proc_setattr,
+};
+
+
 static struct dentry *proc_pident_instantiate(struct inode *dir,
 	struct dentry *dentry, struct task_struct *task, const void *ptr)
 {
@@ -1881,6 +1975,7 @@ static const struct inode_operations pro
 static const struct pid_entry tgid_base_stuff[] = {
 	DIR("task",       S_IRUGO|S_IXUGO, task),
 	DIR("fd",         S_IRUSR|S_IXUSR, fd),
+	DIR("fdinfo",     S_IRUSR|S_IXUSR, fdinfo),
 	INF("environ",    S_IRUSR, pid_environ),
 	INF("auxv",       S_IRUSR, pid_auxv),
 	INF("status",     S_IRUGO, pid_status),
@@ -2034,7 +2129,7 @@ static struct dentry *proc_pid_instantia
 	inode->i_op = &proc_tgid_base_inode_operations;
 	inode->i_fop = &proc_tgid_base_operations;
 	inode->i_flags|=S_IMMUTABLE;
-	inode->i_nlink = 4;
+	inode->i_nlink = 5;
 #ifdef CONFIG_SECURITY
 	inode->i_nlink += 1;
 #endif
@@ -2164,6 +2259,7 @@ out_no_task:
  */
 static const struct pid_entry tid_base_stuff[] = {
 	DIR("fd",        S_IRUSR|S_IXUSR, fd),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, fdinfo),
 	INF("environ",   S_IRUSR, pid_environ),
 	INF("auxv",      S_IRUSR, pid_auxv),
 	INF("status",    S_IRUGO, pid_status),
@@ -2244,7 +2340,7 @@ static struct dentry *proc_task_instanti
 	inode->i_op = &proc_tid_base_inode_operations;
 	inode->i_fop = &proc_tid_base_operations;
 	inode->i_flags|=S_IMMUTABLE;
-	inode->i_nlink = 3;
+	inode->i_nlink = 4;
 #ifdef CONFIG_SECURITY
 	inode->i_nlink += 1;
 #endif
_

Patches currently in -mm which might be from mszeredi@suse.cz are

shmem-fix-bug-in-shmem_writepage.patch
shmem-dont-release-lock-for-hole-punching.patch
fix-quadratic-behavior-of-shrink_dcache_parent.patch
mm-shrink-parent-dentries-when-shrinking-slab.patch
add-filesystem-subtype-support.patch
add-file-position-info-to-proc.patch
consolidate-generic_writepages-and-mpage_writepages.patch

Recursive ->i_mutex lockdep complaint

[Alexey Dobriyan]

On Mon, Mar 26, 2007 at 11:35:42PM -0800, akpm@linux-foundation.org wrote:
> The patch titled
>      add file position info to proc
> has been added to the -mm tree.  Its filename is
>      add-file-position-info-to-proc.patch

I tried to stress-test it with the following program and script and
lockdep barfs on me reasonably quickly:
--------------------------
#include <sys/types.h>
#include <unistd.h>
#include <stdio.h>

int main(void)
{
	int fd[2];

	printf("%u\n", getpid());
	while (1) {
		pipe(fd);
		close(fd[0]);
		close(fd[1]);
	}
	return 0;
}
----------------------------
#!/bin/sh
while true; do find /proc -type f 2>/dev/null   | \
grep -v '/proc/bus/pci'                         | \
xargs cat >/dev/null 2>/dev/null; done
----------------------------

=============================================
[ INFO: possible recursive locking detected ]
2.6.21-rc5-mm4 #1
---------------------------------------------
find/15348 is trying to acquire lock:
 (&inode->i_mutex){--..}, at: [<c016656e>] pipe_read_fasync+0x22/0x53

but task is already holding lock:
 (&inode->i_mutex){--..}, at: [<c016bdd2>] vfs_readdir+0x41/0x85

other info that might help us debug this:
1 lock held by find/15348:
 #0:  (&inode->i_mutex){--..}, at: [<c016bdd2>] vfs_readdir+0x41/0x85

stack backtrace:
 [<c0131b62>] __lock_acquire+0xbc1/0x1021
 [<c012ec07>] lockdep_init_map+0x31/0x45e
 [<c013202a>] lock_acquire+0x68/0x82
 [<c016656e>] pipe_read_fasync+0x22/0x53
 [<c01ca2d0>] _atomic_dec_and_lock+0x10/0x50
 [<c02e0ef3>] __mutex_lock_slowpath+0x6a/0x2c1
 [<c016656e>] pipe_read_fasync+0x22/0x53
 [<c015d33a>] kmem_cache_free+0xa2/0xd8
 [<c016656e>] pipe_read_fasync+0x22/0x53
 [<c01668f7>] pipe_read_release+0x12/0x24
 [<c0161ee1>] __fput+0x4e/0x12f
 [<c015fa14>] filp_close+0x3e/0x62
 [<c0118d33>] put_files_struct+0xb2/0xe0
 [<c01ce901>] snprintf+0x1f/0x23
 [<c018e812>] proc_readfd_common+0x173/0x286
 [<c018f57c>] proc_fdinfo_instantiate+0x0/0x64
 [<c02e0fe0>] __mutex_lock_slowpath+0x157/0x2c1
 [<c016bbe4>] filldir64+0x0/0xf2
 [<c016bbe4>] filldir64+0x0/0xf2
 [<c018e934>] proc_readfdinfo+0xf/0x13
 [<c018f57c>] proc_fdinfo_instantiate+0x0/0x64
 [<c016bbe4>] filldir64+0x0/0xf2
 [<c016be01>] vfs_readdir+0x70/0x85
 [<c016be7c>] sys_getdents64+0x66/0xa9
 [<c0130a8a>] trace_hardirqs_on+0xbe/0x15d
 [<c0103eaa>] sysenter_past_esp+0x5f/0x99
 =======================


It seems that lockdep is unhappy about ->i_mutex taken in
->release/pipe_read_release()/pipe_read_fasync()
which is triggered from put_files_struct() in proc_readfd_common()

Now checking if giving pipe's i_mutex its own lockdep class with fix
things.

Re: Recursive ->i_mutex lockdep complaint

[Miklos Szeredi]

On Tue, 2007-04-03 at 18:21 +0400, Alexey Dobriyan wrote:
> On Mon, Mar 26, 2007 at 11:35:42PM -0800, akpm@linux-foundation.org wrote:
> > The patch titled
> >      add file position info to proc
> > has been added to the -mm tree.  Its filename is
> >      add-file-position-info-to-proc.patch
> 
> I tried to stress-test it with the following program and script and
> lockdep barfs on me reasonably quickly:

Ugh.  As I see it, this is independent from the above patch, as the same
can happen with /proc/PID/fd.  Or did I miss something?

And it seems quite benign, one of the mutexes is for the proc inode, the
other is for the pipe inode, and hopefully they haven't got anything
else to do with each other.

> It seems that lockdep is unhappy about ->i_mutex taken in
> ->release/pipe_read_release()/pipe_read_fasync()
> which is triggered from put_files_struct() in proc_readfd_common()
> 
> Now checking if giving pipe's i_mutex its own lockdep class with fix
> things.

Should help.  Although I'm wondering if it's worth bothering with, as it
doesn't seem to happen in real life for lockdep users.

Thanks,
Miklos

Re: Recursive ->i_mutex lockdep complaint

[Alexey Dobriyan]

On Tue, Apr 03, 2007 at 05:05:20PM +0200, Miklos Szeredi wrote:
> On Tue, 2007-04-03 at 18:21 +0400, Alexey Dobriyan wrote:
> > On Mon, Mar 26, 2007 at 11:35:42PM -0800, akpm@linux-foundation.org wrote:
> > > The patch titled
> > >      add file position info to proc
> > > has been added to the -mm tree.  Its filename is
> > >      add-file-position-info-to-proc.patch
> >
> > I tried to stress-test it with the following program and script and
> > lockdep barfs on me reasonably quickly:
>
> Ugh.  As I see it, this is independent from the above patch, as the same
> can happen with /proc/PID/fd.  Or did I miss something?

Yeah, it's independent from /proc/*/fdinfo stuff.

> And it seems quite benign, one of the mutexes is for the proc inode, the
> other is for the pipe inode, and hopefully they haven't got anything
> else to do with each other.
>
> > It seems that lockdep is unhappy about ->i_mutex taken in
> > ->release/pipe_read_release()/pipe_read_fasync()
> > which is triggered from put_files_struct() in proc_readfd_common()
> >
> > Now checking if giving pipe's i_mutex its own lockdep class with fix
> > things.
>
> Should help.  Although I'm wondering if it's worth bothering with, as it
> doesn't seem to happen in real life for lockdep users.

The following patch makes lockdep happy but I wonder if it's correct
way.

--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -908,6 +908,8 @@ static struct dentry_operations pipefs_d
 	.d_dname	= pipefs_dname,
 };
 
+static struct lock_class_key pipe_inode_imutex_key;
+
 static struct inode * get_pipe_inode(void)
 {
 	struct inode *inode = new_inode(pipe_mnt->mnt_sb);
@@ -936,6 +938,8 @@ static struct inode * get_pipe_inode(voi
 	inode->i_gid = current->fsgid;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 
+	lockdep_set_class(&inode->i_mutex, &pipe_inode_imutex_key);
+
 	return inode;
 
 fail_iput: