[patch 0/6] Rework file::show_fdinfo method to use seq-files engine

[patch 0/6] Rework file::show_fdinfo method to use seq-files engine

[Cyrill Gorcunov]

Hi, in criu we intensively use additional information provided by
/proc/<pid>/fdinfo/<fd> particular to the object opened file
represents. The output is printed out by file_operations::show_fdinfo
method and the implementation of this method is pretty simple. Still
I think better would be to convert this method to seq_operations pointer
where all suppliers (epoll, eventfd and such) provide own seq-file
operations. While this makes code a little more complex this will
decrease the amount of kernel memory needed for output (because
we won't need to provide all information in one pass but by chunks).

Please take a look, thanks!

[patch 1/6] procfs: Introduce sequential fdinfo engine

[Cyrill Gorcunov]

[-- Attachment #1: 0001-procfs-fdinfo-new-4 --]
[-- Type: text/plain, Size: 6389 bytes --]

At moment the fdinfo operations (ie the output from /proc/$pid/fdinfo/$fd)
are generating output in one pass, which makes useless memory pressue
if the reader/user provides a buffer with a small size.

Instead, provide proc_fdinfo structure where each file hook own seq_operations
for fdinfo output thus memory won't be needlessly allocated if the recepient
buffer is small or the reader is seeking some particular position.

Once all callers to fdinfo are converted (addressed in next patches)
the former @show_fdinfo will be deleted.

Note the patch doesn't change current output it's rather preparing
the ground for next patches.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/proc/fd.c       |  162 ++++++++++++++++++++++++++++++++++++++++++-----------
 include/linux/fs.h |    1 
 2 files changed, 130 insertions(+), 33 deletions(-)

Index: linux-2.6.git/fs/proc/fd.c
===================================================================
--- linux-2.6.git.orig/fs/proc/fd.c
+++ linux-2.6.git/fs/proc/fd.c
@@ -14,60 +14,156 @@
 #include "internal.h"
 #include "fd.h"
 
+struct proc_fdinfo {
+	struct seq_file		seq;		/* Make sure it's first */
+	struct seq_operations	*fdinfo_ops;
+	loff_t			pos;
+	struct file		*f_file;
+	unsigned int		f_flags;
+};
+
+#define seq_info(m)	(container_of(m, struct proc_fdinfo, seq))
+
+static void *seq_start(struct seq_file *m, loff_t *pos)
+{
+	struct proc_fdinfo *info = seq_info(m);
+
+	info->pos = *pos;
+	if (*pos == 0)
+		return info;
+	return info->fdinfo_ops ? info->fdinfo_ops->start(m, pos) : NULL;
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+	struct proc_fdinfo *info = seq_info(m);
+
+	if (info->pos > 0 && info->fdinfo_ops)
+		info->fdinfo_ops->stop(m, v);
+}
+
+static void *seq_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	struct proc_fdinfo *info = seq_info(m);
+	void *v = NULL;
+
+	if (info->fdinfo_ops) {
+		int ret = 0;
+
+		if (*pos == 0) {
+			v = info->fdinfo_ops->start(m, pos);
+			if (v) {
+				ret = info->fdinfo_ops->show(m, v);
+				p = v;
+			} else
+				ret = -1;
+		}
+
+		if (!ret)
+			v = info->fdinfo_ops->next(m, p, pos);
+		else
+			++*pos;
+	} else
+		++*pos;
+
+	info->pos = *pos;
+	return v;
+}
+
 static int seq_show(struct seq_file *m, void *v)
 {
+	struct proc_fdinfo *info = seq_info(m);
+
+	if (info->fdinfo_ops && info->pos > 0)
+		return info->fdinfo_ops->show(m, v);
+
+	seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
+		   (long long)info->f_file->f_pos, info->f_flags);
+
+	/* Legacy interface */
+	if (info->f_file->f_op->show_fdinfo)
+		return info->f_file->f_op->show_fdinfo(m, info->f_file);
+
+	return 0;
+}
+
+static const struct seq_operations fdinfo_seq_ops = {
+	.start	= seq_start,
+	.stop	= seq_stop,
+	.next	= seq_next,
+	.show	= seq_show,
+};
+
+static int seq_fdinfo_open(struct inode *inode, struct file *file)
+{
 	struct files_struct *files = NULL;
-	int f_flags = 0, ret = -ENOENT;
-	struct file *file = NULL;
+	struct proc_fdinfo *info;
 	struct task_struct *task;
+	int ret;
 
-	task = get_proc_task(m->private);
-	if (!task)
-		return -ENOENT;
-
-	files = get_files_struct(task);
-	put_task_struct(task);
-
-	if (files) {
-		int fd = proc_fd(m->private);
-
-		spin_lock(&files->file_lock);
-		file = fcheck_files(files, fd);
-		if (file) {
-			struct fdtable *fdt = files_fdtable(files);
-
-			f_flags = file->f_flags;
-			if (close_on_exec(fd, fdt))
-				f_flags |= O_CLOEXEC;
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	file->private_data = &info->seq;
 
-			get_file(file);
-			ret = 0;
+	ret = seq_open(file, &fdinfo_seq_ops);
+	if (!ret) {
+		ret = -ENOENT;
+
+		task = get_proc_task(inode);
+		if (task) {
+			files = get_files_struct(task);
+			put_task_struct(task);
+		}
+
+		if (files) {
+			struct seq_file *m = file->private_data;
+			int fd = proc_fd(inode);
+
+			spin_lock(&files->file_lock);
+			info->f_file = fcheck_files(files, fd);
+			if (info->f_file) {
+				struct fdtable *fdt = files_fdtable(files);
+
+				info->f_flags = info->f_file->f_flags & ~O_CLOEXEC;
+				if (close_on_exec(fd, fdt))
+					info->f_flags |= O_CLOEXEC;
+
+				info->fdinfo_ops = info->f_file->f_op->fdinfo_ops;
+				m->private = info->f_file;
+
+				get_file(info->f_file);
+				ret = 0;
+			}
+			spin_unlock(&files->file_lock);
+			put_files_struct(files);
 		}
-		spin_unlock(&files->file_lock);
-		put_files_struct(files);
 	}
 
-	if (!ret) {
-                seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
-			   (long long)file->f_pos, f_flags);
-		if (file->f_op->show_fdinfo)
-			ret = file->f_op->show_fdinfo(m, file);
-		fput(file);
+	if (ret) {
+		if (info->f_file)
+			fput(info->f_file);
+		file->private_data = NULL;
+		kfree(info);
 	}
 
 	return ret;
 }
 
-static int seq_fdinfo_open(struct inode *inode, struct file *file)
+static int seq_fdinfo_release(struct inode *inode, struct file *file)
 {
-	return single_open(file, seq_show, inode);
+	struct seq_file *m = file->private_data;
+	struct proc_fdinfo *info = seq_info(m);
+
+	fput(info->f_file);
+	return seq_release(inode, file);
 }
 
 static const struct file_operations proc_fdinfo_file_operations = {
 	.open		= seq_fdinfo_open,
 	.read		= seq_read,
 	.llseek		= seq_lseek,
-	.release	= single_release,
+	.release	= seq_fdinfo_release,
 };
 
 static int tid_fd_revalidate(struct dentry *dentry, unsigned int flags)
Index: linux-2.6.git/include/linux/fs.h
===================================================================
--- linux-2.6.git.orig/include/linux/fs.h
+++ linux-2.6.git/include/linux/fs.h
@@ -1552,6 +1552,7 @@ struct file_operations {
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
 	int (*show_fdinfo)(struct seq_file *m, struct file *f);
+	struct seq_operations *fdinfo_ops;
 };
 
 struct inode_operations {

[patch 2/6] epoll: Use sequential fdinfo engine

[Cyrill Gorcunov]

[-- Attachment #1: 0002-procfs-fdinfo-epoll --]
[-- Type: text/plain, Size: 2645 bytes --]

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/eventpoll.c |   45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

Index: linux-2.6.git/fs/eventpoll.c
===================================================================
--- linux-2.6.git.orig/fs/eventpoll.c
+++ linux-2.6.git/fs/eventpoll.c
@@ -834,32 +834,53 @@ static unsigned int ep_eventpoll_poll(st
 }
 
 #ifdef CONFIG_PROC_FS
-static int ep_show_fdinfo(struct seq_file *m, struct file *f)
+static void *seq_start(struct seq_file *m, loff_t *pos)
 {
-	struct eventpoll *ep = f->private_data;
+	struct eventpoll *ep = ((struct file *)m->private)->private_data;
 	struct rb_node *rbp;
-	int ret = 0;
+	loff_t num = *pos;
 
 	mutex_lock(&ep->mtx);
 	for (rbp = rb_first(&ep->rbr); rbp; rbp = rb_next(rbp)) {
-		struct epitem *epi = rb_entry(rbp, struct epitem, rbn);
-
-		ret = seq_printf(m, "tfd: %8d events: %8x data: %16llx\n",
-				 epi->ffd.fd, epi->event.events,
-				 (long long)epi->event.data);
-		if (ret)
-			break;
+		if (num-- == 0)
+			return rbp;
 	}
+	return NULL;
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+	struct eventpoll *ep = ((struct file *)m->private)->private_data;
 	mutex_unlock(&ep->mtx);
+}
 
-	return ret;
+static void *seq_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	++*pos;
+	return (void *)rb_next(p);
 }
+
+static int seq_show(struct seq_file *m, void *v)
+{
+	struct epitem *epi = rb_entry(v, struct epitem, rbn);
+
+	return seq_printf(m, "tfd: %8d events: %8x data: %16llx\n",
+			  epi->ffd.fd, epi->event.events,
+			  (long long)epi->event.data);
+}
+
+static struct seq_operations ep_fdinfo_ops = {
+	.start		= seq_start,
+	.next		= seq_next,
+	.stop		= seq_stop,
+	.show		= seq_show,
+};
 #endif
 
 /* File callbacks that implement the eventpoll file behaviour */
 static const struct file_operations eventpoll_fops = {
 #ifdef CONFIG_PROC_FS
-	.show_fdinfo	= ep_show_fdinfo,
+	.fdinfo_ops	= &ep_fdinfo_ops,
 #endif
 	.release	= ep_eventpoll_release,
 	.poll		= ep_eventpoll_poll,

[patch 3/6] eventfd: Use sequential fdinfo engine

[Cyrill Gorcunov]

[-- Attachment #1: 0003-procfs-eventfd --]
[-- Type: text/plain, Size: 2005 bytes --]

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/eventfd.c |   30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

Index: linux-2.6.git/fs/eventfd.c
===================================================================
--- linux-2.6.git.orig/fs/eventfd.c
+++ linux-2.6.git/fs/eventfd.c
@@ -287,9 +287,26 @@ static ssize_t eventfd_write(struct file
 }
 
 #ifdef CONFIG_PROC_FS
-static int eventfd_show_fdinfo(struct seq_file *m, struct file *f)
+static void *seq_start(struct seq_file *m, loff_t *pos)
 {
-	struct eventfd_ctx *ctx = f->private_data;
+	if (*pos == 0)
+		return m->private;
+	return NULL;
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+}
+
+static void *seq_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	++*pos;
+	return NULL;
+}
+
+static int seq_show(struct seq_file *m, void *v)
+{
+	struct eventfd_ctx *ctx = ((struct file *)v)->private_data;
 	int ret;
 
 	spin_lock_irq(&ctx->wqh.lock);
@@ -299,11 +316,18 @@ static int eventfd_show_fdinfo(struct se
 
 	return ret;
 }
+
+static struct seq_operations eventfd_fdinfo_ops = {
+	.start		= seq_start,
+	.next		= seq_next,
+	.stop		= seq_stop,
+	.show		= seq_show,
+};
 #endif
 
 static const struct file_operations eventfd_fops = {
 #ifdef CONFIG_PROC_FS
-	.show_fdinfo	= eventfd_show_fdinfo,
+	.fdinfo_ops	= &eventfd_fdinfo_ops,
 #endif
 	.release	= eventfd_release,
 	.poll		= eventfd_poll,

[patch 4/6] signalfd: Use sequential fdinfo engine

[Cyrill Gorcunov]

[-- Attachment #1: 0004-procfs-signalfd --]
[-- Type: text/plain, Size: 2018 bytes --]

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/signalfd.c |   30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

Index: linux-2.6.git/fs/signalfd.c
===================================================================
--- linux-2.6.git.orig/fs/signalfd.c
+++ linux-2.6.git/fs/signalfd.c
@@ -230,9 +230,26 @@ static ssize_t signalfd_read(struct file
 }
 
 #ifdef CONFIG_PROC_FS
-static int signalfd_show_fdinfo(struct seq_file *m, struct file *f)
+static void *seq_start(struct seq_file *m, loff_t *pos)
 {
-	struct signalfd_ctx *ctx = f->private_data;
+	if (*pos == 0)
+		return m->private;
+	return NULL;
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+}
+
+static void *seq_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	++*pos;
+	return NULL;
+}
+
+static int seq_show(struct seq_file *m, void *v)
+{
+	struct signalfd_ctx *ctx = ((struct file *)v)->private_data;
 	sigset_t sigmask;
 
 	sigmask = ctx->sigmask;
@@ -241,11 +258,18 @@ static int signalfd_show_fdinfo(struct s
 
 	return 0;
 }
+
+static struct seq_operations signalfd_fdinfo_ops = {
+	.start		= seq_start,
+	.next		= seq_next,
+	.stop		= seq_stop,
+	.show		= seq_show,
+};
 #endif
 
 static const struct file_operations signalfd_fops = {
 #ifdef CONFIG_PROC_FS
-	.show_fdinfo	= signalfd_show_fdinfo,
+	.fdinfo_ops	= &signalfd_fdinfo_ops,
 #endif
 	.release	= signalfd_release,
 	.poll		= signalfd_poll,

[patch 5/6] fsnotify: Use sequential fdinfo engine

[Cyrill Gorcunov]

[-- Attachment #1: 0005-procfs-notify --]
[-- Type: text/plain, Size: 7399 bytes --]

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/notify/fanotify/fanotify_user.c |    4 -
 fs/notify/fdinfo.c                 |  121 +++++++++++++++++++++++++------------
 fs/notify/fdinfo.h                 |   12 ---
 fs/notify/inotify/inotify_user.c   |    4 -
 4 files changed, 94 insertions(+), 47 deletions(-)

Index: linux-2.6.git/fs/notify/fanotify/fanotify_user.c
===================================================================
--- linux-2.6.git.orig/fs/notify/fanotify/fanotify_user.c
+++ linux-2.6.git/fs/notify/fanotify/fanotify_user.c
@@ -432,7 +432,9 @@ static long fanotify_ioctl(struct file *
 }
 
 static const struct file_operations fanotify_fops = {
-	.show_fdinfo	= fanotify_show_fdinfo,
+#ifdef CONFIG_PROC_FS
+	.fdinfo_ops	= &fanotify_fdinfo_ops,
+#endif
 	.poll		= fanotify_poll,
 	.read		= fanotify_read,
 	.write		= fanotify_write,
Index: linux-2.6.git/fs/notify/fdinfo.c
===================================================================
--- linux-2.6.git.orig/fs/notify/fdinfo.c
+++ linux-2.6.git/fs/notify/fdinfo.c
@@ -16,25 +16,50 @@
 #include "inotify/inotify.h"
 #include "../fs/mount.h"
 
+#define seq_notify_group(m) ((struct file*)(m)->private)->private_data
+
 #if defined(CONFIG_PROC_FS)
 
 #if defined(CONFIG_INOTIFY_USER) || defined(CONFIG_FANOTIFY)
 
-static int show_fdinfo(struct seq_file *m, struct file *f,
-		       int (*show)(struct seq_file *m, struct fsnotify_mark *mark))
+static void *notify_start(struct seq_file *m, loff_t *pos,
+			  void *(*__start)(struct seq_file *m, loff_t *pos,
+					   struct fsnotify_group *group,
+					   struct fsnotify_mark *mark))
 {
-	struct fsnotify_group *group = f->private_data;
+	struct fsnotify_group *group = seq_notify_group(m);
 	struct fsnotify_mark *mark;
-	int ret = 0;
+	loff_t num = *pos;
 
 	mutex_lock(&group->mark_mutex);
 	list_for_each_entry(mark, &group->marks_list, g_list) {
-		ret = show(m, mark);
-		if (ret)
-			break;
+		if (num-- == 0) {
+			if (__start)
+				return __start(m, pos, group, mark);
+			return mark;
+		}
 	}
+
+	return NULL;
+}
+
+static void notify_stop(struct seq_file *m, void *v)
+{
+	struct fsnotify_group *group = seq_notify_group(m);
 	mutex_unlock(&group->mark_mutex);
-	return ret;
+}
+
+static void *notify_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	struct fsnotify_group *group = seq_notify_group(m);
+	struct fsnotify_mark *mark = p;
+
+	if (!list_is_last(&mark->g_list, &group->marks_list))
+		mark = list_first_entry(&mark->g_list, struct fsnotify_mark, g_list);
+	else
+		mark = NULL;
+	++*pos;
+	return mark;
 }
 
 #if defined(CONFIG_EXPORTFS)
@@ -75,9 +100,15 @@ static int show_mark_fhandle(struct seq_
 
 #ifdef CONFIG_INOTIFY_USER
 
-static int inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
+static void *inotify_start(struct seq_file *m, loff_t *pos)
+{
+	return notify_start(m, pos, NULL);
+}
+
+static int inotify_show(struct seq_file *m, void *v)
 {
 	struct inotify_inode_mark *inode_mark;
+	struct fsnotify_mark *mark = v;
 	struct inode *inode;
 	int ret = 0;
 
@@ -100,17 +131,20 @@ static int inotify_fdinfo(struct seq_fil
 	return ret;
 }
 
-int inotify_show_fdinfo(struct seq_file *m, struct file *f)
-{
-	return show_fdinfo(m, f, inotify_fdinfo);
-}
+struct seq_operations inotify_fdinfo_ops = {
+	.start		= inotify_start,
+	.next		= notify_next,
+	.stop		= notify_stop,
+	.show		= inotify_show,
+};
 
 #endif /* CONFIG_INOTIFY_USER */
 
 #ifdef CONFIG_FANOTIFY
 
-static int fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
+static int fanotify_show(struct seq_file *m, void *v)
 {
+	struct fsnotify_mark *mark = v;
 	unsigned int mflags = 0;
 	struct inode *inode;
 	int ret = 0;
@@ -143,35 +177,50 @@ out:
 	return ret;
 }
 
-int fanotify_show_fdinfo(struct seq_file *m, struct file *f)
-{
-	struct fsnotify_group *group = f->private_data;
-	unsigned int flags = 0;
+static void *__fanotify_start(struct seq_file *m, loff_t *pos,
+			      struct fsnotify_group *group,
+			      struct fsnotify_mark *mark)
+{
+	if (*pos == 0) {
+		unsigned int flags = 0;
+
+		switch (group->priority) {
+		case FS_PRIO_0:
+			flags |= FAN_CLASS_NOTIF;
+			break;
+		case FS_PRIO_1:
+			flags |= FAN_CLASS_CONTENT;
+			break;
+		case FS_PRIO_2:
+			flags |= FAN_CLASS_PRE_CONTENT;
+			break;
+		}
 
-	switch (group->priority) {
-	case FS_PRIO_0:
-		flags |= FAN_CLASS_NOTIF;
-		break;
-	case FS_PRIO_1:
-		flags |= FAN_CLASS_CONTENT;
-		break;
-	case FS_PRIO_2:
-		flags |= FAN_CLASS_PRE_CONTENT;
-		break;
-	}
+		if (group->max_events == UINT_MAX)
+			flags |= FAN_UNLIMITED_QUEUE;
 
-	if (group->max_events == UINT_MAX)
-		flags |= FAN_UNLIMITED_QUEUE;
+		if (group->fanotify_data.max_marks == UINT_MAX)
+			flags |= FAN_UNLIMITED_MARKS;
 
-	if (group->fanotify_data.max_marks == UINT_MAX)
-		flags |= FAN_UNLIMITED_MARKS;
+		seq_printf(m, "fanotify flags:%x event-flags:%x\n",
+			   flags, group->fanotify_data.f_flags);
+	}
 
-	seq_printf(m, "fanotify flags:%x event-flags:%x\n",
-		   flags, group->fanotify_data.f_flags);
+	return mark;
+}
 
-	return show_fdinfo(m, f, fanotify_fdinfo);
+static void *fanotify_start(struct seq_file *m, loff_t *pos)
+{
+	return notify_start(m, pos, __fanotify_start);
 }
 
+struct seq_operations fanotify_fdinfo_ops = {
+	.start		= fanotify_start,
+	.next		= notify_next,
+	.stop		= notify_stop,
+	.show		= fanotify_show,
+};
+
 #endif /* CONFIG_FANOTIFY */
 
 #endif /* CONFIG_INOTIFY_USER || CONFIG_FANOTIFY */
Index: linux-2.6.git/fs/notify/fdinfo.h
===================================================================
--- linux-2.6.git.orig/fs/notify/fdinfo.h
+++ linux-2.6.git/fs/notify/fdinfo.h
@@ -4,24 +4,18 @@
 #include <linux/errno.h>
 #include <linux/proc_fs.h>
 
-struct seq_file;
-struct file;
+struct seq_operations;
 
 #ifdef CONFIG_PROC_FS
 
 #ifdef CONFIG_INOTIFY_USER
-extern int inotify_show_fdinfo(struct seq_file *m, struct file *f);
+extern struct seq_operations inotify_fdinfo_ops;
 #endif
 
 #ifdef CONFIG_FANOTIFY
-extern int fanotify_show_fdinfo(struct seq_file *m, struct file *f);
+extern struct seq_operations fanotify_fdinfo_ops;
 #endif
 
-#else /* CONFIG_PROC_FS */
-
-#define inotify_show_fdinfo	NULL
-#define fanotify_show_fdinfo	NULL
-
 #endif /* CONFIG_PROC_FS */
 
 #endif /* __FSNOTIFY_FDINFO_H__ */
Index: linux-2.6.git/fs/notify/inotify/inotify_user.c
===================================================================
--- linux-2.6.git.orig/fs/notify/inotify/inotify_user.c
+++ linux-2.6.git/fs/notify/inotify/inotify_user.c
@@ -327,7 +327,9 @@ static long inotify_ioctl(struct file *f
 }
 
 static const struct file_operations inotify_fops = {
-	.show_fdinfo	= inotify_show_fdinfo,
+#ifdef CONFIG_PROC_FS
+	.fdinfo_ops	= &inotify_fdinfo_ops,
+#endif
 	.poll		= inotify_poll,
 	.read		= inotify_read,
 	.fasync		= fsnotify_fasync,

[patch 6/6] procfs: Drop legacy show_fdinfo file operation

[Cyrill Gorcunov]

[-- Attachment #1: 0006-procfs-ridof --]
[-- Type: text/plain, Size: 1796 bytes --]

All former users of show_fdinfo are converted to sequential
output engine thus we don't need it anymore.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/proc/fd.c       |    5 -----
 include/linux/fs.h |    1 -
 2 files changed, 6 deletions(-)

Index: linux-2.6.git/fs/proc/fd.c
===================================================================
--- linux-2.6.git.orig/fs/proc/fd.c
+++ linux-2.6.git/fs/proc/fd.c
@@ -79,11 +79,6 @@ static int seq_show(struct seq_file *m,
 
 	seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
 		   (long long)info->f_file->f_pos, info->f_flags);
-
-	/* Legacy interface */
-	if (info->f_file->f_op->show_fdinfo)
-		return info->f_file->f_op->show_fdinfo(m, info->f_file);
-
 	return 0;
 }
 
Index: linux-2.6.git/include/linux/fs.h
===================================================================
--- linux-2.6.git.orig/include/linux/fs.h
+++ linux-2.6.git/include/linux/fs.h
@@ -1551,7 +1551,6 @@ struct file_operations {
 	int (*setlease)(struct file *, long, struct file_lock **);
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
-	int (*show_fdinfo)(struct seq_file *m, struct file *f);
 	struct seq_operations *fdinfo_ops;
 };
 

Re: [patch 1/6] procfs: Introduce sequential fdinfo engine

[Alexey Dobriyan]

On Wed, Oct 30, 2013 at 10:59 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> At moment the fdinfo operations (ie the output from /proc/$pid/fdinfo/$fd)
> are generating output in one pass, which makes useless memory pressue
> if the reader/user provides a buffer with a small size.

cat(1) uses 64 KB buffer.
The output doesn't exceed one page anyway.

Re: [patch 1/6] procfs: Introduce sequential fdinfo engine

[Cyrill Gorcunov]

On Thu, Oct 31, 2013 at 01:32:13PM +0300, Alexey Dobriyan wrote:
> On Wed, Oct 30, 2013 at 10:59 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> > At moment the fdinfo operations (ie the output from /proc/$pid/fdinfo/$fd)
> > are generating output in one pass, which makes useless memory pressue
> > if the reader/user provides a buffer with a small size.
> 
> cat(1) uses 64 KB buffer.
> The output doesn't exceed one page anyway.

Yes, good point. I probably need to update changelog (forgot that i'm using
single_open here). What if we meet a file with that big number of epoll
or say notifies assigned that the fdinfo won't fit a page? I didn't meet
such scenario yet, but I think it's possible?

Re: [patch 1/6] procfs: Introduce sequential fdinfo engine

[Cyrill Gorcunov]

On Thu, Oct 31, 2013 at 02:57:52PM +0400, Cyrill Gorcunov wrote:
> > > At moment the fdinfo operations (ie the output from /proc/$pid/fdinfo/$fd)
> > > are generating output in one pass, which makes useless memory pressue
> > > if the reader/user provides a buffer with a small size.
> > 
> > cat(1) uses 64 KB buffer.
> > The output doesn't exceed one page anyway.
> 
> Yes, good point. I probably need to update changelog (forgot that i'm using
> single_open here). What if we meet a file with that big number of epoll
> or say notifies assigned that the fdinfo won't fit a page? I didn't meet
> such scenario yet, but I think it's possible?

Alexey, would the following changelog sound better? I've been testing the
program which creates 100 pipes, which in turn are watched with epoll
(I don't think someone ever need to do that actully) then tried to read
fdinfo and it failed. Instead if I'm using this series I can see all 100
pipes in the output.
---
From: Cyrill Gorcunov <gorcunov@openvz.org>
Subject: procfs: Introduce sequential fdinfo engine

At moment the fdinfo operations (ie the output from /proc/$pid/fdinfo/$fd)
are generating output in one pass, allocating one page buffer as maximum.
But in case of lots of event polls watchees (100 and more) read procedure
will fail because one page is not enough to provide all information associated
with watch descriptors.

Instead, provide proc_fdinfo structure where each file hook own seq_operations
for fdinfo output and the seq-file engine will be able to traverse over the all
records and provide the reader complete information increasing buffer size if
needed.

Once all callers to fdinfo are converted (addressed in next patches)
the former @show_fdinfo will be deleted.

Note the patch doesn't change current output it's rather preparing
the ground for next patches.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Matthew Helsley <matt.helsley@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/proc/fd.c       |  162 ++++++++++++++++++++++++++++++++++++++++++-----------
 include/linux/fs.h |    1 
 2 files changed, 130 insertions(+), 33 deletions(-)

Index: linux-2.6.git/fs/proc/fd.c
===================================================================
--- linux-2.6.git.orig/fs/proc/fd.c
+++ linux-2.6.git/fs/proc/fd.c
@@ -14,60 +14,156 @@
 #include "internal.h"
 #include "fd.h"
 
+struct proc_fdinfo {
+	struct seq_file		seq;		/* Make sure it's first */
+	struct seq_operations	*fdinfo_ops;
+	loff_t			pos;
+	struct file		*f_file;
+	unsigned int		f_flags;
+};
+
+#define seq_info(m)	(container_of(m, struct proc_fdinfo, seq))
+
+static void *seq_start(struct seq_file *m, loff_t *pos)
+{
+	struct proc_fdinfo *info = seq_info(m);
+
+	info->pos = *pos;
+	if (*pos == 0)
+		return info;
+	return info->fdinfo_ops ? info->fdinfo_ops->start(m, pos) : NULL;
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+	struct proc_fdinfo *info = seq_info(m);
+
+	if (info->pos > 0 && info->fdinfo_ops)
+		info->fdinfo_ops->stop(m, v);
+}
+
+static void *seq_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	struct proc_fdinfo *info = seq_info(m);
+	void *v = NULL;
+
+	if (info->fdinfo_ops) {
+		int ret = 0;
+
+		if (*pos == 0) {
+			v = info->fdinfo_ops->start(m, pos);
+			if (v) {
+				ret = info->fdinfo_ops->show(m, v);
+				p = v;
+			} else
+				ret = -1;
+		}
+
+		if (!ret)
+			v = info->fdinfo_ops->next(m, p, pos);
+		else
+			++*pos;
+	} else
+		++*pos;
+
+	info->pos = *pos;
+	return v;
+}
+
 static int seq_show(struct seq_file *m, void *v)
 {
+	struct proc_fdinfo *info = seq_info(m);
+
+	if (info->fdinfo_ops && info->pos > 0)
+		return info->fdinfo_ops->show(m, v);
+
+	seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
+		   (long long)info->f_file->f_pos, info->f_flags);
+
+	/* Legacy interface */
+	if (info->f_file->f_op->show_fdinfo)
+		return info->f_file->f_op->show_fdinfo(m, info->f_file);
+
+	return 0;
+}
+
+static const struct seq_operations fdinfo_seq_ops = {
+	.start	= seq_start,
+	.stop	= seq_stop,
+	.next	= seq_next,
+	.show	= seq_show,
+};
+
+static int seq_fdinfo_open(struct inode *inode, struct file *file)
+{
 	struct files_struct *files = NULL;
-	int f_flags = 0, ret = -ENOENT;
-	struct file *file = NULL;
+	struct proc_fdinfo *info;
 	struct task_struct *task;
+	int ret;
 
-	task = get_proc_task(m->private);
-	if (!task)
-		return -ENOENT;
-
-	files = get_files_struct(task);
-	put_task_struct(task);
-
-	if (files) {
-		int fd = proc_fd(m->private);
-
-		spin_lock(&files->file_lock);
-		file = fcheck_files(files, fd);
-		if (file) {
-			struct fdtable *fdt = files_fdtable(files);
-
-			f_flags = file->f_flags;
-			if (close_on_exec(fd, fdt))
-				f_flags |= O_CLOEXEC;
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	file->private_data = &info->seq;
 
-			get_file(file);
-			ret = 0;
+	ret = seq_open(file, &fdinfo_seq_ops);
+	if (!ret) {
+		ret = -ENOENT;
+
+		task = get_proc_task(inode);
+		if (task) {
+			files = get_files_struct(task);
+			put_task_struct(task);
+		}
+
+		if (files) {
+			struct seq_file *m = file->private_data;
+			int fd = proc_fd(inode);
+
+			spin_lock(&files->file_lock);
+			info->f_file = fcheck_files(files, fd);
+			if (info->f_file) {
+				struct fdtable *fdt = files_fdtable(files);
+
+				info->f_flags = info->f_file->f_flags & ~O_CLOEXEC;
+				if (close_on_exec(fd, fdt))
+					info->f_flags |= O_CLOEXEC;
+
+				info->fdinfo_ops = info->f_file->f_op->fdinfo_ops;
+				m->private = info->f_file;
+
+				get_file(info->f_file);
+				ret = 0;
+			}
+			spin_unlock(&files->file_lock);
+			put_files_struct(files);
 		}
-		spin_unlock(&files->file_lock);
-		put_files_struct(files);
 	}
 
-	if (!ret) {
-                seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
-			   (long long)file->f_pos, f_flags);
-		if (file->f_op->show_fdinfo)
-			ret = file->f_op->show_fdinfo(m, file);
-		fput(file);
+	if (ret) {
+		if (info->f_file)
+			fput(info->f_file);
+		file->private_data = NULL;
+		kfree(info);
 	}
 
 	return ret;
 }
 
-static int seq_fdinfo_open(struct inode *inode, struct file *file)
+static int seq_fdinfo_release(struct inode *inode, struct file *file)
 {
-	return single_open(file, seq_show, inode);
+	struct seq_file *m = file->private_data;
+	struct proc_fdinfo *info = seq_info(m);
+
+	fput(info->f_file);
+	return seq_release(inode, file);
 }
 
 static const struct file_operations proc_fdinfo_file_operations = {
 	.open		= seq_fdinfo_open,
 	.read		= seq_read,
 	.llseek		= seq_lseek,
-	.release	= single_release,
+	.release	= seq_fdinfo_release,
 };
 
 static int tid_fd_revalidate(struct dentry *dentry, unsigned int flags)
Index: linux-2.6.git/include/linux/fs.h
===================================================================
--- linux-2.6.git.orig/include/linux/fs.h
+++ linux-2.6.git/include/linux/fs.h
@@ -1552,6 +1552,7 @@ struct file_operations {
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
 	int (*show_fdinfo)(struct seq_file *m, struct file *f);
+	struct seq_operations *fdinfo_ops;
 };
 
 struct inode_operations {