missing code in fcntl_dirnotify

missing code in fcntl_dirnotify

[Steven French]

I noticed that fcntl_dirnotify in fs/dnotify.c is missing logic to call
an optional fs specific inode operation for file/directory notify.  
Since directory and file notification to make any sense for many
filesystems (especially network ones) would require that this call
(fcntl F_NOTIFY) to pass through to the filesystem who owns the inode
(to let the particular filesystem know it has to call dnotify_parent on
remote updates to the file/directory) it seems like a bug. Without
notification to the underlying network filesystem, it can not know to
read the updated inode->i_dnotify_mask  Is there any objection why
dnotify_parent should not be added to ksysm.c and to adding an optional
inode_operation for change_notify so filesystems that can support change
notify (cifs, nfs, presumably afs etc.) could do change notify to the
proper kernel routine(dnotify_parent)?

Re: missing code in fcntl_dirnotify

[Stephen Rothwell]

Hi Steven,

On Fri, 15 Aug 2003 21:35:48 -0500 Steven French <smfrench@austin.rr.com> wrote:
>
> I noticed that fcntl_dirnotify in fs/dnotify.c is missing logic to call
> an optional fs specific inode operation for file/directory notify.  
> Since directory and file notification to make any sense for many
> filesystems (especially network ones) would require that this call
> (fcntl F_NOTIFY) to pass through to the filesystem who owns the inode
> (to let the particular filesystem know it has to call dnotify_parent on
> remote updates to the file/directory) it seems like a bug. Without

Less a bug - more a design feature :-)

> notification to the underlying network filesystem, it can not know to
> read the updated inode->i_dnotify_mask  Is there any objection why
> dnotify_parent should not be added to ksysm.c and to adding an optional
> inode_operation for change_notify so filesystems that can support change
> notify (cifs, nfs, presumably afs etc.) could do change notify to the
> proper kernel routine(dnotify_parent)?

No objection whatsoever.  At the time we implemented dnotify, we did not
need it to work on networked file systems (at least for remote events)
and we were not confident enough of ourselves or our understanding of
the file system code (locking in particular) to attempt this.  So we
documented it ... :-)  (see Documentation/dnotify.txt)

You probably need to add __inode_dir_notify as well (I think).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

P.S. Are you the Steven French from IBM in Austin?  Because I am the
Stephen Rothwell from IBM LTC Ozlabs in Canberra (in case it wasn't
obvious).

Re: missing code in fcntl_dirnotify

[Jamie Lokier]

Stephen Rothwell wrote:
> > notification to the underlying network filesystem, it can not know to
> > read the updated inode->i_dnotify_mask  Is there any objection why
> > dnotify_parent should not be added to ksysm.c and to adding an optional
> > inode_operation for change_notify so filesystems that can support change
> > notify (cifs, nfs, presumably afs etc.) could do change notify to the
> > proper kernel routine(dnotify_parent)?
> 
> No objection whatsoever.  At the time we implemented dnotify, we did not
> need it to work on networked file systems (at least for remote events)
> and we were not confident enough of ourselves or our understanding of
> the file system code (locking in particular) to attempt this.  So we
> documented it ... :-)  (see Documentation/dnotify.txt)
> 
> You probably need to add __inode_dir_notify as well (I think).

Note that local dnotify has well defined ordering, such that if you
know the underlying filesystem is local, stat() results can be safely
cached until a dnotify event is received, and dnotify events that are
sent are available to the target task immediately, without delay.

This is quite useful, when recreating a cached web page or program
file would require a stat() of all its component source files and
databases, as you can avoid all of them and just see whether you
received a dnotify before the page request.  This makes it feasible to
do accurate cacheing on every request.

At the moment, it's necessary to only use this feature if it's known
to be a local filesystem time, or if it's known that only the local
host modifies the files.

When dnotify is added for network filesystems, it seems likely that
the well defined ordering would be relaxed - that you'd expect to see
the dnotify _eventually_, rather than guaranteed to see it as soon as
a file is changed.  (NFS does not return up to date stat() results
either, so dnotify is not the only thing which is relaxed).

It would be great if there were some way for the application to be
told which kind of dnotify ordering it is getting (immediate or
delayed), and of course whether it will get them at all for files
modified remotely.

Cheers,
- Jamie

missing code in fcntl_dirnotify

