[PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Maxim Uvarov]

LKML patch was here:
https://lkml.org/lkml/2011/12/14/457

CC: inotify and NFS developers

[PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Maxim Uvarov]

This issue was introduced with LTP inotify02 test.
If file system is not NFS user inotify application
gets IN_DELETE event. But on NFS code avoids d_delete()
which sends this event. This patch makes notification
on NFS the same as non-NFS. I.e. vfs_unlink still avoids
deletion but it sends event for NFS sillyrenamed files.
More details here:
https://lkml.org/lkml/2011/11/29/421

Signed-off-by: Maxim Uvarov <maxim.uvarov@oracle.com>
---
 fs/namei.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index a9a7166..44a997e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2737,10 +2737,23 @@ int vfs_unlink(struct inode *dir, struct dentry *dentry)
 	}
 	mutex_unlock(&dentry->d_inode->i_mutex);
 
-	/* We don't d_delete() NFS sillyrenamed files--they still exist. */
-	if (!error && !(dentry->d_flags & DCACHE_NFSFS_RENAMED)) {
+
+	if (!error) {
 		fsnotify_link_count(dentry->d_inode);
-		d_delete(dentry);
+		if (!(dentry->d_flags & DCACHE_NFSFS_RENAMED))
+			d_delete(dentry);
+		else {
+			/* We don't d_delete() NFS sillyrenamed files - they
+			 * still exist. But from user side this file was
+			 * deleted. So that we need to generate notify event
+			 * about it.
+			 */
+			struct inode *inode;
+			int isdir;
+			inode = dentry->d_inode;
+			isdir = S_ISDIR(inode->i_mode);
+			fsnotify_nameremove(dentry, isdir);
+		}
 	}
 
 	return error;
-- 
1.7.4.1

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Al Viro]

On Wed, Dec 14, 2011 at 02:58:15PM -0800, Maxim Uvarov wrote:
> This issue was introduced with LTP inotify02 test.
> If file system is not NFS user inotify application
> gets IN_DELETE event. But on NFS code avoids d_delete()
> which sends this event. This patch makes notification
> on NFS the same as non-NFS. I.e. vfs_unlink still avoids
> deletion but it sends event for NFS sillyrenamed files.
> More details here:
> https://lkml.org/lkml/2011/11/29/421

NAK.  Too damn ugly.  And completely misses the point of inotify -
seeing events generated only by this client is going to be worthless.
Face it, inotify is broken by design for network filesystems.  Any
software using it on NFS is broken; trying to paper over that is
not a good idea.

Just.  Don't.  Use.  That.  Shit.

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Maxim Uvarov]

On 12/14/2011 03:22 PM, Al Viro wrote:
> On Wed, Dec 14, 2011 at 02:58:15PM -0800, Maxim Uvarov wrote:
>> This issue was introduced with LTP inotify02 test.
>> If file system is not NFS user inotify application
>> gets IN_DELETE event. But on NFS code avoids d_delete()
>> which sends this event. This patch makes notification
>> on NFS the same as non-NFS. I.e. vfs_unlink still avoids
>> deletion but it sends event for NFS sillyrenamed files.
>> More details here:
>> https://lkml.org/lkml/2011/11/29/421
>
> NAK.  Too damn ugly.  And completely misses the point of inotify -
> seeing events generated only by this client is going to be worthless.
> Face it, inotify is broken by design for network filesystems.  Any
> software using it on NFS is broken; trying to paper over that is
> not a good idea.
>
> Just.  Don't.  Use.  That.  Shit.

Thank you for so technical answer. It's really difficult to object 
against such considered and well-grounded, forcible arguments.

Now to be serious:

1. Original VFS code already has "if NFS", in vfs_unlink(). Because of 
code does not call d_delete() it has to call notification from d_delete().

2. inotify is done on VFS layer. So logically it has to work on all file 
systems.

3. We think that software works if it's execution matches specification. 
Because we don't have any specification for inotify we can relay on 
tests. I.e. if all test for all notification events passed the same way 
for all file systems; events, number of events and sequence for events 
matches and do not depend on file system type - we can say that this 
feature works. As the base for inotify we can take LTP tests, which have 
good coverage for all events.


Best regards,
Maxim Uvarov.

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Al Viro]

On Thu, Dec 15, 2011 at 11:12:30AM -0800, Maxim Uvarov wrote:

