2002-03-10 21:08:46

by Oskar Liljeblad

[permalink] [raw]
Subject: directory notifications lost after fork?

The code snipper demonstrates what I consider a bug in the
dnotify facilities in the kernel. After a fork, all registered
notifications are lost in the process where they originally
where registered (the parent process). "lost" here means that
the signal specified with F_SETSIG fcntl no longer is delivered
when notified.

How to reproduce (tested with 2.4.17):
gcc -o dnoticebug dnoticebug.c
dnoticebug & # run in background
cat dnoticebug.c >/dev/null # "notified" should now be printed
cat dnoticebug.c >/dev/null # nothing is printed this time

If you comment out the line with fork below, "notified" *will* be
printed every time you cat dnoticebug.c.

I'm not subscribed to the list so I'd appreciate if you CCed me.
(Otherwise I'd have to use the archives :) Thanks.

Oskar Liljeblad ([email protected])

===

#define _GNU_SOURCE
#include <fcntl.h>
#include <signal.h>
#include <stdio.h>
#include <unistd.h>

static void handler(int sig, siginfo_t *si, void *data)
{
printf("notified\n");
}

int main(void)
{
struct sigaction act;
int fd;

act.sa_sigaction = handler;
sigemptyset(&act.sa_mask);
act.sa_flags = SA_SIGINFO;
sigaction(SIGRTMIN, &act, NULL);

fd = open(".", O_RDONLY);
fcntl(fd, F_SETSIG, SIGRTMIN);
fcntl(fd, F_NOTIFY, DN_ACCESS|DN_MULTISHOT);

while (1) {
pause();
if (fork() <= 0) exit(0);
wait(NULL);
}
}


2002-03-11 07:42:28

by Alex Riesen

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

Just a "me too".
I have tried also to use the default (SIGIO) by setting owner pid (just
in case). It is all the same.
Does someone use the notifications, btw?
The whole thing seems somewhat untested.
-alex

On Sun, Mar 10, 2002 at 10:08:02PM +0100, Oskar Liljeblad wrote:
> The code snipper demonstrates what I consider a bug in the
> dnotify facilities in the kernel. After a fork, all registered
> notifications are lost in the process where they originally
> where registered (the parent process). "lost" here means that
> the signal specified with F_SETSIG fcntl no longer is delivered
> when notified.
>
> How to reproduce (tested with 2.4.17):
> gcc -o dnoticebug dnoticebug.c
> dnoticebug & # run in background
> cat dnoticebug.c >/dev/null # "notified" should now be printed
> cat dnoticebug.c >/dev/null # nothing is printed this time
>
> If you comment out the line with fork below, "notified" *will* be
> printed every time you cat dnoticebug.c.
>
> I'm not subscribed to the list so I'd appreciate if you CCed me.
> (Otherwise I'd have to use the archives :) Thanks.
>
> Oskar Liljeblad ([email protected])
>
> ===
>
> #define _GNU_SOURCE
> #include <fcntl.h>
> #include <signal.h>
> #include <stdio.h>
> #include <unistd.h>
>
> static void handler(int sig, siginfo_t *si, void *data)
> {
> printf("notified\n");
> }
>
> int main(void)
> {
> struct sigaction act;
> int fd;
>
> act.sa_sigaction = handler;
> sigemptyset(&act.sa_mask);
> act.sa_flags = SA_SIGINFO;
> sigaction(SIGRTMIN, &act, NULL);
>
> fd = open(".", O_RDONLY);
> fcntl(fd, F_SETSIG, SIGRTMIN);
> fcntl(fd, F_NOTIFY, DN_ACCESS|DN_MULTISHOT);
>
> while (1) {
> pause();
> if (fork() <= 0) exit(0);
> wait(NULL);
> }
> }
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2002-03-11 08:50:29

by Oskar Liljeblad

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

On Sunday, March 10, 2002 at 22:02, usel wrote:
> The code snipper demonstrates what I consider a bug in the
> dnotify facilities in the kernel. After a fork, all registered
> notifications are lost in the process where they originally
> where registered (the parent process). [..]

FWIW, as long as you keep the child alive after fork the
notifications are not lost. Also the same effect when you
keep the parent(s) alive and decide to receive notification
in the newly created process instead.

Oskar Liljeblad ([email protected])

2002-03-11 10:21:30

by Alex Riesen

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

On Mon, Mar 11, 2002 at 09:50:06AM +0100, Oskar Liljeblad wrote:
> On Sunday, March 10, 2002 at 22:02, usel wrote:
> > The code snipper demonstrates what I consider a bug in the
> > dnotify facilities in the kernel. After a fork, all registered
> > notifications are lost in the process where they originally
> > where registered (the parent process). [..]
>
> FWIW, as long as you keep the child alive after fork the
> notifications are not lost. Also the same effect when you
> keep the parent(s) alive and decide to receive notification
> in the newly created process instead.
What process are the notifications sent, in this case?
IMHO, only the parent can catch them, as fcntl called in the parent
only.

Anyway, strange effect.



>
> Oskar Liljeblad ([email protected])
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2002-03-11 10:26:04

by Andrea Arcangeli

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

On Sun, Mar 10, 2002 at 10:08:02PM +0100, Oskar Liljeblad wrote:
> The code snipper demonstrates what I consider a bug in the
> dnotify facilities in the kernel. After a fork, all registered
> notifications are lost in the process where they originally
> where registered (the parent process). "lost" here means that
> the signal specified with F_SETSIG fcntl no longer is delivered
> when notified.
>
> How to reproduce (tested with 2.4.17):
> gcc -o dnoticebug dnoticebug.c
> dnoticebug & # run in background
> cat dnoticebug.c >/dev/null # "notified" should now be printed
> cat dnoticebug.c >/dev/null # nothing is printed this time
>
> If you comment out the line with fork below, "notified" *will* be
> printed every time you cat dnoticebug.c.
>
> I'm not subscribed to the list so I'd appreciate if you CCed me.
> (Otherwise I'd have to use the archives :) Thanks.

this should fix your problem:

ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.19pre2aa2/00_dnotify-fl_owner-1

Andrea

2002-03-12 01:06:51

by Stephen Rothwell

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

Hi Andrea,

On Mon, 11 Mar 2002 11:26:52 +0100 Andrea Arcangeli <[email protected]> wrote:
>
> On Sun, Mar 10, 2002 at 10:08:02PM +0100, Oskar Liljeblad wrote:
> > The code snipper demonstrates what I consider a bug in the
> > dnotify facilities in the kernel. After a fork, all registered
> > notifications are lost in the process where they originally
> > where registered (the parent process). "lost" here means that
> > the signal specified with F_SETSIG fcntl no longer is delivered
> > when notified.
>
> this should fix your problem:
>
> ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.19pre2aa2/00_dnotify-fl_owner-1
>
> Andrea

Can you see any reason we should not use hte patch below instead?
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

diff -ruN 2.4.19-pre3/fs/dnotify.c 2.4.19-pre3-notify/fs/dnotify.c
--- 2.4.19-pre3/fs/dnotify.c Wed Nov 8 18:27:57 2000
+++ 2.4.19-pre3-notify/fs/dnotify.c Tue Mar 12 12:02:15 2002
@@ -1,7 +1,7 @@
/*
* Directory notifications for Linux.
*
- * Copyright (C) 2000 Stephen Rothwell
+ * Copyright (C) 2000,2001,2002 Stephen Rothwell
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -59,7 +59,7 @@
write_lock(&dn_lock);
prev = &inode->i_dnotify;
for (odn = *prev; odn != NULL; prev = &odn->dn_next, odn = *prev)
- if (odn->dn_filp == filp)
+ if ((odn->dn_owner == current->files) && (odn->dn_filp == filp))
break;
if (odn != NULL) {
if (turning_off) {
@@ -82,6 +82,7 @@
dn->dn_mask = arg;
dn->dn_fd = fd;
dn->dn_filp = filp;
+ dn->dn_owner = current->files;
inode->i_dnotify_mask |= arg & ~DN_MULTISHOT;
dn->dn_next = inode->i_dnotify;
inode->i_dnotify = dn;
diff -ruN 2.4.19-pre3/include/linux/dnotify.h 2.4.19-pre3-notify/include/linux/dnotify.h
--- 2.4.19-pre3/include/linux/dnotify.h Wed Mar 6 16:08:12 2002
+++ 2.4.19-pre3-notify/include/linux/dnotify.h Tue Mar 12 11:23:07 2002
@@ -11,6 +11,7 @@
see linux/fcntl.h */
int dn_fd;
struct file * dn_filp;
+ fl_owner_t dn_owner;
};

#define DNOTIFY_MAGIC 0x444E4F54

2002-03-12 01:19:53

by Andrea Arcangeli

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

On Tue, Mar 12, 2002 at 12:04:52PM +1100, Stephen Rothwell wrote:
> Hi Andrea,
>
> On Mon, 11 Mar 2002 11:26:52 +0100 Andrea Arcangeli <[email protected]> wrote:
> >
> > On Sun, Mar 10, 2002 at 10:08:02PM +0100, Oskar Liljeblad wrote:
> > > The code snipper demonstrates what I consider a bug in the
> > > dnotify facilities in the kernel. After a fork, all registered
> > > notifications are lost in the process where they originally
> > > where registered (the parent process). "lost" here means that
> > > the signal specified with F_SETSIG fcntl no longer is delivered
> > > when notified.
> >
> > this should fix your problem:
> >
> > ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.19pre2aa2/00_dnotify-fl_owner-1
> >
> > Andrea
>
> Can you see any reason we should not use hte patch below instead?

If somebody overrides the dnotify on the same file, he should become the
new owner, that's not handled in the below patch.

Secondly I prefer to return -EPERM to userspace if somebody tries to
drop a dnotify that it doesn't own, it gives more information back to
userspace.

On the same lines I would prefer that also a "turning_off" that fails to
find the file in the i_dnotify list , would return an error to be
strictier, but I didn't changed this bit of course.

Andrea

2002-03-12 01:48:20

by Malte Starostik

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

(Manually quoted from the archive)

> Just a "me too".
> I have tried also to use the default (SIGIO) by setting owner pid (just
> in case). It is all the same.
> Does someone use the notifications, btw?
> The whole thing seems somewhat untested.
> -alex
We used to use them in the KDE libraries. However, we decided to disable it
for now due to this problem since it's not theoretical, it hits quite often
in konqueror and especially kdesktop as both spawn child processes.
On a side note, dnotify is pretty weak compared to FAM with imon
(http://oss.sgi.com/projects/fam/) IMHO:
Imagine a file manager that has a view on a large directory like /usr/bin. Now
a file in that directory is added / removed / changed. With dnotify, the
filemanager will have to rescan the whole directory as a reaction to the
signal, to find out which file has changed. OTOH, with FAM, it gets a precise
event that tells "file `somebinary' has been added" or similar.
I don't have much of a clue about the kernel, so please excuse my ignorance,
but the imon patch seems pretty unintrusive to me and enables more
fine-grained file change notifications than dnotify. Also, FAM can monitor
NFS-mounted directories with almost no network overhead when it's also
running on the server.

> On Sun, Mar 10, 2002 at 10:08:02PM +0100, Oskar Liljeblad wrote:
> > The code snipper demonstrates what I consider a bug in the
> > dnotify facilities in the kernel. After a fork, all registered
> > notifications are lost in the process where they originally
> > where registered (the parent process). "lost" here means that
> > the signal specified with F_SETSIG fcntl no longer is delivered
> > when notified.

Regards,
-Malte

PS: I'm not subscribed, please CC me on answers; I'll also watch the archive.
--
Malte Starostik
PGP: 1024D/D2F3C787 [C138 2121 FAF3 410A 1C2A 27CD 5431 7745 D2F3 C787]

2002-03-12 03:01:42

by Stephen Rothwell

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

Hi Adrea,

On Tue, 12 Mar 2002 02:20:46 +0100 Andrea Arcangeli <[email protected]> wrote:
>
> If somebody overrides the dnotify on the same file, he should become the
> new owner, that's not handled in the below patch.

My contention is that if some process (not in the same thread group as the
process that originally set up the directory notifier) tries to set up a
directory notifier on the same file descriptor (i.e. they are a child of
the original process), then they should get their own notifier. The
parent can remove their notifier if they want to.

Notice that multiple processes can have notifiers enabled for the same
directory (even with a different set of flags).

My patch makes directory notifiers per thread group instead of per process
tree (which they are now).

> Secondly I prefer to return -EPERM to userspace if somebody tries to
> drop a dnotify that it doesn't own, it gives more information back to
> userspace.

This would be equivalent to returning -EPERM if you tried to remove a
lock on a file when you didn't set it ...

--
Cheers, Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

2002-03-12 04:00:04

by Andrea Arcangeli

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

On Tue, Mar 12, 2002 at 01:59:49PM +1100, Stephen Rothwell wrote:
> My patch makes directory notifiers per thread group instead of per process
> tree (which they are now).

I overlooked that in your patch you changed the index for the lookup, so
yes, that looks a better fix.

> This would be equivalent to returning -EPERM if you tried to remove a
> lock on a file when you didn't set it ...

Lock are standard and we cannot change such API, but for a fairly new
API I'd prefer strict stuff. Anyways now -EPERM would be wrong and the
need of the errorback is also lower because there's no risk of
collisions anymore, now different current->files will be able to get
different notifiers, so if something it should be a -ENOENT, it's not a
matter of permissions/ownership anymore.

Andrea

2002-03-12 07:04:20

by Alex Riesen

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

The patch worked for 2.4.19-pre2-ac2+preempt.


> this should fix your problem:
>
> ftp://ftp.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.19pre2aa2/00_dnotify-fl_owner-1
>
> Andrea

2002-03-12 12:20:41

by Jamie Lokier

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

Alex Riesen wrote:
> Just a "me too".
> I have tried also to use the default (SIGIO) by setting owner pid (just
> in case). It is all the same.
> Does someone use the notifications, btw?
> The whole thing seems somewhat untested.

I don't yet, but I am planning to use them for an ultra-high-performance
dynamic web server and make-like tool.

The idea is that a dynamic page's cache validity (or the validity of a
subrequest) may depend on a large number of stat() results.

Dnotify can be used, as I understand it, to avoid having to actually do
the stat() calls on each request.

In this way, cached dynamic pages can be served as quickly as static
pages, right down to using the minimum set of system calls.

At the same time, dynamic pages are served perfectly _as if_ they are
recalculated every time from their prerequisite files, including: script
code (Perl, PHP etc.), included files, templates, images, database
files. So with things like setting the width and height of each IMG tag
from scanning the image files, and complex templates such as navigation
bars that you currently remake by hand using "make" -- these things
immediately update as you modify the site's source files.

It makes a pretty nice document editing environment too -- save file
from Emacs, immediately view formatted file in web browser :-)

I can do this at the moment, but with a number of stat() calls on each
request. With dnotify I think I can eliminate those but I haven't got
that far yet.

If dnotify is too buggy as stands then for this project I'd very much
like it to be fixed. Loss of notifications after fork() sounds like a
rather serious bug :-/

cheers,
-- Jamie

2002-03-12 12:57:29

by Jamie Lokier

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

Malte Starostik wrote:
> Imagine a file manager that has a view on a large directory like
> /usr/bin. Now a file in that directory is added / removed /
> changed. With dnotify, the filemanager will have to rescan the whole
> directory as a reaction to the signal, to find out which file has
> changed. OTOH, with FAM, it gets a precise event that tells "file
> `somebinary' has been added" or similar.

It would indeed be very cool if dnotify could say which inode has been
updated. The problem I have with FAM is that notifications aren't
immediate, which makes it useless for my fast dynamic web server
application. For that, notifications have to be strong enough to
support the rule "if I have not received a dnotify _now_, then my cached
stat() result is valid for files in that directory _now_".

> I don't have much of a clue about the kernel, so please excuse my ignorance,
> but the imon patch seems pretty unintrusive to me and enables more
> fine-grained file change notifications than dnotify. Also, FAM can monitor
> NFS-mounted directories with almost no network overhead when it's also
> running on the server.

Ah, thanks, I didn't know about imon.

The imon API is nice -- I could use that and more efficiently than
dnotify. (I can't use FAM because of notification delays, but the imon
device is just right).

Unfortunately the implementation is rather intrusive. As it says, it
implies a performance hit on every file operation on all files, if any
file is being monitored. So I can see it being disabled on high
performance servers, which is where I need it.

A fusion of imon and dnotify would be good:

- absolute minimum impact on file operations: a small inline check,
per inode not globally, as dnotify does now.

- imon-style hash table used for inodes that aren't in core, so you
can monitor a large number of files individually without pulling
their inodes into core. When inodes are brought into core then the
hash table entries are moved to the in core inode, and vice versa.

(Not sure about the reliability of hashing on inode numbers,
though -- some filesystems don't provide reliable inode numbers).

- dnotify-style lists of monitoring requests attached to in core
inodes.

- per-process monitoring sets, as dnotify does.

- imon-style precision notification of individual files, including
reliably monitoring files which are hard linked in different
directories.

- dnotify causes files to notify their parent directory (yes it's
ambiguous with hard links). In a per-file scheme, one possible
flag on a file's inode would be "notify my parent directory". I
see no reason not to extend that to "I am a directory. If I
receive a notification from a child, notify _my_ parent directory",
for those occasions when you'd like to monitor a whole directory
hierarchy without having to do the equivalent slow disk accesses of
"find -print" on it first.

- guarantee that the event will be sent to the monitoring process
immediately after the operation which triggers the event, and
before the operation releases inode semaphore, if it has it -- so
that monitoring processes can depend on "if I haven't received a
notification then my stat() cache is still valid".

Some ideas. I would really like this to be usable with my idea for a
fast dynamic web server, which is also a "make" server, so I'm willing
to put some work in here.

Feedback appreciated,
-- Jamie

2002-03-12 16:43:58

by Daniel Phillips

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

On March 12, 2002 01:55 pm, Jamie Lokier wrote:
> - dnotify causes files to notify their parent directory (yes it's
> ambiguous with hard links).

That's a bitch, isn't it? The only way I can think of to deal with it is via
a hardlink reverse map, and there are lots of worms in that can, including
where you store it, how much it costs to maintain it, how persistent it
should be and how to make it perfectly non-racy.

--
Daniel

2002-03-12 17:23:09

by Jamie Lokier

[permalink] [raw]
Subject: Re: directory notifications lost after fork?

Daniel Phillips wrote:
> On March 12, 2002 01:55 pm, Jamie Lokier wrote:
> > - dnotify causes files to notify their parent directory (yes it's
> > ambiguous with hard links).
>
> That's a bitch, isn't it? The only way I can think of to deal with it
> is via a hardlink reverse map, and there are lots of worms in that
> can, including where you store it, how much it costs to maintain it,
> how persistent it should be and how to make it perfectly non-racy.

For dnotify purposes this may be solvable without a full reverse map.
Suppose that we have per-inode notifiers as I suggested, and as the imon
patch implements. Of course, multiple listeners can attach to an
inode's notifier chain -- this is needed to support multiple processes
listening.

Then you can implement dnotify by attaching the parent directory as a
listener to each of its child inodes. (It's a bit heavy to set up,
though).

Now, when an inode is modifed we don't guarantee to notify all the
parent directories... but we do guarantee to notify all the ones which
are actually listening at the moment. So it's a partial reverse map. I
expect Al Viro would have something to say about dcache races at this
point.

For recursive parent notification, such as monitoring "/usr" to learn
about changes anywhere underneath "/usr", the above is perhaps
impractical. We're right back to having to do "find -print" equivalent
disk activity. Or reverse maps in the filesystem. Ugh.

In practice I'd just give up trying to cache stat() results of hard
linked files, unless I knew I'd found all the paths to those files.
Just don't use hard links ;-)

cheers,
-- Jamie