[PATCH] procfs fixes for inotify/dnotify

[PATCH] procfs fixes for inotify/dnotify

[Thomas Ogrisegg]

Problem description:
When a new process is created, the creation of the respective PID
subdirectory of /proc is deferred until the /proc-directory is beeing
read (by e.g. ps(1)). This causes file notification frameworks like
dnotify and inotify to not work correctly with /proc.

This patch fixes the problem.

Signed-off-by: Thomas Ogrisegg <tom-lkml@lkml.fnord.at>

Re: [PATCH] procfs fixes for inotify/dnotify

[Thomas Ogrisegg]

[-- Attachment #1: Type: text/plain, Size: 27 bytes --]

Sorry, forgot the patch...

[-- Attachment #2: inotify.diff --]
[-- Type: text/plain, Size: 1917 bytes --]

diff -uNr -X linux-2.6.15/Documentation/dontdiff linux-2.6.15/kernel/fork.c linux-2.6.15.4/kernel/fork.c
--- linux-2.6.15/kernel/fork.c	2006-01-03 04:21:10.000000000 +0100
+++ linux-2.6.15.4/kernel/fork.c	2006-02-18 14:09:54.000000000 +0100
@@ -43,6 +43,7 @@
 #include <linux/rmap.h>
 #include <linux/acct.h>
 #include <linux/cn_proc.h>
+#include <linux/proc_fs.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1141,6 +1142,7 @@
 	write_unlock_irq(&tasklist_lock);
 	proc_fork_connector(p);
 	cpuset_fork(p);
+	proc_root_notify(pid);
 	retval = 0;
 
 fork_out:
--- linux-2.6.15/include/linux/proc_fs.h	2006-01-03 04:21:10.000000000 +0100
+++ linux-2.6.15.4/include/linux/proc_fs.h	2006-02-18 13:55:52.000000000 +0100
@@ -4,6 +4,8 @@
 #include <linux/config.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
+#include <linux/fsnotify.h>
+#include <linux/mount.h>
 #include <asm/atomic.h>
 
 /*
@@ -194,6 +196,13 @@
 	remove_proc_entry(name,proc_net);
 }
 
+static inline void proc_root_notify (pid_t pid)
+{
+	char buf[20];
+	snprintf (buf, sizeof (buf), "%d", pid);
+	fsnotify_mkdir (proc_mnt->mnt_root->d_inode, buf);
+}
+
 #else
 
 #define proc_root_driver NULL
@@ -224,6 +233,8 @@
 	mode_t mode, struct proc_dir_entry *base, get_info_t *get_info)
 	{ return NULL; }
 
+static inline void proc_root_notify (pid_t pid) {}
+
 struct tty_driver;
 static inline void proc_tty_register_driver(struct tty_driver *driver) {};
 static inline void proc_tty_unregister_driver(struct tty_driver *driver) {};
diff -uNr -X linux-2.6.15/Documentation/dontdiff linux-2.6.15/fs/proc/base.c linux-2.6.15.4/fs/proc/base.c
--- linux-2.6.15/fs/proc/base.c	2006-01-03 04:21:10.000000000 +0100
+++ linux-2.6.15.4/fs/proc/base.c	2006-02-19 10:44:28.000000000 +0100
@@ -1945,6 +1945,7 @@
 	if(proc_dentry != NULL) {
 		shrink_dcache_parent(proc_dentry);
 		dput(proc_dentry);
+		fsnotify_nameremove (proc_dentry, 1);
 	}
 }
 

Re: [PATCH] procfs fixes for inotify/dnotify

[Andrew Morton]

On Tue, 21 Feb 2006 22:16:23 +0100
Thomas Ogrisegg <tom-lkml@lkml.fnord.at> wrote:

> Problem description:
> When a new process is created, the creation of the respective PID
> subdirectory of /proc is deferred until the /proc-directory is beeing
> read (by e.g. ps(1)). This causes file notification frameworks like
> dnotify and inotify to not work correctly with /proc.
> 
> This patch fixes the problem.
> 
> Signed-off-by: Thomas Ogrisegg <tom-lkml@lkml.fnord.at>

Robin has raised this report:

	http://bugzilla.kernel.org/show_bug.cgi?id=8782

pointing at your 15-month-old patch, which seemed to have got lost.

The patch rather doesn't work any more.  If you have time to update it then
please do so, and copy John and Eric on the patch?  

Thanks.

A few things I noticed:

> 
> diff -uNr -X linux-2.6.15/Documentation/dontdiff linux-2.6.15/kernel/fork.c linux-2.6.15.4/kernel/fork.c
> --- linux-2.6.15/kernel/fork.c	2006-01-03 04:21:10.000000000 +0100
> +++ linux-2.6.15.4/kernel/fork.c	2006-02-18 14:09:54.000000000 +0100
> @@ -43,6 +43,7 @@
>  #include <linux/rmap.h>
>  #include <linux/acct.h>
>  #include <linux/cn_proc.h>
> +#include <linux/proc_fs.h>
>  
>  #include <asm/pgtable.h>
>  #include <asm/pgalloc.h>
> @@ -1141,6 +1142,7 @@
>  	write_unlock_irq(&tasklist_lock);
>  	proc_fork_connector(p);
>  	cpuset_fork(p);
> +	proc_root_notify(pid);
>  	retval = 0;
>  
>  fork_out:
> --- linux-2.6.15/include/linux/proc_fs.h	2006-01-03 04:21:10.000000000 +0100
> +++ linux-2.6.15.4/include/linux/proc_fs.h	2006-02-18 13:55:52.000000000 +0100
> @@ -4,6 +4,8 @@
>  #include <linux/config.h>
>  #include <linux/slab.h>
>  #include <linux/fs.h>
> +#include <linux/fsnotify.h>
> +#include <linux/mount.h>
>  #include <asm/atomic.h>
>  
>  /*
> @@ -194,6 +196,13 @@
>  	remove_proc_entry(name,proc_net);
>  }
>  
> +static inline void proc_root_notify (pid_t pid)
> +{
> +	char buf[20];
> +	snprintf (buf, sizeof (buf), "%d", pid);
> +	fsnotify_mkdir (proc_mnt->mnt_root->d_inode, buf);
> +}

This shouldn't inlined.

Don't put spaces between the function name and the (

>  #else
>  
>  #define proc_root_driver NULL
> @@ -224,6 +233,8 @@
>  	mode_t mode, struct proc_dir_entry *base, get_info_t *get_info)
>  	{ return NULL; }
>  
> +static inline void proc_root_notify (pid_t pid) {}

This can remain inlined in the header.

>  struct tty_driver;
>  static inline void proc_tty_register_driver(struct tty_driver *driver) {};
>  static inline void proc_tty_unregister_driver(struct tty_driver *driver) {};
> diff -uNr -X linux-2.6.15/Documentation/dontdiff linux-2.6.15/fs/proc/base.c linux-2.6.15.4/fs/proc/base.c
> --- linux-2.6.15/fs/proc/base.c	2006-01-03 04:21:10.000000000 +0100
> +++ linux-2.6.15.4/fs/proc/base.c	2006-02-19 10:44:28.000000000 +0100
> @@ -1945,6 +1945,7 @@
>  	if(proc_dentry != NULL) {
>  		shrink_dcache_parent(proc_dentry);
>  		dput(proc_dentry);
> +		fsnotify_nameremove (proc_dentry, 1);
>  	}

I wonder if this is racy - after the dput() we might no longer have a
reference?

Re: [PATCH] procfs fixes for inotify/dnotify

[Alexey Dobriyan]

On Thu, Jul 19, 2007 at 03:03:55PM -0700, Andrew Morton wrote:
> 	http://bugzilla.kernel.org/show_bug.cgi?id=8782

[fsnotify and /proc]

> > --- linux-2.6.15/fs/proc/base.c
> > +++ linux-2.6.15.4/fs/proc/base.c
> > @@ -1945,6 +1945,7 @@
> >  	if(proc_dentry != NULL) {
> >  		shrink_dcache_parent(proc_dentry);
> >  		dput(proc_dentry);
> > +		fsnotify_nameremove (proc_dentry, 1);
> >  	}
> 
> I wonder if this is racy - after the dput() we might no longer have a
> reference?

It is indeed racy. dput() => kill_it: => d_kill() => d_free() =>
__d_free() eventually. *boom*