> 1. Original VFS code already has "if NFS", in vfs_unlink(). Because
> of code does not call d_delete() it has to call notification from
> d_delete().
> 
> 2. inotify is done on VFS layer. So logically it has to work on all
> file systems.

You are using a very odd meaning of the word "logically", then.  Note
that inotify does *not* work on NFS, no matter what vfs_unlink() would do.
Simply because files are removed on server, not in VFS.  And server does
not notify clients of such removals.  Ergo, any software that relies on
inotify delivering notifications of files being removed is broken on NFS.
That has nothing whatsoever to the layer in kernel where it's handled;
the information asked for is simply not available to client.  Period.

Incidentally, inotify does not work on a bunch of local filesystems,
starting with procfs.  And won't work, unless you are seriously proposing
to generate events on things like open()/dup2()/etc.  In this case we
might very well have objects appearing and disappearing without ever
having had a dentry.

RE: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Myklebust, Trond]

> -----Original Message-----
> From: Al Viro [mailto:viro@ZenIV.linux.org.uk]
> Sent: Thursday, December 15, 2011 4:33 PM
> To: Maxim Uvarov
> Cc: linux-nfs@vger.kernel.org; bfields@fieldses.org; Myklebust, Trond;
> john@johnmccutchan.com; rlove@rlove.org; eparis@parisplace.org
> Subject: Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on
nfs
> 
> On Thu, Dec 15, 2011 at 11:12:30AM -0800, Maxim Uvarov wrote:
> 
> > 1. Original VFS code already has "if NFS", in vfs_unlink(). Because
of
> > code does not call d_delete() it has to call notification from
> > d_delete().
> >
> > 2. inotify is done on VFS layer. So logically it has to work on all
> > file systems.
> 
> You are using a very odd meaning of the word "logically", then.  Note
that
> inotify does *not* work on NFS, no matter what vfs_unlink() would do.
> Simply because files are removed on server, not in VFS.  And server
does not
> notify clients of such removals.  Ergo, any software that relies on
inotify
> delivering notifications of files being removed is broken on NFS.
> That has nothing whatsoever to the layer in kernel where it's handled;
the
> information asked for is simply not available to client.  Period.
> 
> Incidentally, inotify does not work on a bunch of local filesystems,
starting
> with procfs.  And won't work, unless you are seriously proposing to
generate
> events on things like open()/dup2()/etc.  In this case we might very
well have
> objects appearing and disappearing without ever having had a dentry.

The other thing to note is that even if there were value in having only
client support for the locally performed operations, the way NFS deletes
work is fundamentally different to the way that POSIX unlink works: if
the file is still open, it isn't deleted, it is just renamed. In
consequence, it not only still appears in readdir() requests (albeit
under a different name), but it acts in all ways shapes and forms as the
same regular file but with the curious property that when the last user
closes it gets deleted.

IOW: the assumption that we would need to generate an
fsnotify_nameremove event here is in any case flawed. It would rather be
fsnotify_move, which I suspect would still trip up these LTP tests...

Cheers
   Trond

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Maxim Uvarov]

On 12/15/2011 01:52 PM, Myklebust, Trond wrote:
>> -----Original Message-----
>> From: Al Viro [mailto:viro@ZenIV.linux.org.uk]
>> Sent: Thursday, December 15, 2011 4:33 PM
>> To: Maxim Uvarov
>> Cc: linux-nfs@vger.kernel.org; bfields@fieldses.org; Myklebust, Trond;
>> john@johnmccutchan.com; rlove@rlove.org; eparis@parisplace.org
>> Subject: Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on
> nfs
>>
>> On Thu, Dec 15, 2011 at 11:12:30AM -0800, Maxim Uvarov wrote:
>>
>>> 1. Original VFS code already has "if NFS", in vfs_unlink(). Because
> of
>>> code does not call d_delete() it has to call notification from
>>> d_delete().
>>>
>>> 2. inotify is done on VFS layer. So logically it has to work on all
>>> file systems.
>>
>> You are using a very odd meaning of the word "logically", then.  Note
> that
>> inotify does *not* work on NFS, no matter what vfs_unlink() would do.
>> Simply because files are removed on server, not in VFS.  And server
> does not
>> notify clients of such removals.  Ergo, any software that relies on
> inotify
>> delivering notifications of files being removed is broken on NFS.
>> That has nothing whatsoever to the layer in kernel where it's handled;
> the
>> information asked for is simply not available to client.  Period.
>>
>> Incidentally, inotify does not work on a bunch of local filesystems,
> starting
>> with procfs.  And won't work, unless you are seriously proposing to
> generate
>> events on things like open()/dup2()/etc.  In this case we might very
> well have
>> objects appearing and disappearing without ever having had a dentry.
>
> The other thing to note is that even if there were value in having only
> client support for the locally performed operations, the way NFS deletes
> work is fundamentally different to the way that POSIX unlink works: if
> the file is still open, it isn't deleted, it is just renamed. In
> consequence, it not only still appears in readdir() requests (albeit
> under a different name), but it acts in all ways shapes and forms as the
> same regular file but with the curious property that when the last user
> closes it gets deleted.
>
> IOW: the assumption that we would need to generate an
> fsnotify_nameremove event here is in any case flawed. It would rather be
> fsnotify_move, which I suspect would still trip up these LTP tests...
>
> Cheers
>     Trond
>

