All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@openvz.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Pavel Emelyanov <xemul@parallels.com>,
	James Bottomley <jbottomley@parallels.com>,
	Matthew Helsley <matt.helsley@gmail.com>
Subject: Re: [patch 3/8] procfs: Add ability to plug in auxiliary fdinfo providers
Date: Tue, 14 Aug 2012 23:56:49 +0400	[thread overview]
Message-ID: <20120814195649.GB31043@moon> (raw)
In-Reply-To: <20120814183558.GA1551@moon>

On Tue, Aug 14, 2012 at 10:35:58PM +0400, Cyrill Gorcunov wrote:
> On Tue, Aug 14, 2012 at 07:31:42PM +0100, Al Viro wrote:
> > > Initially we considered to inject some "show" metod to
> > > file_operations but since there really a number of
> > > file_operations declared inside kernel (and in real the
> > > further patches cover onle eventfd/epoll/inotify) the
> > > waste of memory space will be inacceptable I think.
> > 
> > NAK.  This is too ugly to live.  Put it into file_operations,
> > with NULL meaning default output, or don't do it at all.
> > 
> > And no, "it's only if you enable CONFIG_SOME_SHIT" gambit won't
> > fly - we have all seen it played too many times.  All it takes
> > is one politically-inclined induhvidual adding a dependency to
> > some "vertically integrated" turd (*cough* systemd *spit* udev
> > *cough*) and we are stuck with the damn thing.  CGROUP shite
> > is already there, DEVTMPFS is well on its way, etc.
> 
> OK, I'll put it into file_operations then (actually Pavel was
> proposing the same but I've been scared by amount of file_operations
> declared). Thanks!

Al, does the patch below looks better? If so I'll fix up the rest.
---
 fs/proc/fd.c            |  109 ++++++++++++++++++++++++++++++++++++++----------
 include/linux/fs.h      |    3 +
 include/linux/proc_fs.h |    8 +++
 3 files changed, 99 insertions(+), 21 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
@@ -8,18 +8,14 @@
 #include <linux/security.h>
 #include <linux/file.h>
 #include <linux/seq_file.h>
+#include <linux/spinlock.h>
 
 #include <linux/proc_fs.h>
 
 #include "internal.h"
 #include "fd.h"
 
-struct proc_fdinfo {
-	loff_t	f_pos;
-	int	f_flags;
-};
-
-static int fdinfo_open_helper(struct inode *inode, int *f_flags, struct path *path)
+static int fdinfo_open_helper(struct inode *inode, int *f_flags, struct file **f_file, struct path *path)
 {
 	struct files_struct *files = NULL;
 	struct task_struct *task;
@@ -49,6 +45,10 @@ static int fdinfo_open_helper(struct ino
 				*path = fd_file->f_path;
 				path_get(&fd_file->f_path);
 			}
+			if (f_file) {
+				*f_file = fd_file;
+				get_file(fd_file);
+			}
 			ret = 0;
 		}
 		spin_unlock(&files->file_lock);
@@ -58,43 +58,110 @@ static int fdinfo_open_helper(struct ino
 	return ret;
 }
 
+static void *seq_start(struct seq_file *m, loff_t *pos)
+{
+	struct proc_fdinfo_extra *extra = m->private;
+
+	extra->pos = *pos;
+
+	return *pos == 0 ? extra :
+		(extra->fdinfo_ops ?
+		 extra->fdinfo_ops->start(m, pos) : NULL);
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+	struct proc_fdinfo_extra *extra = m->private;
+
+	if (extra->fdinfo_ops && extra->pos > 0)
+		extra->fdinfo_ops->stop(m, v);
+}
+
+static void *seq_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	struct proc_fdinfo_extra *extra = m->private;
+	void *v = NULL;
+
+	if (extra->fdinfo_ops) {
+		int ret = 0;
+
+		if (*pos == 0) {
+			v = extra->fdinfo_ops->start(m, pos);
+			if (v) {
+				ret = extra->fdinfo_ops->show(m, v);
+				p = v;
+			} else
+				ret = -1;
+		}
+
+		if (!ret)
+			v = extra->fdinfo_ops->next(m, p, pos);
+	} else
+		++*pos;
+
+	extra->pos = *pos;
+	return v;
+}
+
 static int seq_show(struct seq_file *m, void *v)
 {
-	struct proc_fdinfo *fdinfo = m->private;
+	struct proc_fdinfo_extra *extra = m->private;
+
+	if (extra->fdinfo_ops && extra->pos > 0)
+		return extra->fdinfo_ops->show(m, v);
+
 	seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
-		   (long long)fdinfo->f_pos,
-		   fdinfo->f_flags);
+		   (long long)extra->f_file->f_pos,
+		   extra->f_flags);
 	return 0;
 }
 
