Inotify fails to send IN_ATTRIB events

Inotify fails to send IN_ATTRIB events

[Morten Welinder]

I am seeing missing inotify IN_ATTRIB events in the following situation:

1. "touch foo"

2. Make inotify watch "foo"

3. "ln foo bar"
   --> Link count changed so I should have gotten an IN_ATTRIB.

4. "rm foo"
   --> Link count changed so I should have gotten an IN_ATTRIB.  (Or
IN_DELETE_SELF;
   I don't care which.)

5. "ln bar foo && rm bar"
   --> Still no events.

6. "mv foo bar"
   --> I get IN_MOVED_SELF.  Good!

7. "mv bar foo"
   --> I get IN_MOVED_SELF.  Good!


3+4 is pretty much the same as 6, so I really ought to be told that my
file has changed
name.  I don't really care much about getting notified about 3, but
for completeness
it ought to be handled.

As far as I can see, the only way to be told about 4 is to put a watch
on the directory in
which foo resides.  That is inelegant and has an inherent race condition.

This is with "Linux version 2.6.22.12-0.1-default" (SuSE 10.3)

Looking at current source, fs/namei.c, I notice that vfs_rename has a
fsnotify_move call
(which notified directory as well as files) whereas sys_link only has
a fsnotify_create call
(which notified the directory only).

Morten

Re: Inotify fails to send IN_ATTRIB events

[Jan Kara]

  Hi,

> I am seeing missing inotify IN_ATTRIB events in the following situation:
> 
> 1. "touch foo"
> 
> 2. Make inotify watch "foo"
> 
> 3. "ln foo bar"
>    --> Link count changed so I should have gotten an IN_ATTRIB.
> 
> 4. "rm foo"
>    --> Link count changed so I should have gotten an IN_ATTRIB.  (Or
> IN_DELETE_SELF;
>    I don't care which.)
> 
> 5. "ln bar foo && rm bar"
>    --> Still no events.
> 
> 6. "mv foo bar"
>    --> I get IN_MOVED_SELF.  Good!
> 
> 7. "mv bar foo"
>    --> I get IN_MOVED_SELF.  Good!
> 
> 
> 3+4 is pretty much the same as 6, so I really ought to be told that my
> file has changed
> name.  I don't really care much about getting notified about 3, but
> for completeness
> it ought to be handled.
> 
> As far as I can see, the only way to be told about 4 is to put a watch
> on the directory in
> which foo resides.  That is inelegant and has an inherent race condition.
  It looks sensible what you ask for ;) Wanna try the patch below?
Anybody has some objection to it?

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

---

Send inotify events to the inode itself when its link count has changed.

Signed-off-by: Jan Kara <jack@suse.cz>

diff --git a/fs/namei.c b/fs/namei.c
index 3b993db..9a91130 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2188,6 +2188,7 @@ int vfs_unlink(struct inode *dir, struct
 
 	/* We don't d_delete() NFS sillyrenamed files--they still exist. */
 	if (!error && !(dentry->d_flags & DCACHE_NFSFS_RENAMED)) {
+		fsnotify_link_count(dentry->d_inode);
 		d_delete(dentry);
 	}
 
@@ -2360,7 +2361,7 @@ int vfs_link(struct dentry *old_dentry,
 	error = dir->i_op->link(old_dentry, dir, new_dentry);
 	mutex_unlock(&old_dentry->d_inode->i_mutex);
 	if (!error)
-		fsnotify_create(dir, new_dentry);
+		fsnotify_link(dir, new_dentry);
 	return error;
 }
 
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 2bd31fa..b21b818 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -92,6 +92,14 @@ static inline void fsnotify_inoderemove(
 }
 
 /*
+ * fsnotify_link_count - inode's link count changed
+ */
+static inline void fsnotify_link_count(struct inode *inode)
+{
+	inotify_inode_queue_event(inode, IN_ATTRIB, 0, NULL, NULL);
+}
+
+/*
  * fsnotify_create - 'name' was linked in
  */
 static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
@@ -103,6 +111,18 @@ static inline void fsnotify_create(struc
 }
 
 /*
+ * fsnotify_link - new hardlink in 'inode' directory
+ */
+static inline void fsnotify_link(struct inode *inode, struct dentry *new_dentry)
+{
+	inode_dir_notify(inode, DN_CREATE);
+	inotify_inode_queue_event(inode, IN_CREATE, 0, new_dentry->d_name.name,
+				  new_dentry->d_inode);
+	fsnotify_link_count(new_dentry->d_inode);
+	audit_inode_child(new_dentry->d_name.name, new_dentry, inode);
+}
+
+/*
  * fsnotify_mkdir - directory 'name' was created
  */
 static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry)

Re: Inotify fails to send IN_ATTRIB events

[Morten Welinder]

> Wanna try the patch below?

With this patch I am seeing a endless stream of  IN_IGNORED events
for a removed watch.  I don't see a reason that user space should
ever see any IN_IGNORED, but an endless steam is not good.

Utterly unrelated, inotify does not work in /proc/.  The list archives
suggest that it isn't likely to start working anytime soon, but shouldn't
inotify_add_watch when fail with ENOSYS instead of pretending
it worked?

Morten




Failed to create inotify watch for /home/welinder/hi: No such file or directory
Created inotify watch 1 with mask 0x000007c0 for /home/welinder
# "touch hi" here
Got event 00000100 for 1
Created inotify watch 2 with mask 0x00000fc6 for /home/welinder/hi
Removing notify watch 1
Got event 00008000 for 1
Got event 00000004 for 2
# "rm hi" here
Got event 00000400 for 2
Removing notify watch 2
Got event 00008000 for 2
Failed to create inotify watch for /home/welinder/hi: No such file or directory
Created inotify watch 3 with mask 0x000007c0 for /home/welinder
Got event 00000200 for 3
Failed to create inotify watch for /home/welinder/hi: No such file or directory
Created inotify watch 3 with mask 0x000007c0 for /home/welinder
# "touch hi" here
Got event 00000100 for 3
Created inotify watch 4 with mask 0x00000fc6 for /home/welinder/hi
Removing notify watch 3
Got event 00008000 for 3
Got event 00000004 for 4
Got event 00000004 for 4
# "rm hi" here
Got event 00000400 for 4
Removing notify watch 4
Failed to create inotify watch for /home/welinder/hi: No such file or directory
Created inotify watch 5 with mask 0x000007c0 for /home/welinder
Got event 00008000 for 4
Got event 00008000 for 4
Got event 00008000 for 4
Got event 00008000 for 4
...

Re: Inotify fails to send IN_ATTRIB events

[Morten Welinder]

> With this patch I am seeing a endless stream of  IN_IGNORED events
> for a removed watch.

Sorry, that problem was on the chair. side of the keyboard -- too much
turkey, I fear. It just happened to be easier to trigger with the patch's
double event on deletion.

I'd still love to see ENOSYS when inotify can't handle a given virtual
file.

I'm not sure "signed-off-by" if appropriate for me, but at least...

Acked-by: Morten Welinder <mwelinder@gmail.com>

Morten

Re: Inotify fails to send IN_ATTRIB events

[Morten Welinder]

This looks bad, though:

include/linux/fsnotify.h:121: warning: passing argument 2 of
'audit_inode_child' from incompatible pointer type

Missing "->d_inode"?

M.

Re: Inotify fails to send IN_ATTRIB events

[Jan Kara]

> This looks bad, though:
> 
> include/linux/fsnotify.h:121: warning: passing argument 2 of
> 'audit_inode_child' from incompatible pointer type
> 
> Missing "->d_inode"?
  That's the difference between 2.6.22 and 2.6.24-git against which I
wrote the patch :).

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