Thanks. My original idea was to generate absolutely the same event under 
unlink(). Yes, original file in that case is not deleted, it is moved. 
But from user side it's deletion. Users don't take care about
/tmp/.nfs..  files, right? When you call unlink the only thing you do
is to check return code, you are not walking along all folders to check
where it could be moved.

I also agree that if server or other mounts does something with file, 
server will not send any notification. But for single mount it has to be ok.

Maxim.

RE: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Myklebust, Trond]

> -----Original Message-----
> From: Maxim Uvarov [mailto:maxim.uvarov@oracle.com]
> Sent: Thursday, December 15, 2011 5:34 PM
> To: Myklebust, Trond
> Cc: Al Viro; linux-nfs@vger.kernel.org; bfields@fieldses.org;
> john@johnmccutchan.com; rlove@rlove.org; eparis@parisplace.org
> Subject: Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on
nfs
> 
> On 12/15/2011 01:52 PM, Myklebust, Trond wrote:
> >> -----Original Message-----
> >> From: Al Viro [mailto:viro@ZenIV.linux.org.uk]
> >> Sent: Thursday, December 15, 2011 4:33 PM
> >> To: Maxim Uvarov
> >> Cc: linux-nfs@vger.kernel.org; bfields@fieldses.org; Myklebust,
> >> Trond; john@johnmccutchan.com; rlove@rlove.org;
> eparis@parisplace.org
> >> Subject: Re: [PATCH] NFS/INOTIFY: inotify user when deleting files
on
> > nfs
> >>
> >> On Thu, Dec 15, 2011 at 11:12:30AM -0800, Maxim Uvarov wrote:
> >>
> >>> 1. Original VFS code already has "if NFS", in vfs_unlink().
Because
> > of
> >>> code does not call d_delete() it has to call notification from
> >>> d_delete().
> >>>
> >>> 2. inotify is done on VFS layer. So logically it has to work on
all
> >>> file systems.
> >>
> >> You are using a very odd meaning of the word "logically", then.
Note
> > that
> >> inotify does *not* work on NFS, no matter what vfs_unlink() would
do.
> >> Simply because files are removed on server, not in VFS.  And server
> > does not
> >> notify clients of such removals.  Ergo, any software that relies on
> > inotify
> >> delivering notifications of files being removed is broken on NFS.
> >> That has nothing whatsoever to the layer in kernel where it's
> >> handled;
> > the
> >> information asked for is simply not available to client.  Period.
> >>
> >> Incidentally, inotify does not work on a bunch of local
filesystems,
> > starting
> >> with procfs.  And won't work, unless you are seriously proposing to
> > generate
> >> events on things like open()/dup2()/etc.  In this case we might
very
> > well have
> >> objects appearing and disappearing without ever having had a
dentry.
> >
> > The other thing to note is that even if there were value in having
> > only client support for the locally performed operations, the way
NFS
> > deletes work is fundamentally different to the way that POSIX unlink
> > works: if the file is still open, it isn't deleted, it is just
> > renamed. In consequence, it not only still appears in readdir()
> > requests (albeit under a different name), but it acts in all ways
> > shapes and forms as the same regular file but with the curious
> > property that when the last user closes it gets deleted.
> >
> > IOW: the assumption that we would need to generate an
> > fsnotify_nameremove event here is in any case flawed. It would
rather
> > be fsnotify_move, which I suspect would still trip up these LTP
tests...
> >
> > Cheers
> >     Trond
> >
> 
> Thanks. My original idea was to generate absolutely the same event
under
> unlink(). Yes, original file in that case is not deleted, it is moved.
> But from user side it's deletion. Users don't take care about
/tmp/.nfs..  files,
> right? When you call unlink the only thing you do is to check return
code, you
> are not walking along all folders to check where it could be moved.

