This patch brings ability to plug in auxiliary fdinfo providers.
For example in further patches eventfd, evenpoll and fsnotify
will print out information associated with files.
This feature is CONFIG_CHECKPOINT_RESTORE guarded to eliminate
overhead for those who don't need it at all (this
unfortunately makes patch bigger than I wanted).
The basic usage rule is the following
- fdinfo provider should register own "show" method
via proc_register_fdinfo_driver call, where "show"
methods are rather well known seq-file operations
- once the kernel opens /proc/$pid/fdinfo/$fd file
it calls for ->probe() method in registered fdinfo
drivers, and if probe success, then seq-file "show"
operations will be called to provide out additional
infomation
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.
Pavel, I've left seq_next memthod as it was simply because
we can't leave seq_next() after calling extra->driver->ops->start
without increasing "pos", thus we need to call for "show" manually
once.
Signed-off-by: Cyrill Gorcunov <[email protected]>
CC: Al Viro <[email protected]>
CC: Alexey Dobriyan <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Pavel Emelyanov <[email protected]>
CC: James Bottomley <[email protected]>
---
fs/proc/fd.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++--
include/linux/proc_fs.h | 26 ++++++
2 files changed, 216 insertions(+), 8 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,6 +58,186 @@ static int fdinfo_open_helper(struct ino
return ret;
}
+#ifdef CONFIG_CHECKPOINT_RESTORE
+
+static LIST_HEAD(fdinfo_drivers);
+static DECLARE_RWSEM(fdinfo_drivers_sem);
+
+int proc_register_fdinfo_driver(struct proc_fdinfo_driver *s)
+{
+ struct proc_fdinfo_driver *i;
+ int ret = 0;
+
+ if (!s->ops || !s->probe)
+ return -EINVAL;
+
+ down_write(&fdinfo_drivers_sem);
+ list_for_each_entry(i, &fdinfo_drivers, list) {
+ if (i == s) {
+ WARN_ONCE("Trying reassign fdinfo driver `%s'\n",
+ i->name);
+ ret = -EINVAL;
+ break;
+ }
+ }
+ if (!ret)
+ list_add(&s->list, &fdinfo_drivers);
+ up_write(&fdinfo_drivers_sem);
+
+ return ret;
+}
+
+void proc_unregister_fdinfo_driver(struct proc_fdinfo_driver *s)
+{
+ struct proc_fdinfo_driver *i, *tmp;
+
+ down_write(&fdinfo_drivers_sem);
+ list_for_each_entry_safe(i, tmp, &fdinfo_drivers, list) {
+ if (i == s) {
+ list_del(&i->list);
+ break;
+ }
+ }
+ up_write(&fdinfo_drivers_sem);
+}
+
+static int prep_fdinfo_driver(struct proc_fdinfo_extra *extra)
+{
+ struct proc_fdinfo_driver *s;
+
+ down_read(&fdinfo_drivers_sem);
+ list_for_each_entry(s, &fdinfo_drivers, list) {
+ if (s->probe(extra->f_file)) {
+ extra->driver = s;
+ break;
+ }
+ }
+ up_read(&fdinfo_drivers_sem);
+
+ return 0;
+}
+
+static void *seq_start(struct seq_file *m, loff_t *pos)
+{
+ struct proc_fdinfo_extra *extra = m->private;
+
+ down_read(&fdinfo_drivers_sem);
+ extra->pos = *pos;
+
+ return *pos == 0 ? extra :
+ (extra->driver ? extra->driver->ops->start(m, pos) : NULL);
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+{
+ struct proc_fdinfo_extra *extra = m->private;
+
+ if (extra->driver && extra->pos > 0)
+ extra->driver->ops->stop(m, v);
+ up_read(&fdinfo_drivers_sem);
+}
+
+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->driver) {
+ int ret = 0;
+
+ if (*pos == 0) {
+ v = extra->driver->ops->start(m, pos);
+ if (v) {
+ ret = extra->driver->ops->show(m, v);
+ p = v;
+ } else
+ ret = -1;
+ }
+
+ if (!ret)
+ v = extra->driver->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_extra *extra = m->private;
+
+ if (extra->driver && extra->pos > 0)
+ return extra->driver->ops->show(m, v);
+
+ seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
+ (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_extra *extra;
+ struct seq_file *m;
+ int ret;
+
+ extra = kzalloc(sizeof(*extra), GFP_KERNEL);
+ if (!extra)
+ return -ENOMEM;
+
+ ret = seq_open(file, &fdinfo_seq_ops);
+ if (!ret) {
+ ret = -ENOENT;
+ m = file->private_data;
+ m->private = extra;
+
+ ret = fdinfo_open_helper(inode, &extra->f_flags,
+ &extra->f_file, NULL);
+ if (!ret)
+ ret = prep_fdinfo_driver(extra);
+ }
+
+ 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_extra *extra = m->private;
+
+ put_filp(extra->f_file);
+ kfree(m->private);
+
+ 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 = seq_fdinfo_release,
+};
+
+#else /* CONFIG_CHECKPOINT_RESTORE */
+
+struct proc_fdinfo {
+ loff_t f_pos;
+ int f_flags;
+};
+
static int seq_show(struct seq_file *m, void *v)
{
struct proc_fdinfo *fdinfo = m->private;
@@ -76,7 +256,7 @@ static int seq_fdinfo_open(struct inode
if (!fdinfo)
return -ENOMEM;
- ret = fdinfo_open_helper(inode, &fdinfo->f_flags, NULL);
+ ret = fdinfo_open_helper(inode, &fdinfo->f_flags, NULL, NULL);
if (!ret) {
ret = single_open(file, seq_show, fdinfo);
if (!ret)
@@ -104,6 +284,8 @@ static const struct file_operations proc
.release = seq_fdinfo_release,
};
+#endif /* CONFIG_CHECKPOINT_RESTORE */
+
static int tid_fd_revalidate(struct dentry *dentry, unsigned int flags)
{
struct files_struct *files;
@@ -173,7 +355,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/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,24 @@ struct vmcore {
loff_t offset;
};
+struct seq_operations;
+
+/* fdinfo auxiliary information drivers */
+struct proc_fdinfo_driver {
+ struct list_head list;
+ const char *name;
+ const struct seq_operations *ops;
+ int (*probe)(struct file *file);
+};
+
+/* auxiliary data allocated per fdinfo reader */
+struct proc_fdinfo_extra {
+ struct proc_fdinfo_driver *driver;
+ loff_t pos;
+ struct file *f_file;
+ unsigned int f_flags;
+};
+
#ifdef CONFIG_PROC_FS
extern void proc_root_init(void);
@@ -175,6 +193,11 @@ extern struct proc_dir_entry *proc_net_m
extern struct file *proc_ns_fget(int fd);
+#ifdef CONFIG_CHECKPOINT_RESTORE
+extern int proc_register_fdinfo_driver(struct proc_fdinfo_driver *s);
+extern void proc_unregister_fdinfo_driver(struct proc_fdinfo_driver *s);
+#endif /* CONFIG_CHECKPOINT_RESTORE */
+
#else
#define proc_net_fops_create(net, name, mode, fops) ({ (void)(mode), NULL; })
@@ -229,6 +252,9 @@ static inline struct file *proc_ns_fget(
return ERR_PTR(-EINVAL);
}
+static inline int proc_register_fdinfo_driver(struct proc_fdinfo_driver *s) { return -EINVAL; }
+static inline void proc_unregister_fdinfo_driver(struct proc_fdinfo_driver *s) { }
+
#endif /* CONFIG_PROC_FS */
#if !defined(CONFIG_PROC_KCORE)
On 08/14/2012 06:03 PM, Cyrill Gorcunov wrote:
> This patch brings ability to plug in auxiliary fdinfo providers.
> For example in further patches eventfd, evenpoll and fsnotify
> will print out information associated with files.
>
> This feature is CONFIG_CHECKPOINT_RESTORE guarded to eliminate
> overhead for those who don't need it at all (this
> unfortunately makes patch bigger than I wanted).
>
> The basic usage rule is the following
>
> - fdinfo provider should register own "show" method
> via proc_register_fdinfo_driver call, where "show"
> methods are rather well known seq-file operations
>
> - once the kernel opens /proc/$pid/fdinfo/$fd file
> it calls for ->probe() method in registered fdinfo
> drivers, and if probe success, then seq-file "show"
> operations will be called to provide out additional
> infomation
>
> 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.
>
> Pavel, I've left seq_next memthod as it was simply because
> we can't leave seq_next() after calling extra->driver->ops->start
> without increasing "pos", thus we need to call for "show" manually
> once.
>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> CC: Al Viro <[email protected]>
> CC: Alexey Dobriyan <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Pavel Emelyanov <[email protected]>
> CC: James Bottomley <[email protected]>
Acked-by: Pavel Emelyanov <[email protected]>
On Tue, Aug 14, 2012 at 06:03:45PM +0400, Cyrill Gorcunov wrote:
> This patch brings ability to plug in auxiliary fdinfo providers.
> For example in further patches eventfd, evenpoll and fsnotify
> will print out information associated with files.
>
> This feature is CONFIG_CHECKPOINT_RESTORE guarded to eliminate
> overhead for those who don't need it at all (this
> unfortunately makes patch bigger than I wanted).
>
> The basic usage rule is the following
>
> - fdinfo provider should register own "show" method
> via proc_register_fdinfo_driver call, where "show"
> methods are rather well known seq-file operations
>
> - once the kernel opens /proc/$pid/fdinfo/$fd file
> it calls for ->probe() method in registered fdinfo
> drivers, and if probe success, then seq-file "show"
> operations will be called to provide out additional
> infomation
>
> 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.
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!
Cyrill
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);
On Tue, Aug 14, 2012 at 11:56:49PM +0400, Cyrill Gorcunov wrote:
> Al, does the patch below looks better? If so I'll fix up the rest.
> struct file_operations {
> struct module *owner;
> + struct seq_operations *fdinfo_ops;
IDGI. Why on the earth do you need the whole iterator? All it takes
is show_fdinfo(struct seq_file *m, struct file *f); have ->show() in
your iterator (one going through the files) call that sucker for the
file we are trying to show.
I think you are severely overdesigning that thing...
On Tue, Aug 14, 2012 at 10:27:21PM +0100, Al Viro wrote:
> On Tue, Aug 14, 2012 at 11:56:49PM +0400, Cyrill Gorcunov wrote:
> > Al, does the patch below looks better? If so I'll fix up the rest.
>
> > struct file_operations {
> > struct module *owner;
> > + struct seq_operations *fdinfo_ops;
>
> IDGI. Why on the earth do you need the whole iterator? All it takes
> is show_fdinfo(struct seq_file *m, struct file *f); have ->show() in
> your iterator (one going through the files) call that sucker for the
> file we are trying to show.
>
> I think you are severely overdesigning that thing...
Hmm, in very first versions I've been using one ->show method, but
then I thought that this is not very correlate with seq-files idea
where for each record show/next sequence is called. I'll update (this
for sure will make code simplier, and I'll have to check for seq-file
overflow after seq_printf call to not continue printing data for too
long if buffer already out of space).
Cyrill
On Wed, Aug 15, 2012 at 01:56:16AM +0400, Cyrill Gorcunov wrote:
> > > struct file_operations {
> > > struct module *owner;
> > > + struct seq_operations *fdinfo_ops;
> >
> > IDGI. Why on the earth do you need the whole iterator? All it takes
> > is show_fdinfo(struct seq_file *m, struct file *f); have ->show() in
> > your iterator (one going through the files) call that sucker for the
> > file we are trying to show.
> >
> > I think you are severely overdesigning that thing...
>
> Hmm, in very first versions I've been using one ->show method, but
> then I thought that this is not very correlate with seq-files idea
> where for each record show/next sequence is called. I'll update (this
> for sure will make code simplier, and I'll have to check for seq-file
> overflow after seq_printf call to not continue printing data for too
> long if buffer already out of space).
Al, I'll cook the whole series tomorrow and resend it for review,
also I guess the new show_fdinfo() member in file-operations should
be guarded with CONFIG_PROC_FS, right?
On Wed, Aug 15, 2012 at 02:21:47AM +0400, Cyrill Gorcunov wrote:
> > Hmm, in very first versions I've been using one ->show method, but
> > then I thought that this is not very correlate with seq-files idea
> > where for each record show/next sequence is called. I'll update (this
> > for sure will make code simplier, and I'll have to check for seq-file
> > overflow after seq_printf call to not continue printing data for too
> > long if buffer already out of space).
>
> Al, I'll cook the whole series tomorrow and resend it for review,
> also I guess the new show_fdinfo() member in file-operations should
> be guarded with CONFIG_PROC_FS, right?
I seriously doubt that it's worth bothering. If somebody cares, they
can add making it conditional later.
On Wed, Aug 15, 2012 at 01:07:03AM +0100, Al Viro wrote:
> On Wed, Aug 15, 2012 at 02:21:47AM +0400, Cyrill Gorcunov wrote:
> > > Hmm, in very first versions I've been using one ->show method, but
> > > then I thought that this is not very correlate with seq-files idea
> > > where for each record show/next sequence is called. I'll update (this
> > > for sure will make code simplier, and I'll have to check for seq-file
> > > overflow after seq_printf call to not continue printing data for too
> > > long if buffer already out of space).
> >
> > Al, I'll cook the whole series tomorrow and resend it for review,
> > also I guess the new show_fdinfo() member in file-operations should
> > be guarded with CONFIG_PROC_FS, right?
>
> I seriously doubt that it's worth bothering. If somebody cares, they
> can add making it conditional later.
That's what I've beed testing, does it looks good for you?
---
From: Cyrill Gorcunov <[email protected]>
Subject: procfs: Add ability to plug in auxiliary fdinfo providers
This patch brings ability to print out auxiliary data associated
with file in procfs interface /proc/pid/fdinfo/fd.
Inparticular further patches make eventfd, evenpoll, signalfd
and fsnotify to print additional information complete enough
to restore these objects after checkpoint.
To simplify the code we add show_fdinfo callback into
struct file_operations (as Al proposed).
Signed-off-by: Cyrill Gorcunov <[email protected]>
CC: Pavel Emelyanov <[email protected]>
CC: Al Viro <[email protected]>
CC: Alexey Dobriyan <[email protected]>
CC: Andrew Morton <[email protected]>
CC: James Bottomley <[email protected]>
---
fs/proc/fd.c | 51 ++++++++++++++++++++++++++++++++++++---------------
include/linux/fs.h | 3 +++
2 files changed, 39 insertions(+), 15 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
@@ -15,11 +15,11 @@
#include "fd.h"
struct proc_fdinfo {
- loff_t f_pos;
- int f_flags;
+ struct file *f_file;
+ 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 +49,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);
@@ -61,28 +65,44 @@ static int fdinfo_open_helper(struct ino
static int seq_show(struct seq_file *m, void *v)
{
struct proc_fdinfo *fdinfo = m->private;
- seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
- (long long)fdinfo->f_pos,
- fdinfo->f_flags);
- return 0;
+ int ret;
+
+ ret = seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
+ (long long)fdinfo->f_file->f_pos,
+ fdinfo->f_flags);
+
+ if (!ret && fdinfo->f_file->f_op->show_fdinfo)
+ ret = fdinfo->f_file->f_op->show_fdinfo(m, fdinfo->f_file);
+
+ return ret;
}
static int seq_fdinfo_open(struct inode *inode, struct file *file)
{
- struct proc_fdinfo *fdinfo = NULL;
- int ret = -ENOENT;
+ struct proc_fdinfo *fdinfo;
+ struct seq_file *m;
+ int ret;
fdinfo = kzalloc(sizeof(*fdinfo), GFP_KERNEL);
if (!fdinfo)
return -ENOMEM;
- ret = fdinfo_open_helper(inode, &fdinfo->f_flags, NULL);
- if (!ret) {
- ret = single_open(file, seq_show, fdinfo);
- if (!ret)
- fdinfo = NULL;
+ ret = fdinfo_open_helper(inode, &fdinfo->f_flags, &fdinfo->f_file, NULL);
+ if (ret)
+ goto err_free;
+
+ ret = single_open(file, seq_show, fdinfo);
+ if (ret) {
+ put_filp(fdinfo->f_file);
+ goto err_free;
}
+ m = file->private_data;
+ m->private = fdinfo;
+
+ return ret;
+
+err_free:
kfree(fdinfo);
return ret;
}
@@ -92,6 +112,7 @@ static int seq_fdinfo_release(struct ino
struct seq_file *m = file->private_data;
struct proc_fdinfo *fdinfo = m->private;
+ put_filp(fdinfo->f_file);
kfree(fdinfo);
return single_release(inode, file);
@@ -173,7 +194,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,6 +1775,8 @@ struct block_device_operations;
#define HAVE_COMPAT_IOCTL 1
#define HAVE_UNLOCKED_IOCTL 1
+struct seq_file;
+
struct file_operations {
struct module *owner;
loff_t (*llseek) (struct file *, loff_t, int);
@@ -1803,6 +1805,7 @@ 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 inode_operations {