[PATCH] Send IN_ATTRIB events when link count changes

[Jan Kara]

  Hi Andrew,

  would you pick up this patch for testing in -mm? Thanks.

								Honza

-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

---

Send inotify events to the inode itself when its link count has changed.

Signed-off-by: Jan Kara <jack@suse.cz>
Acked-by: Morten Welinder <mwelinder@gmail.com>

diff --git a/fs/namei.c b/fs/namei.c
index 3b993db..9a91130 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2188,6 +2188,7 @@ int vfs_unlink(struct inode *dir, struct
 
 	/* We don't d_delete() NFS sillyrenamed files--they still exist. */
 	if (!error && !(dentry->d_flags & DCACHE_NFSFS_RENAMED)) {
+		fsnotify_link_count(dentry->d_inode);
 		d_delete(dentry);
 	}
 
@@ -2360,7 +2361,7 @@ int vfs_link(struct dentry *old_dentry,
 	error = dir->i_op->link(old_dentry, dir, new_dentry);
 	mutex_unlock(&old_dentry->d_inode->i_mutex);
 	if (!error)
-		fsnotify_create(dir, new_dentry);
+		fsnotify_link(dir, new_dentry);
 	return error;
 }
 
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 2bd31fa..b21b818 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -92,6 +92,14 @@ static inline void fsnotify_inoderemove(
 }
 
 /*
+ * fsnotify_link_count - inode's link count changed
+ */
+static inline void fsnotify_link_count(struct inode *inode)
+{
+	inotify_inode_queue_event(inode, IN_ATTRIB, 0, NULL, NULL);
+}
+
+/*
  * fsnotify_create - 'name' was linked in
  */
 static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
@@ -103,6 +111,18 @@ static inline void fsnotify_create(struc
 }
 
 /*
+ * fsnotify_link - new hardlink in 'inode' directory
+ */
+static inline void fsnotify_link(struct inode *inode, struct dentry *new_dentry)
+{
+	inode_dir_notify(inode, DN_CREATE);
+	inotify_inode_queue_event(inode, IN_CREATE, 0, new_dentry->d_name.name,
+				  new_dentry->d_inode);
+	fsnotify_link_count(new_dentry->d_inode);
+	audit_inode_child(new_dentry->d_name.name, new_dentry, inode);
+}
+
+/*
  * fsnotify_mkdir - directory 'name' was created
  */
 static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry)