It is not deletion because it is not an atomic operation.

Until the last close occurs. ls -a will still list the .nfsxxxx entry.
Most operations that the user may expect to fail on a deleted file such
as link(), open(), access(), stat(), etc. will still succeed provided
that you use the .nfsxxxx name. On the other hand, an operation such as
rmdir() on the parent directory will not succeed because it still
contains at least one file entry...

> I also agree that if server or other mounts does something with file,
server
> will not send any notification. But for single mount it has to be ok.

What's the killer application?

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Maxim Uvarov]

On 12/15/2011 02:58 PM, Myklebust, Trond wrote:
>> -----Original Message-----
>> From: Maxim Uvarov [mailto:maxim.uvarov@oracle.com]
>> Sent: Thursday, December 15, 2011 5:34 PM
>> To: Myklebust, Trond
>> Cc: Al Viro; linux-nfs@vger.kernel.org; bfields@fieldses.org;
>> john@johnmccutchan.com; rlove@rlove.org; eparis@parisplace.org
>> Subject: Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on
> nfs
>>
>> On 12/15/2011 01:52 PM, Myklebust, Trond wrote:
>>>> -----Original Message-----
>>>> From: Al Viro [mailto:viro@ZenIV.linux.org.uk]
>>>> Sent: Thursday, December 15, 2011 4:33 PM
>>>> To: Maxim Uvarov
>>>> Cc: linux-nfs@vger.kernel.org; bfields@fieldses.org; Myklebust,
>>>> Trond; john@johnmccutchan.com; rlove@rlove.org;
>> eparis@parisplace.org
>>>> Subject: Re: [PATCH] NFS/INOTIFY: inotify user when deleting files
> on
>>> nfs
>>>>
>>>> On Thu, Dec 15, 2011 at 11:12:30AM -0800, Maxim Uvarov wrote:
>>>>
>>>>> 1. Original VFS code already has "if NFS", in vfs_unlink().
> Because
>>> of
>>>>> code does not call d_delete() it has to call notification from
>>>>> d_delete().
>>>>>
>>>>> 2. inotify is done on VFS layer. So logically it has to work on
> all
>>>>> file systems.
>>>>
>>>> You are using a very odd meaning of the word "logically", then.
> Note
>>> that
>>>> inotify does *not* work on NFS, no matter what vfs_unlink() would
> do.
>>>> Simply because files are removed on server, not in VFS.  And server
>>> does not
>>>> notify clients of such removals.  Ergo, any software that relies on
>>> inotify
>>>> delivering notifications of files being removed is broken on NFS.
>>>> That has nothing whatsoever to the layer in kernel where it's
>>>> handled;
>>> the
>>>> information asked for is simply not available to client.  Period.
>>>>
>>>> Incidentally, inotify does not work on a bunch of local
> filesystems,
>>> starting
>>>> with procfs.  And won't work, unless you are seriously proposing to
>>> generate
>>>> events on things like open()/dup2()/etc.  In this case we might
> very
>>> well have
>>>> objects appearing and disappearing without ever having had a
> dentry.
>>>
>>> The other thing to note is that even if there were value in having
>>> only client support for the locally performed operations, the way
> NFS
>>> deletes work is fundamentally different to the way that POSIX unlink
>>> works: if the file is still open, it isn't deleted, it is just
>>> renamed. In consequence, it not only still appears in readdir()
>>> requests (albeit under a different name), but it acts in all ways
>>> shapes and forms as the same regular file but with the curious
>>> property that when the last user closes it gets deleted.
>>>
>>> IOW: the assumption that we would need to generate an
>>> fsnotify_nameremove event here is in any case flawed. It would
> rather
>>> be fsnotify_move, which I suspect would still trip up these LTP
> tests...
>>>
>>> Cheers
>>>      Trond
>>>
>>
>> Thanks. My original idea was to generate absolutely the same event
> under
>> unlink(). Yes, original file in that case is not deleted, it is moved.
>> But from user side it's deletion. Users don't take care about
> /tmp/.nfs..  files,
>> right? When you call unlink the only thing you do is to check return
> code, you
>> are not walking along all folders to check where it could be moved.
>
> It is not deletion because it is not an atomic operation.
>
> Until the last close occurs. ls -a will still list the .nfsxxxx entry.
> Most operations that the user may expect to fail on a deleted file such
> as link(), open(), access(), stat(), etc. will still succeed provided
> that you use the .nfsxxxx name. On the other hand, an operation such as
> rmdir() on the parent directory will not succeed because it still
> contains at least one file entry...
>
>> I also agree that if server or other mounts does something with file,
> server
>> will not send any notification. But for single mount it has to be ok.
>
> What's the killer application?
>
No application, I only have information that one of LTP tests is failed.