+static const struct seq_operations fdinfo_seq_ops = {
+	.start	= seq_start,
+	.next	= seq_next,
+	.stop	= seq_stop,
+	.show	= seq_show,
+};
+
 static int seq_fdinfo_open(struct inode *inode, struct file *file)
 {
-	struct proc_fdinfo *fdinfo = NULL;
-	int ret = -ENOENT;
+	struct proc_fdinfo_extra *extra;
+	struct seq_file *m;
+	int ret;
 
-	fdinfo = kzalloc(sizeof(*fdinfo), GFP_KERNEL);
-	if (!fdinfo)
+	extra = kzalloc(sizeof(*extra), GFP_KERNEL);
+	if (!extra)
 		return -ENOMEM;
 
-	ret = fdinfo_open_helper(inode, &fdinfo->f_flags, NULL);
+	ret = seq_open(file, &fdinfo_seq_ops);
 	if (!ret) {
-		ret = single_open(file, seq_show, fdinfo);
+		ret = -ENOENT;
+		m = file->private_data;
+		m->private = extra;
+
+		ret = fdinfo_open_helper(inode, &extra->f_flags,
+					 &extra->f_file, NULL);
 		if (!ret)
-			fdinfo = NULL;
+			extra->fdinfo_ops = extra->f_file->f_op->fdinfo_ops;
 	}
 
-	kfree(fdinfo);
+	if (ret) {
+		if (extra->f_file)
+			put_filp(extra->f_file);
+		kfree(extra);
+	}
 	return ret;
 }
 
 static int seq_fdinfo_release(struct inode *inode, struct file *file)
 {
 	struct seq_file *m = file->private_data;
-	struct proc_fdinfo *fdinfo = m->private;
+	struct proc_fdinfo_extra *extra = m->private;
 
-	kfree(fdinfo);
+	put_filp(extra->f_file);
+	kfree(m->private);
 
-	return single_release(inode, file);
+	return seq_release(inode, file);
 }
 
 static const struct file_operations proc_fdinfo_file_operations = {
@@ -173,7 +240,7 @@ static const struct dentry_operations ti
 
 static int proc_fd_link(struct dentry *dentry, struct path *path)
 {
-	return fdinfo_open_helper(dentry->d_inode, NULL, path);
+	return fdinfo_open_helper(dentry->d_inode, NULL, NULL, path);
 }
 
 static struct dentry *
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
@@ -1775,8 +1775,11 @@ struct block_device_operations;
 #define HAVE_COMPAT_IOCTL 1
 #define HAVE_UNLOCKED_IOCTL 1
 
+struct seq_operations;
+
 struct file_operations {
 	struct module *owner;
+	struct seq_operations *fdinfo_ops;
 	loff_t (*llseek) (struct file *, loff_t, int);
 	ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
 	ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
Index: linux-2.6.git/include/linux/proc_fs.h
===================================================================
--- linux-2.6.git.orig/include/linux/proc_fs.h
+++ linux-2.6.git/include/linux/proc_fs.h
@@ -100,6 +100,14 @@ struct vmcore {
 	loff_t offset;
 };
 
+/* auxiliary data allocated per fdinfo reader */
+struct proc_fdinfo_extra {
+	loff_t			pos;
+	struct file		*f_file;
+	struct seq_operations	*fdinfo_ops;
+	unsigned int		f_flags;
+};
+
 #ifdef CONFIG_PROC_FS
 
 extern void proc_root_init(void);

  reply	other threads:[~2012-08-14 19:56 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-14 14:03 [patch 0/8] procfs fdinfo providers updated Cyrill Gorcunov
2012-08-14 14:03 ` [patch 1/8] procfs: Move /proc/pid/fd[info] handling code to fd.[ch] Cyrill Gorcunov
2012-08-14 14:21   ` Pavel Emelyanov
2012-08-14 14:03 ` [patch 2/8] procfs: Convert /proc/pid/fdinfo/ handling routines to seq-file Cyrill Gorcunov
2012-08-14 14:21   ` Pavel Emelyanov
2012-08-14 14:03 ` [patch 3/8] procfs: Add ability to plug in auxiliary fdinfo providers Cyrill Gorcunov
2012-08-14 14:22   ` Pavel Emelyanov
2012-08-14 18:31   ` Al Viro
2012-08-14 18:35     ` Cyrill Gorcunov
2012-08-14 19:56       ` Cyrill Gorcunov [this message]
2012-08-14 21:27         ` Al Viro
2012-08-14 21:56           ` Cyrill Gorcunov
2012-08-14 22:21             ` Cyrill Gorcunov
2012-08-15  0:07               ` Al Viro
2012-08-15  7:40                 ` Cyrill Gorcunov
2012-08-14 14:03 ` [patch 4/8] fs, eventfd: Add procfs fdinfo helper Cyrill Gorcunov
2012-08-14 14:22   ` Pavel Emelyanov
2012-08-14 14:03 ` [patch 5/8] fs, epoll: Add procfs fdinfo helper v2 Cyrill Gorcunov
2012-08-14 14:22   ` Pavel Emelyanov
2012-08-14 14:03 ` [patch 6/8] fs, exportfs: Add export_encode_inode_fh helper Cyrill Gorcunov
2012-08-14 14:23   ` Pavel Emelyanov
2012-08-14 14:03 ` [patch 7/8] fs, notify: Add procfs fdinfo helper v3 Cyrill Gorcunov
2012-08-14 14:23   ` Pavel Emelyanov
2012-08-14 14:03 ` [patch 8/8] fdinfo: Show sigmask for signalfd fd Cyrill Gorcunov
  -- strict thread matches above, loose matches on Subject: below --
2012-08-15  9:21 [patch 0/8] procfs, fdinfo updated Cyrill Gorcunov
2012-08-15  9:21 ` [patch 3/8] procfs: Add ability to plug in auxiliary fdinfo providers Cyrill Gorcunov
2012-08-15 21:16   ` Al Viro
2012-08-15 21:31     ` Cyrill Gorcunov
2012-08-15 21:29   ` Al Viro
2012-08-15 21:34     ` Cyrill Gorcunov
2012-08-16 10:58     ` Cyrill Gorcunov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120814195649.GB31043@moon \
    --to=gorcunov@openvz.org \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=jbottomley@parallels.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt.helsley@gmail.com \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=xemul@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.