Re: [PATCH] Send IN_ATTRIB events when link count changes

[Andrew Morton]

On Mon, 26 Nov 2007 17:27:58 +0100
Jan Kara <jack@suse.cz> wrote:

>   Hi Andrew,
> 
>   would you pick up this patch for testing in -mm? Thanks.
> 
> 								Honza
> 
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
> 
> ---
> 
> Send inotify events to the inode itself when its link count has changed.

Could we have a better changelog please?  What is the reason for making
this change?  What are the before-and-after effects upon inotify clients,
etc?

Re: [PATCH] Send IN_ATTRIB events when link count changes

[Jan Kara]

On Tue 27-11-07 12:46:40, Andrew Morton wrote:
> On Mon, 26 Nov 2007 17:27:58 +0100
> Jan Kara <jack@suse.cz> wrote:
> 
> >   Hi Andrew,
> > 
> >   would you pick up this patch for testing in -mm? Thanks.
> > 
> > 								Honza
> > 
> > -- 
> > Jan Kara <jack@suse.cz>
> > SUSE Labs, CR
> > 
> > ---
> > 
> > Send inotify events to the inode itself when its link count has changed.
> 
> Could we have a better changelog please?  What is the reason for making
> this change?  What are the before-and-after effects upon inotify clients,
> etc?
  OK, hopefully a better change log:

  Currently, no notification event has been sent when inode's link count
changed. This is inconvenient for the application in some cases:
  Suppose you have the following directory structure
    foo/test
    bar/

  and you watch test. If someone does "mv foo/test bar/", you get event
IN_MOVE_SELF and you know something has happened with the file "test".
However if someone does "ln foo/test bar/test" and "rm foo/test" you get no
inotify event for the file "test" (only directories "foo" and "bar" receive
events).
  Furthermore it could be argued that link count belongs to file's metadata
and thus IN_ATTRIB should be sent when it changes.
  The following patch implements sending of IN_ATTRIB inotify events when
link count of the inode changes, i.e., when a hardlink to the inode is
created or when it is removed. This event is sent in addition to all the
events sent so far. In particular, when a last link to a file is removed,
IN_ATTRIB event is sent in addition to IN_DELETE_SELF event.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR