* [patch 1/7] procfs: Add ability to plug in auxiliary fdinfo providers
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-13 0:40 ` Andrew Morton
2012-11-12 10:14 ` [patch 2/7] fs, exportfs: Escape nil dereference if no s_export_op present Cyrill Gorcunov
` (5 subsequent siblings)
6 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro
[-- Attachment #1: seq-fdinfo-seq-ops-helpers-12 --]
[-- Type: text/plain, Size: 2156 bytes --]
This patch brings ability to print out auxiliary data associated
with file in procfs interface /proc/pid/fdinfo/fd.
In particular 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 inside
struct file_operations (as Al and Pavel are proposing).
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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>
---
fs/proc/fd.c | 2 ++
include/linux/fs.h | 3 +++
2 files changed, 5 insertions(+)
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
@@ -50,6 +50,8 @@ static int seq_show(struct seq_file *m,
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);
}
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
@@ -1517,6 +1517,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);
@@ -1545,6 +1547,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 {
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 1/7] procfs: Add ability to plug in auxiliary fdinfo providers
2012-11-12 10:14 ` [patch 1/7] procfs: Add ability to plug in auxiliary fdinfo providers Cyrill Gorcunov
@ 2012-11-13 0:40 ` Andrew Morton
2012-11-13 7:03 ` Cyrill Gorcunov
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Morton @ 2012-11-13 0:40 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, 12 Nov 2012 14:14:41 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> This patch brings ability to print out auxiliary data associated
> with file in procfs interface /proc/pid/fdinfo/fd.
>
> In particular 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 inside
> struct file_operations (as Al and Pavel are proposing).
>
> ...
>
> --- linux-2.6.git.orig/include/linux/fs.h
> +++ linux-2.6.git/include/linux/fs.h
> @@ -1517,6 +1517,8 @@ struct block_device_operations;
> #define HAVE_COMPAT_IOCTL 1
> #define HAVE_UNLOCKED_IOCTL 1
>
> +struct seq_file;
> +
We already have a forward declaration of seq_file in fs.h. At line
1583(!).
This is why forward declarations should always be placed at the start
of the file, please.
--- a/include/linux/fs.h~procfs-add-ability-to-plug-in-auxiliary-fdinfo-providers-fix
+++ a/include/linux/fs.h
@@ -44,6 +44,7 @@ struct vm_area_struct;
struct vfsmount;
struct cred;
struct swap_info_struct;
+struct seq_file;
extern void __init inode_init(void);
extern void __init inode_init_early(void);
@@ -1517,8 +1518,6 @@ 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);
@@ -1583,8 +1582,6 @@ struct inode_operations {
umode_t create_mode, int *opened);
} ____cacheline_aligned;
-struct seq_file;
-
ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
unsigned long nr_segs, unsigned long fast_segs,
struct iovec *fast_pointer,
_
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 1/7] procfs: Add ability to plug in auxiliary fdinfo providers
2012-11-13 0:40 ` Andrew Morton
@ 2012-11-13 7:03 ` Cyrill Gorcunov
0 siblings, 0 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-13 7:03 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, Nov 12, 2012 at 04:40:22PM -0800, Andrew Morton wrote:
> >
> > --- linux-2.6.git.orig/include/linux/fs.h
> > +++ linux-2.6.git/include/linux/fs.h
> > @@ -1517,6 +1517,8 @@ struct block_device_operations;
> > #define HAVE_COMPAT_IOCTL 1
> > #define HAVE_UNLOCKED_IOCTL 1
> >
> > +struct seq_file;
> > +
>
> We already have a forward declaration of seq_file in fs.h. At line
> 1583(!).
>
> This is why forward declarations should always be placed at the start
> of the file, please.
Yes, thanks a lot, Andrew!
Cyrill
^ permalink raw reply [flat|nested] 44+ messages in thread
* [patch 2/7] fs, exportfs: Escape nil dereference if no s_export_op present
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
2012-11-12 10:14 ` [patch 1/7] procfs: Add ability to plug in auxiliary fdinfo providers Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-12 10:14 ` [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark Cyrill Gorcunov
` (4 subsequent siblings)
6 siblings, 0 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro
[-- Attachment #1: fs-exportfs-add-null-check --]
[-- Type: text/plain, Size: 1304 bytes --]
This routine will be used to generate a file handle in fdinfo
output for inotify subsystem, where if no s_export_op present
the general export_encode_fh should be used. Thus add
a test if s_export_op present inside exportfs_encode_fh itself.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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>
---
fs/exportfs/expfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.git/fs/exportfs/expfs.c
===================================================================
--- linux-2.6.git.orig/fs/exportfs/expfs.c
+++ linux-2.6.git/fs/exportfs/expfs.c
@@ -357,7 +357,7 @@ int exportfs_encode_fh(struct dentry *de
*/
parent = p->d_inode;
}
- if (nop->encode_fh)
+ if (nop && nop->encode_fh)
error = nop->encode_fh(inode, fid->raw, max_len, parent);
else
error = export_encode_fh(inode, fid, max_len, parent);
^ permalink raw reply [flat|nested] 44+ messages in thread
* [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
2012-11-12 10:14 ` [patch 1/7] procfs: Add ability to plug in auxiliary fdinfo providers Cyrill Gorcunov
2012-11-12 10:14 ` [patch 2/7] fs, exportfs: Escape nil dereference if no s_export_op present Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-13 0:55 ` Andrew Morton
2012-11-12 10:14 ` [patch 4/7] fs, notify: Add procfs fdinfo helper v4 Cyrill Gorcunov
` (3 subsequent siblings)
6 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro
[-- Attachment #1: fsnotify-add-fhandler --]
[-- Type: text/plain, Size: 4759 bytes --]
This file handle will be used in /proc/pid/fdinfo/fd
output, which in turn will allow to restore a watch
target after checkpoint (thus it's provided for
CONFIG_CHECKPOINT_RESTORE only).
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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>
---
fs/notify/inotify/inotify.h | 8 +++++++
fs/notify/inotify/inotify_user.c | 42 ++++++++++++++++++++++++++++++++++-----
2 files changed, 45 insertions(+), 5 deletions(-)
Index: linux-2.6.git/fs/notify/inotify/inotify.h
===================================================================
--- linux-2.6.git.orig/fs/notify/inotify/inotify.h
+++ linux-2.6.git/fs/notify/inotify/inotify.h
@@ -1,6 +1,7 @@
#include <linux/fsnotify_backend.h>
#include <linux/inotify.h>
#include <linux/slab.h> /* struct kmem_cache */
+#include <linux/exportfs.h>
extern struct kmem_cache *event_priv_cachep;
@@ -9,9 +10,16 @@ struct inotify_event_private_data {
int wd;
};
+#if defined(CONFIG_PROC_FS) && defined(CONFIG_EXPORTFS) && defined(CONFIG_CHECKPOINT_RESTORE)
+# define INOTIFY_USE_FHANDLE
+#endif
+
struct inotify_inode_mark {
struct fsnotify_mark fsn_mark;
int wd;
+#ifdef INOTIFY_USE_FHANDLE
+ __u8 fhandle[sizeof(struct file_handle) + MAX_HANDLE_SZ];
+#endif
};
extern void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
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
@@ -566,6 +566,33 @@ static void inotify_free_mark(struct fsn
kmem_cache_free(inotify_inode_mark_cachep, i_mark);
}
+#ifdef INOTIFY_USE_FHANDLE
+static int inotify_encode_wd_fhandle(struct inotify_inode_mark *mark, struct dentry *dentry)
+{
+ struct file_handle *fhandle = (struct file_handle *)mark->fhandle;
+ int size, ret;
+
+ BUILD_BUG_ON(sizeof(mark->fhandle) <= sizeof(struct file_handle));
+
+ fhandle->handle_bytes = sizeof(mark->fhandle) - sizeof(struct file_handle);
+ size = fhandle->handle_bytes >> 2;
+
+ ret = exportfs_encode_fh(dentry, (struct fid *)fhandle->f_handle, &size, 0);
+ if ((ret == 255) || (ret == -ENOSPC))
+ return -EOVERFLOW;
+
+ fhandle->handle_type = ret;
+ fhandle->handle_bytes = size * sizeof(u32);
+
+ return 0;
+}
+# else
+static int inotify_encode_wd_fhandle(struct inotify_inode_mark *mark, struct dentry *dentry)
+{
+ return 0;
+}
+#endif
+
static int inotify_update_existing_watch(struct fsnotify_group *group,
struct inode *inode,
u32 arg)
@@ -621,10 +648,11 @@ static int inotify_update_existing_watch
}
static int inotify_new_watch(struct fsnotify_group *group,
- struct inode *inode,
+ struct dentry *dentry,
u32 arg)
{
struct inotify_inode_mark *tmp_i_mark;
+ struct inode *inode = dentry->d_inode;
__u32 mask;
int ret;
struct idr *idr = &group->inotify_data.idr;
@@ -647,6 +675,10 @@ static int inotify_new_watch(struct fsno
if (atomic_read(&group->inotify_data.user->inotify_watches) >= inotify_max_user_watches)
goto out_err;
+ ret = inotify_encode_wd_fhandle(tmp_i_mark, dentry);
+ if (ret)
+ goto out_err;
+
ret = inotify_add_to_idr(idr, idr_lock, &group->inotify_data.last_wd,
tmp_i_mark);
if (ret)
@@ -673,16 +705,16 @@ out_err:
return ret;
}
-static int inotify_update_watch(struct fsnotify_group *group, struct inode *inode, u32 arg)
+static int inotify_update_watch(struct fsnotify_group *group, struct dentry *dentry, u32 arg)
{
int ret = 0;
retry:
/* try to update and existing watch with the new arg */
- ret = inotify_update_existing_watch(group, inode, arg);
+ ret = inotify_update_existing_watch(group, dentry->d_inode, arg);
/* no mark present, try to add a new one */
if (ret == -ENOENT)
- ret = inotify_new_watch(group, inode, arg);
+ ret = inotify_new_watch(group, dentry, arg);
/*
* inotify_new_watch could race with another thread which did an
* inotify_new_watch between the update_existing and the add watch
@@ -785,7 +817,7 @@ SYSCALL_DEFINE3(inotify_add_watch, int,
group = f.file->private_data;
/* create/update an inode mark */
- ret = inotify_update_watch(group, inode, mask);
+ ret = inotify_update_watch(group, path.dentry, mask);
path_put(&path);
fput_and_out:
fdput(f);
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-12 10:14 ` [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark Cyrill Gorcunov
@ 2012-11-13 0:55 ` Andrew Morton
2012-11-13 7:20 ` Cyrill Gorcunov
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Morton @ 2012-11-13 0:55 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, 12 Nov 2012 14:14:43 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> This file handle will be used in /proc/pid/fdinfo/fd
> output, which in turn will allow to restore a watch
> target after checkpoint (thus it's provided for
> CONFIG_CHECKPOINT_RESTORE only).
This changelog isn't very good.
What appears to be happening is that you're borrowing exportfs's
ability to encode file handles and this is being reused to transport
inotify fd's to userspace for c/r? Or something else - I didn't try
very hard. Please explain better?
> --- linux-2.6.git.orig/fs/notify/inotify/inotify.h
> +++ linux-2.6.git/fs/notify/inotify/inotify.h
> @@ -1,6 +1,7 @@
> #include <linux/fsnotify_backend.h>
> #include <linux/inotify.h>
> #include <linux/slab.h> /* struct kmem_cache */
> +#include <linux/exportfs.h>
>
> extern struct kmem_cache *event_priv_cachep;
>
> @@ -9,9 +10,16 @@ struct inotify_event_private_data {
> int wd;
> };
>
> +#if defined(CONFIG_PROC_FS) && defined(CONFIG_EXPORTFS) && defined(CONFIG_CHECKPOINT_RESTORE)
> +# define INOTIFY_USE_FHANDLE
> +#endif
Does this mean that c/r will fail to work properly if
CONFIG_EXPORTFS=n? If so, that should be expressed in Kconfig
dependencies?
> struct inotify_inode_mark {
> struct fsnotify_mark fsn_mark;
> int wd;
> +#ifdef INOTIFY_USE_FHANDLE
> + __u8 fhandle[sizeof(struct file_handle) + MAX_HANDLE_SZ];
> +#endif
> };
Whoa. This adds 128+8 bytes to the inotify_inode_mark. That's a lot of
bloat, and there can be a lot of inotify_inode_mark's in the system?
Also, what about alignment? That embedded `struct file_handle' will
have a 4-byte alignment and if there's anything in there which is an
8-byte quantity then some architectures (ia64?) might get upset about
the kernel-mode unaligned access. It appears that this won't happen in
the present code, but are we future-proof?
Why did you use a __u8, anyway? Could have done something like
struct {
struct file_handle fhandle;
u8 pad[MAX_HANDLE_SZ];
};
and got some additional type safety and less typecasting?
> extern void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
> 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
> @@ -566,6 +566,33 @@ static void inotify_free_mark(struct fsn
> kmem_cache_free(inotify_inode_mark_cachep, i_mark);
> }
>
> +#ifdef INOTIFY_USE_FHANDLE
> +static int inotify_encode_wd_fhandle(struct inotify_inode_mark *mark, struct dentry *dentry)
> +{
> + struct file_handle *fhandle = (struct file_handle *)mark->fhandle;
> + int size, ret;
> +
> + BUILD_BUG_ON(sizeof(mark->fhandle) <= sizeof(struct file_handle));
> +
> + fhandle->handle_bytes = sizeof(mark->fhandle) - sizeof(struct file_handle);
> + size = fhandle->handle_bytes >> 2;
> +
> + ret = exportfs_encode_fh(dentry, (struct fid *)fhandle->f_handle, &size, 0);
> + if ((ret == 255) || (ret == -ENOSPC))
Sigh. ENOSPC means "your disk is full". If this error ever gets back
to userspace, our poor user will go and provision more disks and then
wonder why that didn't fix anything.
> + return -EOVERFLOW;
That doesn't seem very appropriate either, unsure.
> + fhandle->handle_type = ret;
> + fhandle->handle_bytes = size * sizeof(u32);
> +
> + return 0;
> +}
>
> ...
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 0:55 ` Andrew Morton
@ 2012-11-13 7:20 ` Cyrill Gorcunov
2012-11-13 7:40 ` Andrew Morton
0 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-13 7:20 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, Nov 12, 2012 at 04:55:40PM -0800, Andrew Morton wrote:
> On Mon, 12 Nov 2012 14:14:43 +0400
> Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>
> > This file handle will be used in /proc/pid/fdinfo/fd
> > output, which in turn will allow to restore a watch
> > target after checkpoint (thus it's provided for
> > CONFIG_CHECKPOINT_RESTORE only).
>
> This changelog isn't very good.
>
> What appears to be happening is that you're borrowing exportfs's
> ability to encode file handles and this is being reused to transport
> inotify fd's to userspace for c/r? Or something else - I didn't try
> very hard. Please explain better?
Yes, we use fhandle mechanism to encode a watch target. This allows us
to remember fhandle on dump and open() it at restore time (ie when we do
restore a target we use open_by_handle_at syscall with fhandle).
I'll update the changelog and send it.
> > --- linux-2.6.git.orig/fs/notify/inotify/inotify.h
> > +++ linux-2.6.git/fs/notify/inotify/inotify.h
> > @@ -1,6 +1,7 @@
> > #include <linux/fsnotify_backend.h>
> > #include <linux/inotify.h>
> > #include <linux/slab.h> /* struct kmem_cache */
> > +#include <linux/exportfs.h>
> >
> > extern struct kmem_cache *event_priv_cachep;
> >
> > @@ -9,9 +10,16 @@ struct inotify_event_private_data {
> > int wd;
> > };
> >
> > +#if defined(CONFIG_PROC_FS) && defined(CONFIG_EXPORTFS) && defined(CONFIG_CHECKPOINT_RESTORE)
> > +# define INOTIFY_USE_FHANDLE
> > +#endif
>
> Does this mean that c/r will fail to work properly if
> CONFIG_EXPORTFS=n? If so, that should be expressed in Kconfig
> dependencies?
Well, strictly speaking -- yes. We need exportfs to be compiled in.
But note the c/r will fail iif the task we're dumping does use
inotify. if there is no inotify usage -- then nothing will fail
even if exportfs is not compiled in. Also in our tool itself we
provide the "check" command which does verify if all features
we need are provided by the kernel. I'll think about adding
this config entry to deps. Thanks!
> > struct inotify_inode_mark {
> > struct fsnotify_mark fsn_mark;
> > int wd;
> > +#ifdef INOTIFY_USE_FHANDLE
> > + __u8 fhandle[sizeof(struct file_handle) + MAX_HANDLE_SZ];
> > +#endif
> > };
>
> Whoa. This adds 128+8 bytes to the inotify_inode_mark. That's a lot of
> bloat, and there can be a lot of inotify_inode_mark's in the system?
Yes, that's why it's not turned on by default, only if is c/r turned on.
iirc I've been said that usually only about 40 bytes is used (in the tests
I met only 8 bytes). Letme re-check all things.
> Also, what about alignment? That embedded `struct file_handle' will
> have a 4-byte alignment and if there's anything in there which is an
> 8-byte quantity then some architectures (ia64?) might get upset about
> the kernel-mode unaligned access. It appears that this won't happen in
> the present code, but are we future-proof?
>
> Why did you use a __u8, anyway? Could have done something like
>
> struct {
> struct file_handle fhandle;
> u8 pad[MAX_HANDLE_SZ];
> };
>
> and got some additional type safety and less typecasting?
Good point! Agreed on all. I'll update, thanks!
> > +#ifdef INOTIFY_USE_FHANDLE
> > +static int inotify_encode_wd_fhandle(struct inotify_inode_mark *mark, struct dentry *dentry)
> > +{
> > + struct file_handle *fhandle = (struct file_handle *)mark->fhandle;
> > + int size, ret;
> > +
> > + BUILD_BUG_ON(sizeof(mark->fhandle) <= sizeof(struct file_handle));
> > +
> > + fhandle->handle_bytes = sizeof(mark->fhandle) - sizeof(struct file_handle);
> > + size = fhandle->handle_bytes >> 2;
> > +
> > + ret = exportfs_encode_fh(dentry, (struct fid *)fhandle->f_handle, &size, 0);
> > + if ((ret == 255) || (ret == -ENOSPC))
>
> Sigh. ENOSPC means "your disk is full". If this error ever gets back
> to userspace, our poor user will go and provision more disks and then
> wonder why that didn't fix anything.
Hmm, this errno is returned from exportfs_encode_fh. Letme think which one
is better here. I'll update. Thanks!
>
> > + return -EOVERFLOW;
>
> That doesn't seem very appropriate either, unsure.
Cyrill
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 7:20 ` Cyrill Gorcunov
@ 2012-11-13 7:40 ` Andrew Morton
2012-11-13 8:00 ` Cyrill Gorcunov
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Morton @ 2012-11-13 7:40 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Tue, 13 Nov 2012 11:20:57 +0400 Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> > > struct inotify_inode_mark {
> > > struct fsnotify_mark fsn_mark;
> > > int wd;
> > > +#ifdef INOTIFY_USE_FHANDLE
> > > + __u8 fhandle[sizeof(struct file_handle) + MAX_HANDLE_SZ];
> > > +#endif
> > > };
> >
> > Whoa. This adds 128+8 bytes to the inotify_inode_mark. That's a lot of
> > bloat, and there can be a lot of inotify_inode_mark's in the system?
>
> Yes, that's why it's not turned on by default, only if is c/r turned on.
> iirc I've been said that usually only about 40 bytes is used (in the tests
> I met only 8 bytes). Letme re-check all things.
The question is, how many `struct inotify_inode_mark's are instantiated
system-wide? Could be millions?
Dumb question: do we really need inotify_inode_mark.fhandle at all?
What prevents us from assembling this info on demand when ->show_fdinfo() is
called?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 7:40 ` Andrew Morton
@ 2012-11-13 8:00 ` Cyrill Gorcunov
2012-11-13 8:19 ` David Rientjes
2012-11-13 22:38 ` Andrew Morton
0 siblings, 2 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-13 8:00 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, Nov 12, 2012 at 11:40:01PM -0800, Andrew Morton wrote:
> On Tue, 13 Nov 2012 11:20:57 +0400 Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> > >
> > > Whoa. This adds 128+8 bytes to the inotify_inode_mark. That's a lot of
> > > bloat, and there can be a lot of inotify_inode_mark's in the system?
> >
> > Yes, that's why it's not turned on by default, only if is c/r turned on.
> > iirc I've been said that usually only about 40 bytes is used (in the tests
> > I met only 8 bytes). Letme re-check all things.
>
> The question is, how many `struct inotify_inode_mark's are instantiated
> system-wide? Could be millions?
Well, hard to tell, to be fair. On my testing machine only apache has been
using inotify system as far as I remember, but for sure nothing except memory
limit the number of inotify. But I think if one running machine with millions
of inotify it's rather powerful machine with enough memory.
> Dumb question: do we really need inotify_inode_mark.fhandle at all?
> What prevents us from assembling this info on demand when ->show_fdinfo() is
> called?
exportfs requires the dentry to be passed as an argument while inotify works
with inodes instead and at moment of show-fdinfo the target dentry might be
already deleted but inode yet present as far as I remember.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 8:00 ` Cyrill Gorcunov
@ 2012-11-13 8:19 ` David Rientjes
2012-11-13 8:29 ` Cyrill Gorcunov
2012-11-13 22:38 ` Andrew Morton
1 sibling, 1 reply; 44+ messages in thread
From: David Rientjes @ 2012-11-13 8:19 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Andrew Morton, linux-kernel, linux-fsdevel, Al Viro,
Alexey Dobriyan, Pavel Emelyanov, James Bottomley,
Matthew Helsley, aneesh.kumar, bfields
On Tue, 13 Nov 2012, Cyrill Gorcunov wrote:
> > The question is, how many `struct inotify_inode_mark's are instantiated
> > system-wide? Could be millions?
>
> Well, hard to tell, to be fair. On my testing machine only apache has been
> using inotify system as far as I remember, but for sure nothing except memory
> limit the number of inotify. But I think if one running machine with millions
> of inotify it's rather powerful machine with enough memory.
>
Seems easy to determine if you boot with slub_nomerge on the command line
and then read /sys/kernel/slab/inotify_inode_mark/objects. On my system,
that happens to be 210, but I'm sure you could come up with a realistic
synthetic workload to make it much higher.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 8:19 ` David Rientjes
@ 2012-11-13 8:29 ` Cyrill Gorcunov
[not found] ` <2910785.4Vm74eFJyi@deuteros>
0 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-13 8:29 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, linux-kernel, linux-fsdevel, Al Viro,
Alexey Dobriyan, Pavel Emelyanov, James Bottomley,
Matthew Helsley, aneesh.kumar, bfields
On Tue, Nov 13, 2012 at 12:19:25AM -0800, David Rientjes wrote:
> On Tue, 13 Nov 2012, Cyrill Gorcunov wrote:
>
> > > The question is, how many `struct inotify_inode_mark's are instantiated
> > > system-wide? Could be millions?
> >
> > Well, hard to tell, to be fair. On my testing machine only apache has been
> > using inotify system as far as I remember, but for sure nothing except memory
> > limit the number of inotify. But I think if one running machine with millions
> > of inotify it's rather powerful machine with enough memory.
> >
>
> Seems easy to determine if you boot with slub_nomerge on the command line
> and then read /sys/kernel/slab/inotify_inode_mark/objects. On my system,
> that happens to be 210, but I'm sure you could come up with a realistic
> synthetic workload to make it much higher.
Which would give about 26K of additional memory if c/r get used here. Not a
big number i guess?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 8:00 ` Cyrill Gorcunov
2012-11-13 8:19 ` David Rientjes
@ 2012-11-13 22:38 ` Andrew Morton
2012-11-14 6:46 ` Cyrill Gorcunov
1 sibling, 1 reply; 44+ messages in thread
From: Andrew Morton @ 2012-11-13 22:38 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Tue, 13 Nov 2012 12:00:32 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> > Dumb question: do we really need inotify_inode_mark.fhandle at all?
> > What prevents us from assembling this info on demand when ->show_fdinfo() is
> > called?
>
> exportfs requires the dentry to be passed as an argument while inotify works
> with inodes instead and at moment of show-fdinfo the target dentry might be
> already deleted but inode yet present as far as I remember.
How can the c/r restore code reestablish the inode data if the dentry
isn't there any more?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-13 22:38 ` Andrew Morton
@ 2012-11-14 6:46 ` Cyrill Gorcunov
2012-11-14 10:10 ` Pavel Emelyanov
0 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-14 6:46 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Tue, Nov 13, 2012 at 02:38:08PM -0800, Andrew Morton wrote:
> On Tue, 13 Nov 2012 12:00:32 +0400
> Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>
> > > Dumb question: do we really need inotify_inode_mark.fhandle at all?
> > > What prevents us from assembling this info on demand when ->show_fdinfo() is
> > > called?
> >
> > exportfs requires the dentry to be passed as an argument while inotify works
> > with inodes instead and at moment of show-fdinfo the target dentry might be
> > already deleted but inode yet present as far as I remember.
>
> How can the c/r restore code reestablish the inode data if the dentry
> isn't there any more?
By "deleted" I meant deleted from dcache, thus when we call for
open_by_handle_at with fhandle, the kernel reconstruct the path
and we simply read the /proc/self/fd/ link, and then pass this
path to inotify_add_watch.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-14 6:46 ` Cyrill Gorcunov
@ 2012-11-14 10:10 ` Pavel Emelyanov
2012-11-14 10:13 ` Cyrill Gorcunov
0 siblings, 1 reply; 44+ messages in thread
From: Pavel Emelyanov @ 2012-11-14 10:10 UTC (permalink / raw)
To: Cyrill Gorcunov, Andrew Morton
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields
On 11/14/2012 10:46 AM, Cyrill Gorcunov wrote:
> On Tue, Nov 13, 2012 at 02:38:08PM -0800, Andrew Morton wrote:
>> On Tue, 13 Nov 2012 12:00:32 +0400
>> Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>>
>>>> Dumb question: do we really need inotify_inode_mark.fhandle at all?
>>>> What prevents us from assembling this info on demand when ->show_fdinfo() is
>>>> called?
>>>
>>> exportfs requires the dentry to be passed as an argument while inotify works
>>> with inodes instead and at moment of show-fdinfo the target dentry might be
>>> already deleted but inode yet present as far as I remember.
>>
>> How can the c/r restore code reestablish the inode data if the dentry
>> isn't there any more?
>
> By "deleted" I meant deleted from dcache, thus when we call for
> open_by_handle_at with fhandle, the kernel reconstruct the path
> and we simply read the /proc/self/fd/ link, and then pass this
> path to inotify_add_watch.
No we don't do readlink as the path we'd see would be empty. Instead after
we called the open_by_handle_at, we pass the "/proc/self/fd/<fd>" _path_ itself
to inotify_add_watch. The path resolution code follows the link properly and
adds the target inode into the watch list.
> .
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark
2012-11-14 10:10 ` Pavel Emelyanov
@ 2012-11-14 10:13 ` Cyrill Gorcunov
0 siblings, 0 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-14 10:13 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Andrew Morton, linux-kernel, linux-fsdevel, Al Viro,
Alexey Dobriyan, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Wed, Nov 14, 2012 at 02:10:50PM +0400, Pavel Emelyanov wrote:
> >>
> >> How can the c/r restore code reestablish the inode data if the dentry
> >> isn't there any more?
> >
> > By "deleted" I meant deleted from dcache, thus when we call for
> > open_by_handle_at with fhandle, the kernel reconstruct the path
> > and we simply read the /proc/self/fd/ link, and then pass this
> > path to inotify_add_watch.
>
> No we don't do readlink as the path we'd see would be empty. Instead after
> we called the open_by_handle_at, we pass the "/proc/self/fd/<fd>" _path_ itself
> to inotify_add_watch. The path resolution code follows the link properly and
> adds the target inode into the watch list.
Yeah, sorry for confusion.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [patch 4/7] fs, notify: Add procfs fdinfo helper v4
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
` (2 preceding siblings ...)
2012-11-12 10:14 ` [patch 3/7] fs, notify: Add file handle entry into inotify_inode_mark Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-13 1:00 ` Andrew Morton
2012-11-12 10:14 ` [patch 5/7] fs, eventfd: Add procfs fdinfo helper Cyrill Gorcunov
` (2 subsequent siblings)
6 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro
[-- Attachment #1: seq-fdinfo-fsnotify-8 --]
[-- Type: text/plain, Size: 8001 bytes --]
This allow us to print out fsnotify details such as
watchee inode, device, mask and optionally a file handle.
For inotify objects if kernel compiled with exportfs support
the output will be
| pos: 0
| flags: 02000000
| inotify wd: 3 ino: 9e7e sdev: 800013 mask: 800afce ignored_mask: 0 fhandle-bytes: 8 fhandle-type: 1 f_handle: 7e9e0000640d1b6d
| inotify wd: 2 ino: a111 sdev: 800013 mask: 800afce ignored_mask: 0 fhandle-bytes: 8 fhandle-type: 1 f_handle: 11a1000020542153
| inotify wd: 1 ino: 6b149 sdev: 800013 mask: 800afce ignored_mask: 0 fhandle-bytes: 8 fhandle-type: 1 f_handle: 49b1060023552153
If kernel compiled without exportfs support, the file handle
won't be provided but inode and device only.
| pos: 0
| flags: 02000000
| inotify wd: 3 ino: 9e7e sdev: 800013 mask: 800afce ignored_mask: 0
| inotify wd: 2 ino: a111 sdev: 800013 mask: 800afce ignored_mask: 0
| inotify wd: 1 ino: 6b149 sdev: 800013 mask: 800afce ignored_mask: 0
For fanotify the output is like
| pos: 0
| flags: 02
| fanotify ino: 68f71 sdev: 800013 mask: 1 ignored_mask: 40000000
| fanotify mnt_id: 13 mask: 1 ignored_mask: 40000000
To minimize impact on general fsnotify code the new functionality
is gathered in fs/notify/fdinfo.c file.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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>
---
fs/notify/Makefile | 2
fs/notify/fanotify/fanotify_user.c | 4 +
fs/notify/fdinfo.c | 124 +++++++++++++++++++++++++++++++++++++
fs/notify/fdinfo.h | 22 ++++++
fs/notify/inotify/inotify_user.c | 4 +
5 files changed, 155 insertions(+), 1 deletion(-)
Index: linux-2.6.git/fs/notify/Makefile
===================================================================
--- linux-2.6.git.orig/fs/notify/Makefile
+++ linux-2.6.git/fs/notify/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_FSNOTIFY) += fsnotify.o notification.o group.o inode_mark.o \
- mark.o vfsmount_mark.o
+ mark.o vfsmount_mark.o fdinfo.o
obj-y += dnotify/
obj-y += inotify/
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
@@ -17,6 +17,7 @@
#include <asm/ioctls.h>
#include "../../mount.h"
+#include "../fdinfo.h"
#define FANOTIFY_DEFAULT_MAX_EVENTS 16384
#define FANOTIFY_DEFAULT_MAX_MARKS 8192
@@ -427,6 +428,9 @@ static long fanotify_ioctl(struct file *
}
static const struct file_operations fanotify_fops = {
+#ifdef CONFIG_PROC_FS
+ .show_fdinfo = fanotify_show_fdinfo,
+#endif
.poll = fanotify_poll,
.read = fanotify_read,
.write = fanotify_write,
Index: linux-2.6.git/fs/notify/fdinfo.c
===================================================================
--- /dev/null
+++ linux-2.6.git/fs/notify/fdinfo.c
@@ -0,0 +1,124 @@
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/fsnotify_backend.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/inotify.h>
+#include <linux/kernel.h>
+#include <linux/namei.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <linux/seq_file.h>
+#include <linux/proc_fs.h>
+
+#include "inotify/inotify.h"
+#include "../fs/mount.h"
+
+#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))
+{
+ struct fsnotify_group *group = f->private_data;
+ struct fsnotify_mark *mark;
+ int ret = 0;
+
+ spin_lock(&group->mark_lock);
+ list_for_each_entry(mark, &group->marks_list, g_list) {
+ ret = show(m, mark);
+ if (ret)
+ break;
+ }
+ spin_unlock(&group->mark_lock);
+ return ret;
+}
+
+#ifdef CONFIG_INOTIFY_USER
+
+static int inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
+{
+ struct inotify_inode_mark *inode_mark;
+ struct inode *inode;
+ int ret = 0;
+
+ if (!(mark->flags & (FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_INODE)))
+ return 0;
+
+ inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark);
+ inode = igrab(mark->i.inode);
+ if (inode) {
+ ret = seq_printf(m, "inotify wd: %8d ino: %16lx sdev: %8x "
+ "mask: %8x ignored_mask: %8x ",
+ inode_mark->wd, inode->i_ino,
+ inode->i_sb->s_dev,
+ mark->mask, mark->ignored_mask);
+#ifdef INOTIFY_USE_FHANDLE
+ if (!ret) {
+ int i;
+ struct file_handle *fhandle = (struct file_handle *)inode_mark->fhandle;
+ ret = seq_printf(m, "fhandle-bytes: %8x "
+ "fhandle-type: %8x f_handle: ",
+ fhandle->handle_bytes,
+ fhandle->handle_type);
+
+ for (i = 0; i < fhandle->handle_bytes; i++) {
+ ret |= seq_printf(m, "%02x",
+ (int)(unsigned char)fhandle->f_handle[i]);
+ }
+ }
+#endif
+ ret |= seq_putc(m, '\n');
+ iput(inode);
+ }
+
+ return ret;
+}
+
+int inotify_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ return show_fdinfo(m, f, inotify_fdinfo);
+}
+
+#endif /* CONFIG_INOTIFY_USER */
+
+#ifdef CONFIG_FANOTIFY
+
+static int fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
+{
+ struct inode *inode;
+ int ret = 0;
+
+ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE))
+ return 0;
+
+ if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
+ inode = igrab(mark->i.inode);
+ if (!inode)
+ goto out;
+ ret = seq_printf(m, "fanotify ino: %16lx sdev: %8x "
+ "mask: %8x ignored_mask: %8x\n",
+ inode->i_ino, inode->i_sb->s_dev,
+ mark->mask, mark->ignored_mask);
+ iput(inode);
+ } else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT) {
+ struct mount *mnt = real_mount(mark->m.mnt);
+
+ ret = seq_printf(m, "fanotify mnt_id: %8x mask: %8x ignored_mask: %8x\n",
+ mnt->mnt_id, mark->mask, mark->ignored_mask);
+ }
+out:
+ return ret;
+}
+
+int fanotify_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ return show_fdinfo(m, f, fanotify_fdinfo);
+}
+
+#endif /* CONFIG_FANOTIFY */
+
+#endif /* CONFIG_INOTIFY_USER || CONFIG_FANOTIFY */
+
+#endif /* CONFIG_PROC_FS */
Index: linux-2.6.git/fs/notify/fdinfo.h
===================================================================
--- /dev/null
+++ linux-2.6.git/fs/notify/fdinfo.h
@@ -0,0 +1,22 @@
+#ifndef __FSNOTIFY_FDINFO_H__
+#define __FSNOTIFY_FDINFO_H__
+
+#include <linux/errno.h>
+#include <linux/proc_fs.h>
+
+struct seq_file;
+struct file;
+
+#ifdef CONFIG_PROC_FS
+
+#ifdef CONFIG_INOTIFY_USER
+extern int inotify_show_fdinfo(struct seq_file *m, struct file *f);
+#endif
+
+#ifdef CONFIG_FANOTIFY
+extern int fanotify_show_fdinfo(struct seq_file *m, struct file *f);
+#endif
+
+#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
@@ -40,6 +40,7 @@
#include <linux/wait.h>
#include "inotify.h"
+#include "../fdinfo.h"
#include <asm/ioctls.h>
@@ -335,6 +336,9 @@ static long inotify_ioctl(struct file *f
}
static const struct file_operations inotify_fops = {
+#ifdef CONFIG_PROC_FS
+ .show_fdinfo = inotify_show_fdinfo,
+#endif
.poll = inotify_poll,
.read = inotify_read,
.fasync = inotify_fasync,
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 4/7] fs, notify: Add procfs fdinfo helper v4
2012-11-12 10:14 ` [patch 4/7] fs, notify: Add procfs fdinfo helper v4 Cyrill Gorcunov
@ 2012-11-13 1:00 ` Andrew Morton
2012-11-13 7:22 ` Cyrill Gorcunov
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Morton @ 2012-11-13 1:00 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, 12 Nov 2012 14:14:44 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> This allow us to print out fsnotify details such as
> watchee inode, device, mask and optionally a file handle.
>
> For inotify objects if kernel compiled with exportfs support
> the output will be
>
> | pos: 0
> | flags: 02000000
> | inotify wd: 3 ino: 9e7e sdev: 800013 mask: 800afce ignored_mask: 0 fhandle-bytes: 8 fhandle-type: 1 f_handle: 7e9e0000640d1b6d
> | inotify wd: 2 ino: a111 sdev: 800013 mask: 800afce ignored_mask: 0 fhandle-bytes: 8 fhandle-type: 1 f_handle: 11a1000020542153
> | inotify wd: 1 ino: 6b149 sdev: 800013 mask: 800afce ignored_mask: 0 fhandle-bytes: 8 fhandle-type: 1 f_handle: 49b1060023552153
>
> If kernel compiled without exportfs support, the file handle
> won't be provided but inode and device only.
>
> | pos: 0
> | flags: 02000000
> | inotify wd: 3 ino: 9e7e sdev: 800013 mask: 800afce ignored_mask: 0
> | inotify wd: 2 ino: a111 sdev: 800013 mask: 800afce ignored_mask: 0
> | inotify wd: 1 ino: 6b149 sdev: 800013 mask: 800afce ignored_mask: 0
>
> For fanotify the output is like
>
> | pos: 0
> | flags: 02
> | fanotify ino: 68f71 sdev: 800013 mask: 1 ignored_mask: 40000000
> | fanotify mnt_id: 13 mask: 1 ignored_mask: 40000000
>
> To minimize impact on general fsnotify code the new functionality
> is gathered in fs/notify/fdinfo.c file.
>
>
> ...
>
> --- /dev/null
> +++ linux-2.6.git/fs/notify/fdinfo.h
> @@ -0,0 +1,22 @@
> +#ifndef __FSNOTIFY_FDINFO_H__
> +#define __FSNOTIFY_FDINFO_H__
> +
> +#include <linux/errno.h>
> +#include <linux/proc_fs.h>
> +
> +struct seq_file;
> +struct file;
> +
> +#ifdef CONFIG_PROC_FS
> +
> +#ifdef CONFIG_INOTIFY_USER
> +extern int inotify_show_fdinfo(struct seq_file *m, struct file *f);
> +#endif
> +#ifdef CONFIG_FANOTIFY
> +extern int fanotify_show_fdinfo(struct seq_file *m, struct file *f);
> +#endif
#else /* CONFIG_PROC_FS */
#define inotify_show_fdinfo NULL
#define fanotify_show_fdinfo NULL
#endif /* CONFIG_PROC_FS */
> +
>
> ...
>
> @@ -335,6 +336,9 @@ static long inotify_ioctl(struct file *f
> }
>
> static const struct file_operations inotify_fops = {
> +#ifdef CONFIG_PROC_FS
> + .show_fdinfo = inotify_show_fdinfo,
> +#endif
> .poll = inotify_poll,
> .read = inotify_read,
> .fasync = inotify_fasync,
> @@ -427,6 +428,9 @@ static long fanotify_ioctl(struct file *
> }
>
> static const struct file_operations fanotify_fops = {
> +#ifdef CONFIG_PROC_FS
> + .show_fdinfo = fanotify_show_fdinfo,
> +#endif
> .poll = fanotify_poll,
> .read = fanotify_read,
> .write = fanotify_write,
Then remove these ifdefs.
That's if you can be bothered. It's a bit of a party trick which
doesn't make things much clearer IMO.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 4/7] fs, notify: Add procfs fdinfo helper v4
2012-11-13 1:00 ` Andrew Morton
@ 2012-11-13 7:22 ` Cyrill Gorcunov
0 siblings, 0 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-13 7:22 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, Nov 12, 2012 at 05:00:17PM -0800, Andrew Morton wrote:
> >
> > static const struct file_operations inotify_fops = {
> > +#ifdef CONFIG_PROC_FS
> > + .show_fdinfo = inotify_show_fdinfo,
> > +#endif
> > .poll = inotify_poll,
> > .read = inotify_read,
> > .fasync = inotify_fasync,
> > @@ -427,6 +428,9 @@ static long fanotify_ioctl(struct file *
> > }
> >
> > static const struct file_operations fanotify_fops = {
> > +#ifdef CONFIG_PROC_FS
> > + .show_fdinfo = fanotify_show_fdinfo,
> > +#endif
> > .poll = fanotify_poll,
> > .read = fanotify_read,
> > .write = fanotify_write,
>
> Then remove these ifdefs.
>
> That's if you can be bothered. It's a bit of a party trick which
> doesn't make things much clearer IMO.
Sure, i'll update.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [patch 5/7] fs, eventfd: Add procfs fdinfo helper
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
` (3 preceding siblings ...)
2012-11-12 10:14 ` [patch 4/7] fs, notify: Add procfs fdinfo helper v4 Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-12 10:14 ` [patch 6/7] fs, epoll: Add procfs fdinfo helper v2 Cyrill Gorcunov
2012-11-12 10:14 ` [patch 7/7] fdinfo: Show sigmask for signalfd fd v2 Cyrill Gorcunov
6 siblings, 0 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro
[-- Attachment #1: seq-fdinfo-eventfd-7 --]
[-- Type: text/plain, Size: 1727 bytes --]
This allow us to print out raw counter value.
The /proc/pid/fdinfo/fd output is
| pos: 0
| flags: 04002
| eventfd-count: 5a
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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>
---
fs/eventfd.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
Index: linux-2.6.git/fs/eventfd.c
===================================================================
--- linux-2.6.git.orig/fs/eventfd.c
+++ linux-2.6.git/fs/eventfd.c
@@ -19,6 +19,8 @@
#include <linux/export.h>
#include <linux/kref.h>
#include <linux/eventfd.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
struct eventfd_ctx {
struct kref kref;
@@ -284,7 +286,25 @@ static ssize_t eventfd_write(struct file
return res;
}
+#ifdef CONFIG_PROC_FS
+static int eventfd_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ struct eventfd_ctx *ctx = f->private_data;
+ int ret;
+
+ spin_lock_irq(&ctx->wqh.lock);
+ ret = seq_printf(m, "eventfd-count: %16llx\n",
+ (unsigned long long)ctx->count);
+ spin_unlock_irq(&ctx->wqh.lock);
+
+ return ret;
+}
+#endif
+
static const struct file_operations eventfd_fops = {
+#ifdef CONFIG_PROC_FS
+ .show_fdinfo = eventfd_show_fdinfo,
+#endif
.release = eventfd_release,
.poll = eventfd_poll,
.read = eventfd_read,
^ permalink raw reply [flat|nested] 44+ messages in thread
* [patch 6/7] fs, epoll: Add procfs fdinfo helper v2
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
` (4 preceding siblings ...)
2012-11-12 10:14 ` [patch 5/7] fs, eventfd: Add procfs fdinfo helper Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-12 10:14 ` [patch 7/7] fdinfo: Show sigmask for signalfd fd v2 Cyrill Gorcunov
6 siblings, 0 replies; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro, Andrey Vagin
[-- Attachment #1: seq-fdinfo-eventpoll-8 --]
[-- Type: text/plain, Size: 2202 bytes --]
This allow us to print out eventpoll target file descriptor,
events and data, the /proc/pid/fdinfo/fd consists of
| pos: 0
| flags: 02
| tfd: 5 events: 1d data: ffffffffffffffff enabled: 1
[avagin@: fix for unitialized ret variable]
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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: Andrey Vagin <avagin@openvz.org>
---
fs/eventpoll.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
Index: linux-2.6.git/fs/eventpoll.c
===================================================================
--- linux-2.6.git.orig/fs/eventpoll.c
+++ linux-2.6.git/fs/eventpoll.c
@@ -38,6 +38,8 @@
#include <asm/io.h>
#include <asm/mman.h>
#include <linux/atomic.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
/*
* LOCKING:
@@ -783,8 +785,35 @@ static unsigned int ep_eventpoll_poll(st
return pollflags != -1 ? pollflags : 0;
}
+#ifdef CONFIG_PROC_FS
+static int ep_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ struct eventpoll *ep = f->private_data;
+ struct rb_node *rbp;
+ int ret = 0;
+
+ 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 enabled: %d\n",
+ epi->ffd.fd, epi->event.events,
+ (long long)epi->event.data,
+ ep_is_linked(&epi->rdllink));
+ if (ret)
+ break;
+ }
+ mutex_unlock(&ep->mtx);
+
+ return ret;
+}
+#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,
+#endif
.release = ep_eventpoll_release,
.poll = ep_eventpoll_poll,
.llseek = noop_llseek,
^ permalink raw reply [flat|nested] 44+ messages in thread
* [patch 7/7] fdinfo: Show sigmask for signalfd fd v2
2012-11-12 10:14 [patch 0/7] Providing additional information in fdinfo sufficient for c/r Cyrill Gorcunov
` (5 preceding siblings ...)
2012-11-12 10:14 ` [patch 6/7] fs, epoll: Add procfs fdinfo helper v2 Cyrill Gorcunov
@ 2012-11-12 10:14 ` Cyrill Gorcunov
2012-11-13 1:05 ` Andrew Morton
6 siblings, 1 reply; 44+ messages in thread
From: Cyrill Gorcunov @ 2012-11-12 10:14 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel
Cc: Al Viro, Alexey Dobriyan, Andrew Morton, Pavel Emelyanov,
James Bottomley, Matthew Helsley, aneesh.kumar, bfields,
Cyrill Gorcunov, Al Viro
[-- Attachment #1: seq-fdinfo-signalfd-3 --]
[-- Type: text/plain, Size: 3312 bytes --]
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Al Viro <viro@ZenIV.linux.org.uk>
CC: Alexey Dobriyan <adobriyan@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
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>
---
fs/proc/array.c | 2 +-
fs/signalfd.c | 26 ++++++++++++++++++++++++++
include/linux/proc_fs.h | 3 +++
3 files changed, 30 insertions(+), 1 deletion(-)
Index: linux-2.6.git/fs/proc/array.c
===================================================================
--- linux-2.6.git.orig/fs/proc/array.c
+++ linux-2.6.git/fs/proc/array.c
@@ -220,7 +220,7 @@ static inline void task_state(struct seq
seq_putc(m, '\n');
}
-static void render_sigset_t(struct seq_file *m, const char *header,
+void render_sigset_t(struct seq_file *m, const char *header,
sigset_t *set)
{
int i;
Index: linux-2.6.git/fs/signalfd.c
===================================================================
--- linux-2.6.git.orig/fs/signalfd.c
+++ linux-2.6.git/fs/signalfd.c
@@ -29,6 +29,7 @@
#include <linux/anon_inodes.h>
#include <linux/signalfd.h>
#include <linux/syscalls.h>
+#include <linux/proc_fs.h>
void signalfd_cleanup(struct sighand_struct *sighand)
{
@@ -46,6 +47,7 @@ void signalfd_cleanup(struct sighand_str
}
struct signalfd_ctx {
+ seqcount_t cnt;
sigset_t sigmask;
};
@@ -227,7 +229,28 @@ static ssize_t signalfd_read(struct file
return total ? total: ret;
}
+#ifdef CONFIG_PROC_FS
+static int signalfd_show_fdinfo(struct seq_file *m, struct file *f)
+{
+ struct signalfd_ctx *ctx = f->private_data;
+ sigset_t sigmask;
+ unsigned seq;
+
+ do {
+ seq = read_seqcount_begin(&ctx->cnt);
+ sigmask = ctx->sigmask;
+ } while (read_seqcount_retry(&ctx->cnt, seq));
+
+ signotset(&sigmask);
+ render_sigset_t(m, "sigmask:\t", &sigmask);
+ return 0;
+}
+#endif
+
static const struct file_operations signalfd_fops = {
+#ifdef CONFIG_PROC_FS
+ .show_fdinfo = signalfd_show_fdinfo,
+#endif
.release = signalfd_release,
.poll = signalfd_poll,
.read = signalfd_read,
@@ -259,6 +282,7 @@ SYSCALL_DEFINE4(signalfd4, int, ufd, sig
return -ENOMEM;
ctx->sigmask = sigmask;
+ seqcount_init(&ctx->cnt);
/*
* When we call this, the initialization must be complete, since
@@ -278,7 +302,9 @@ SYSCALL_DEFINE4(signalfd4, int, ufd, sig
return -EINVAL;
}
spin_lock_irq(¤t->sighand->siglock);
+ write_seqcount_begin(&ctx->cnt);
ctx->sigmask = sigmask;
+ write_seqcount_end(&ctx->cnt);
spin_unlock_irq(¤t->sighand->siglock);
wake_up(¤t->sighand->signalfd_wqh);
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
@@ -290,4 +290,7 @@ static inline struct net *PDE_NET(struct
return pde->parent->data;
}
+#include <asm/signal.h>
+
+void render_sigset_t(struct seq_file *m, const char *header, sigset_t *set);
#endif /* _LINUX_PROC_FS_H */
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [patch 7/7] fdinfo: Show sigmask for signalfd fd v2
2012-11-12 10:14 ` [patch 7/7] fdinfo: Show sigmask for signalfd fd v2 Cyrill Gorcunov
@ 2012-11-13 1:05 ` Andrew Morton
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Morton @ 2012-11-13 1:05 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-fsdevel, Al Viro, Alexey Dobriyan,
Pavel Emelyanov, James Bottomley, Matthew Helsley, aneesh.kumar,
bfields
On Mon, 12 Nov 2012 14:14:47 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>
> ...
>
> --- linux-2.6.git.orig/include/linux/proc_fs.h
> +++ linux-2.6.git/include/linux/proc_fs.h
> @@ -290,4 +290,7 @@ static inline struct net *PDE_NET(struct
> return pde->parent->data;
> }
>
> +#include <asm/signal.h>
> +
> +void render_sigset_t(struct seq_file *m, const char *header, sigset_t *set);
> #endif /* _LINUX_PROC_FS_H */
hm. Are you sure asm/signal.h is the way to get sigset_t? "grep -l
sigset_t arch/*/include/asm/signal.h" is unpromising and
kernel/signal.c doesn't do it this way?
^ permalink raw reply [flat|nested] 44+ messages in thread