Maxim.

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Stef Bon]

Hi,

not exactly the same topic, I'm looking how to make inotify possible
for filesystems like FUSE, cifs and nfs. As mentioned here earlier,
inotify is an internal kernel subsystem only, and it's not wise to
make userspace filesystems handle inotify the same way as the kernel
does.

But still (I think) that make the "external" filesystems handle notify
is a good thing. I think the way to go is:

a. create a new inotify netlink module (name VFS, subgroup notify),
which sends a message to userspace when a inotify watch is set (with
path (or inode) and mask)

b. filesystem (FUSE based, or nfs or cifs) listens to this

c. when inotify add watch message is received, the fs takes the right
action. In the case of cifs, this is probably something like:

http://msdn.microsoft.com/en-us/library/cc246553(v=PROT.13).aspx

with the mask translated to something that makes sense to the remote server.

in case of a simple overlay FUSE filesystem (backend is on the same
host, is an existing fs), the fs sets a inotify watch on this fs.

In the case of NFS I do not know, I do not have enough knowledge about it.

d. when the fs receives something on the backend specific "notify", it
translates it to a linux VFS operation, and notifies the kernel. FUSE
has recently added some calls which for example notify the kernel when
a directory is removed.
http://msdn.microsoft.com/en-us/library/cc246553(v=PROT.13).aspx

e. when on the netlink socket the notify remove action is received,
the fs removes the related things it has setup in step c.

Any comments please.

Stef

Re: [PATCH] NFS/INOTIFY: inotify user when deleting files on nfs

[Boaz Harrosh]

Forwarding to the NFS mailing list ...

On 12/15/2011 12:27 AM, Maxim Uvarov wrote:
> This issue was introduced with LTP inotify02 test.
> If file system is not NFS user inotify application
> gets IN_DELETE event. But on NFS code avoids d_delete()
> which sends this event. This patch makes notification
> on NFS the same as non-NFS. I.e. vfs_unlink still avoids
> deletion but it sends event for NFS sillyrenamed files.
> More details here:
> https://lkml.org/lkml/2011/11/29/421
> 
> Signed-off-by: Maxim Uvarov <maxim.uvarov@oracle.com>
> ---
>  fs/namei.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index a9a7166..44a997e 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2737,10 +2737,23 @@ int vfs_unlink(struct inode *dir, struct dentry *dentry)
>  	}
>  	mutex_unlock(&dentry->d_inode->i_mutex);
>  
> -	/* We don't d_delete() NFS sillyrenamed files--they still exist. */
> -	if (!error && !(dentry->d_flags & DCACHE_NFSFS_RENAMED)) {
> +
> +	if (!error) {
>  		fsnotify_link_count(dentry->d_inode);
> -		d_delete(dentry);
> +		if (!(dentry->d_flags & DCACHE_NFSFS_RENAMED))
> +			d_delete(dentry);
> +		else {
> +			/* We don't d_delete() NFS sillyrenamed files - they
> +			 * still exist. But from user side this file was
> +			 * deleted. So that we need to generate notify event
> +			 * about it.
> +			 */
> +			struct inode *inode;
> +			int isdir;
> +			inode = dentry->d_inode;
> +			isdir = S_ISDIR(inode->i_mode);
> +			fsnotify_nameremove(dentry, isdir);
> +		}
>  	}
>  
>  	return error;