[Steve French]

  Your point about safe dentry caching is a good one and I have meant to 
experiment with the idea of requesting to the server (via cifs change 
notify) notification for this purpose (since I am caching dentry info 
for about a second which is unsafe but similar to what nfs does - 
although I am safely caching inode data across the network via the cifs 
oplock mechanism) but was concerned about performance.   The particular 
use that a few people were asking me about recently was using the cifs 
vfs for distributed backup change notification (presumably because a 
wide variety of servers already handle the change notification subset of 
the CIFS protocol) and thus my small patch suggestion (which I will code 
up and experiment with next week).

Jamie Lokier wrote:

>Stephen Rothwell wrote:
>
>>>notification to the underlying network filesystem, it can not know to
>>>read the updated inode->i_dnotify_mask  Is there any objection why
>>>dnotify_parent should not be added to ksysm.c and to adding an optional
>>>inode_operation for change_notify so filesystems that can support change
>>>notify (cifs, nfs, presumably afs etc.) could do change notify to the
>>>proper kernel routine(dnotify_parent)?
>>>
>>No objection whatsoever.  At the time we implemented dnotify, we did not
>>need it to work on networked file systems (at least for remote events)
>>and we were not confident enough of ourselves or our understanding of
>>the file system code (locking in particular) to attempt this.  So we
>>documented it ... :-)  (see Documentation/dnotify.txt)
>>
>>You probably need to add __inode_dir_notify as well (I think).
>>
>
>Note that local dnotify has well defined ordering, such that if you
>know the underlying filesystem is local, stat() results can be safely
>cached until a dnotify event is received, and dnotify events that are
>sent are available to the target task immediately, without delay.
>
>This is quite useful, when recreating a cached web page or program
>file would require a stat() of all its component source files and
>databases, as you can avoid all of them and just see whether you
>received a dnotify before the page request.  This makes it feasible to
>do accurate cacheing on every request.
>
>At the moment, it's necessary to only use this feature if it's known
>to be a local filesystem time, or if it's known that only the local
>host modifies the files.
>
>When dnotify is added for network filesystems, it seems likely that
>the well defined ordering would be relaxed - that you'd expect to see
>the dnotify _eventually
>_, rather than guaranteed to see it as soon as
>a file is changed.  (NFS does not return up to date stat() results
>either, so dnotify is not the only thing which is relaxed).
>
>It would be great if there were some way for the application to be
>told which kind of dnotify ordering it is getting (immediate or
>delayed), and of course whether it will get them at all for files
>modified remotely.
>
>Cheers,
>- Jamie
>

Re: missing code in fcntl_dirnotify

[Jamie Lokier]

Steve French wrote:
>  Your point about safe dentry caching is a good one and I have meant to 
> experiment with the idea of requesting to the server (via cifs change 
> notify) notification for this purpose (since I am caching dentry info 
> for about a second which is unsafe but similar to what nfs does - 
> although I am safely caching inode data across the network via the cifs 
> oplock mechanism) but was concerned about performance.   The particular 
> use that a few people were asking me about recently was using the cifs 
> vfs for distributed backup change notification (presumably because a 
> wide variety of servers already handle the change notification subset of 
> the CIFS protocol) and thus my small patch suggestion (which I will code 
> up and experiment with next week).

(This whole message assumes the server doesn't delay notifications
internally - only the network and network protocol does).

Using cifs change notify does not completely fix the problem of
invalid cache results, because there is a latency in the notification
due to the network.  The will be times when the result you assume is
different to the result you'd get if you issued a request to the
server, because the notification is in flight.

However, if when you want to validate the cached dentry you wait until
at least one request has been sent over the cifs connection (_any_
request, from any process, will do), and until a response has been
received for that or any subsequent request, then you have safely
validate the dcache entry.

The trick, when a protocol is run over TCP (such as CIFS, or NFS over
TCP), is to look at your global queue of requests when you want to
revalidate something.  If it's non-empty, just take the serial number
of the first one which hasn't been transmitted yet, and wait until you
receive a response for that serial number or any later one.  If the
queue is empty, then generate a request, whether it's the equivalent
of STAT, NOP or the actual thing that you are checking anyway, and use
that as the serial number threshold to wait on.  If none of the
responses prior to the serial you are waiting for is a change
notification, you can assume your cached value was valid at the time
you started this whole procedure, and therefore return the value.

-- Jamie