Groups > linux.kernel > #1599909 > unrolled thread

[RFC 1/2] fanotify: new event FAN_MODIFY_DIR

Started by	Filip Štědronský <r.lklm@regnarg.cz>
First post	2017-03-14 00:10 +0100
Last post	2017-03-15 06:00 +0100
Articles	11 on this page of 31 — 8 participants

Back to article view | Back to linux.kernel

  [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský <r.lklm@regnarg.cz> - 2017-03-14 00:10 +0100
    Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský <r.lklm@regnarg.cz> - 2017-03-14 00:20 +0100
      Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-14 11:50 +0100
        Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský <r.lkml@regnarg.cz> - 2017-03-14 15:00 +0100
          Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-14 16:10 +0100
          Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-20 13:20 +0100
    [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Filip Štědronský <r.lklm@regnarg.cz> - 2017-03-14 00:20 +0100
      Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Amir Goldstein <amir73il@gmail.com> - 2017-03-14 12:20 +0100
        Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Filip Štědronský <r.lkml@regnarg.cz> - 2017-03-14 16:00 +0100
          Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Amir Goldstein <amir73il@gmail.com> - 2017-03-14 16:40 +0100
          Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Marko Rauhamaa <marko.rauhamaa@f-secure.com> - 2017-03-15 09:50 +0100
            Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Jan Kara <jack@suse.cz> - 2017-03-15 14:50 +0100
              Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Marko Rauhamaa <marko.rauhamaa@f-secure.com> - 2017-03-15 15:20 +0100
              Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Amir Goldstein <amir73il@gmail.com> - 2017-03-15 15:50 +0100
        Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Jan Kara <jack@suse.cz> - 2017-03-19 11:20 +0100
          Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Filip Štědronský <r.lkml@regnarg.cz> - 2017-03-19 11:50 +0100
            Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Jan Kara <jack@suse.cz> - 2017-03-20 12:20 +0100
              Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Amir Goldstein <amir73il@gmail.com> - 2017-03-20 12:50 +0100
              Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Filip Štědronský <r.lkml@regnarg.cz> - 2017-03-20 13:00 +0100
          Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes bfields@fieldses.org (J. Bruce Fields) - 2017-03-21 16:50 +0100
            Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Jan Kara <jack@suse.cz> - 2017-03-21 17:50 +0100
              Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes "J. Bruce Fields" <bfields@fieldses.org> - 2017-03-21 18:50 +0100
    Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-14 11:20 +0100
      Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský <r.lkml@regnarg.cz> - 2017-03-14 13:50 +0100
        Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-14 15:00 +0100
          Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský <r.lkml@regnarg.cz> - 2017-03-14 16:00 +0100
            Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-14 23:40 +0100
      Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Jan Kara <jack@suse.cz> - 2017-03-15 15:10 +0100
        Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Amir Goldstein <amir73il@gmail.com> - 2017-03-15 15:40 +0100
          Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Jan Kara <jack@suse.cz> - 2017-03-16 11:40 +0100
    Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Michael Kerrisk <mtk.manpages@gmail.com> - 2017-03-15 06:00 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1605804 — Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes

From	Jan Kara <jack@suse.cz>
Date	2017-03-21 17:50 +0100
Subject	Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Message-ID	<tnv8T-5UV-23@gated-at.bofh.it>
In reply to	#1605741

On Tue 21-03-17 11:38:49, J. Bruce Fields wrote:
> On Sun, Mar 19, 2017 at 11:19:43AM +0100, Jan Kara wrote:
> > On Tue 14-03-17 13:18:01, Amir Goldstein wrote:
> > > On Tue, Mar 14, 2017 at 1:03 AM, Filip Štědronský <r.lklm@regnarg.cz> wrote:
> > > > Besause fanotify requires `struct path`, the event cannot be generated
> > > > directly in `fsnotify_move` and friends because they only get the inode
> > > > (and their callers, `vfs_rename`&co. cannot supply any better info).
> > > > So instead it needs to be generated higher in the call chain, i.e. in
> > > > the callers of functions like `vfs_rename`.
> > > >
> > > > This leads to some code duplication. Currently, there are several places
> > > > whence functions like `vfs_rename` or `vfs_unlink` are called:
> > > >
> > > >   * syscall handlers (done)
> > > >   * NFS server (done)
> > > >   * stacked filesystems
> > > >       - ecryptfs (done)
> > > >       - overlayfs
> > > >         (Currently doesn't report even ordinary fanotify events, because
> > > >          it internally clones the upper mount; not sure about the
> > > >          rationale.  One can always watch the overlay mount instead.)
> > > >   * few rather minor things
> > > >       - devtmpfs
> > > >         (its internal changes are not tied to any vfsmount so it cannot
> > > >          emit mount-scoped events)
> > > >       - cachefiles (done)
> > > >       - ipc/mqueue.c (done)
> > > >       - fs/nfsd/nfs4recover.c (done)
> > > >       - kernel/bpf/inode.c (done)
> > > >         net/unix/af_unix.c (done)
> > > >
> > > > (grep -rE '\bvfs_(rename|unlink|mknod|whiteout|create|mkdir|rmdir|symlink|link)\(')
> > > >
> > > > Signed-off-by: Filip Štědronský <r.lkml@regnarg.cz>
> > > >
> > > > ---
> > > >
> > > > An alternative might be to create wrapper functions like
> > > > vfs_path_(rename|unlink|...). They could also take care of calling
> > > > security_path_(rename|unlink|...), which is currently also up to
> > > > the indvidual callers (possibly with a flag because it might not
> > > > be always desired).
> > > 
> > > That's an interesting idea. There is some duplicity between security/audit
> > > hook and fsnotify hooks. It should be interesting to try and deduplicate
> > > some of this code.
> > 
> > Yeah, but ecryptfs or nfsd don't actually call these security hooks AFAICT.
> 
> We don't?  E.g. nfsd_unlink calls vfs_unlink which calls
> security_inode_unlink().

OK, I have not been specific enough :). ecryptfs or nfsd don't call *path*
security hooks AFAICT - e.g. security_path_unlink() from nfsd_unlink().

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

[toc] | [prev] | [next] | [standalone]

#1605856 — Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes

From	"J. Bruce Fields" <bfields@fieldses.org>
Date	2017-03-21 18:50 +0100
Subject	Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Message-ID	<tnw4V-6ys-11@gated-at.bofh.it>
In reply to	#1605804

On Tue, Mar 21, 2017 at 05:41:22PM +0100, Jan Kara wrote:
> On Tue 21-03-17 11:38:49, J. Bruce Fields wrote:
> > On Sun, Mar 19, 2017 at 11:19:43AM +0100, Jan Kara wrote:
> > > On Tue 14-03-17 13:18:01, Amir Goldstein wrote:
> > > > On Tue, Mar 14, 2017 at 1:03 AM, Filip Štědronský <r.lklm@regnarg.cz> wrote:
> > > > > An alternative might be to create wrapper functions like
> > > > > vfs_path_(rename|unlink|...). They could also take care of calling
> > > > > security_path_(rename|unlink|...), which is currently also up to
> > > > > the indvidual callers (possibly with a flag because it might not
> > > > > be always desired).
> > > > 
> > > > That's an interesting idea. There is some duplicity between security/audit
> > > > hook and fsnotify hooks. It should be interesting to try and deduplicate
> > > > some of this code.
> > > 
> > > Yeah, but ecryptfs or nfsd don't actually call these security hooks AFAICT.
> > 
> > We don't?  E.g. nfsd_unlink calls vfs_unlink which calls
> > security_inode_unlink().
> 
> OK, I have not been specific enough :). ecryptfs or nfsd don't call *path*
> security hooks AFAICT - e.g. security_path_unlink() from nfsd_unlink().

Oh, got it, thanks.

But, no, nfsd is definitely is not meant to be invisible to security
modules, so that's just a bug.

--b.

[toc] | [prev] | [next] | [standalone]

#1600192

From	Amir Goldstein <amir73il@gmail.com>
Date	2017-03-14 11:20 +0100
Message-ID	<tkRIC-70X-11@gated-at.bofh.it>
In reply to	#1599909

On Tue, Mar 14, 2017 at 1:02 AM, Filip Štědronský <r.lklm@regnarg.cz> wrote:
> Fanotify currently does not report directory modification events
> (rename, unlink, etc.). But these events are essential for about half of
> concievable fanotify use cases, especially:
>
>   - file system indexers / desktop search tools
>   - file synchronization tools (like Dropbox, Nextcloud, etc.),
>     online backup tools

This last one is the use case of my employer, Ctera Networks.
Out of curiosity, what is the use case that you are focusing on?

>
> and pretty much any app that needs to maintain and up-to-date internal
> representation of current contents of the file system.
>
> All applications of the above classes that I'm aware of currently use
> recursive inotify watches, which do not scale (my home dir has ~200k
> directories, creating all the watches takes ~2min and eats several tens
> of MB of kernel memory).
>
> There have been many calls for such a feature, pretty much since the
> creation of fanotify in 2009:
>   * By GNOME developers:
>     https://wiki.gnome.org/BastienNocera/KernelWishlist#VFS.2C_filesystems
>   * By KDE developers:
>     http://lkml.kernel.org/r/201211011352.42476.Martin%40lichtvoll.de
>     'Better support for (desktop) file search / indexing applications'
>   * And others:
>     http://lkml.kernel.org/r/AANLkTi=owK=WZW4oNtpm5WpAZhqCQUdTR2K5gzJ_MqZ+%40mail.gmail.com
>     'Fanotify mv/rename?'
>     http://lkml.kernel.org/r/1238158043.23703.20.camel%40fatty
>     'Issues with using fanotify for a filesystem indexer'
>

Thanks for sharing this summary!
I had the feeling that all recursive inotify users are hiding in the shadows,
but was missing more concrete evidence.

> Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the
> contents of a directory change (a directory entry is added, removed or
> renamed). This covers all the currently missing events: rename, unlink,
> mknod, mkdir, rmdir, etc.
>
> Included with the event is a file descriptor to the modified directory
> but no further info. The userspace application is expected to rescan the
> whole directory and update its model accordingly. This needs to be done
> carefully to prevent race conditions. A cross-directory rename generates
> two FAN_MODIFY_DIR events.
>

Your approach is interesting and I am glad you shared it with us.
I do like it and it gives me an idea, I am going to prepare a branch
with a subset
of my patches, so you can try them with your userspace sample program.

In comments to your patches I am going to argue that, as a matter of fact,
you can take a small sub set of my patches and get the same functionality
of FAN_MODIFY_DIR, with code that is *less* complex then what you propose.
So please treat my comments as technical comments how FAN_MODIFY_DIR
should be implemented IMO and not as an attempt to reject an opposing proposal.

> This minimalistic approach has several advantages:
>   * Does not require changes to the fanotify userspace API or the
>     fsnotify in-kernel framework, apart from adding a new event.

About the argument of not having to change in-kernel framework,
I don't think it should be a consideration at all.
I claim that my 1st fsnotify cleanup series is an improvement of the framework
(https://github.com/amir73il/linux/commits/fsnotify_dentry)
regardless of additional functionality.
So changing the in-kernel framework by adding complexity may be bad,
but changing it by simplifying and making code more readable should not
be an argument against the "non-minimalistic" approach.

About the change to usespace API, if you strip off the *added* functionality
of super block watch from my patches, your proposal for user API looks
like this:

    fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT, \
                           FAN_MODIFY_DIR|FAN_ONDIR,

And my proposal for user API looks like this:

    fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT, \
                           FAN_MOVE|FAN_CREATE|FAN_DELETE,

You can see why my proposal for user API is not any less minimalistic
and it is fully compatible with existing intotify API for the same events.

I understand why it is hard to see this behind my added functionality,
so I will post a minimalistic branch and test program as an example.

>     Especially doesn't complicate it by adding string fields.

So the string fields in my proposal are optional.
userspace opts-in to get them by specifying the  FAN_EVENT_INFO_NAME flag:

    fanotify_init(FAN_CLOEXEC | FAN_CLASS_NOTIF |
                       FAN_EVENT_INFO_PARENT | FAN_EVENT_INFO_NAME,

If you don't specify FAN_EVENT_INFO_NAME, you can get filename events
FAN_MOVE|FAN_CREATE|FAN_DELETE without the name.

What you do get is the file descriptor of the parent. sounds familiar? ;-)

The flag FAN_EVENT_INFO_PARENT in an explicit opt-in to get parent fd
instead of victim id on specific events.
If  FAN_EVENT_INFO_PARENT is not specified in fanotify_init()
the filename events FAN_MOVE|FAN_CREATE|FAN_DELETE are masked
out of fanotify_mark().

This is important because your patchs adds FAN_MODIFY_DIR to
FAN_ALL_EVENTS (as does mine).
So old fanotify userspace code, compiled with new kernel headers, with:

    fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT,
                           FAN_ALL_EVENTS,

Will start getting your FAN_MODIFY_DIR events and those programs
might have wrong assumptions about the provided fd.

So I chose to have stronger backward compatibility, and added the opt-in
FAN_EVENT_INFO_PARENT flag.

>   * Has simple and clear semantics, even with multiple renames occurring
>     in parallel etc. In case of any inconsistencies, one can simply wait
>     for a while and rescan again.
>   * FAN_MODIFY_DIR events are easily merged in case of multiple
>     operations on a directory (e.g. deleting all files).
>

I agree those 2 are important advantages.
You could get them with my proposed API when dropping the
FAN_EVENT_INFO_NAME flag.

Thanks for pointing that out.

> Signed-off-by: Filip Štědronský <r.lkml@regnarg.cz>
>
> ---
>
> An alternative might be to create wrapper functions like
> vfs_path_(rename|unlink|...). They could also take care of calling
> security_path_(rename|unlink|...), which is currently also up to
> the indvidual callers (possibly with a flag because it might not
> be always desired).
>
> An alternative was proposed by Amir Goldstein in several long series of
> patches that add superblock-scoped (as opposed to vfsmount-scoped)
> fanotify watches and specific dentry manipulation events with filenames:
>
>     http://lkml.kernel.org/r/1481984434-13293-1-git-send-email-amir73il%40gmail.com
>     http://lkml.kernel.org/r/1482247207-4424-1-git-send-email-amir73il%40gmail.com
>     http://lkml.kernel.org/r/1476126784-12520-1-git-send-email-amir73il%40gmail.com
>     http://lkml.kernel.org/r/1489411223-12081-1-git-send-email-amir73il%40gmail.com
>
> There is large but not complete overlap between that proposal and
> mine (which is was originally created over a year ago, before Amir's
> work, but never posted).
>

I am proud of the role I played to help your proposal see the day of light :-)

> I think the superblock watch idea is very interesting because it might
> in the future allow reporing fsnotify events from remote and virtual
> filesystems. So I'm posting this more as a potential source of more
> ideas for discussion, or a fallback proposal in case Amir's patches
> don't make it.
> ---
>  fs/notify/fanotify/fanotify.c    |  1 +
>  include/linux/fsnotify.h         | 17 +++++++++++++++++
>  include/linux/fsnotify_backend.h |  1 +
>  include/uapi/linux/fanotify.h    |  5 ++++-
>  4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index bbc175d4213d..5178b06c338c 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -186,6 +186,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>
>         BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
>         BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
> +       BUILD_BUG_ON(FAN_MODIFY_DIR != FS_MODIFY_DIR);
>         BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
>         BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
>         BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index b43d3f5bd9ea..00fb87c975d6 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -208,6 +208,23 @@ static inline void fsnotify_modify(struct file *file)
>  }
>
>  /*
> + * fsnotify_modifydir - directory contents were changed
> + * (as a result of rename, creat, unlink, etc.)
> + */
> +static inline void fsnotify_modify_dir(struct path *path)
> +{
> +       struct inode *inode = path->dentry->d_inode;
> +       __u32 mask = FS_MODIFY_DIR;
> +
> +       if (S_ISDIR(inode->i_mode))
> +               mask |= FS_ISDIR;

It is going to be somewhat confusing for users to understand
the difference between FS_MODIFY_DIR and FS_MODIFY_DIR|FS_ISDIR.
IMO, it is much more intuitive (and compatible with intoify semantics)
when users
get specific event information, e.g. FS_DELETE and FS_DELETE|FS_ISDIR.


> +       else
> +               return;
> +
> +       fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
> +}
> +
> +/*
>   * fsnotify_open - file was opened
>   */
>  static inline void fsnotify_open(struct file *file)
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 487246546ebe..7751b337ec31 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -42,6 +42,7 @@
>
>  #define FS_OPEN_PERM           0x00010000      /* open event in an permission hook */
>  #define FS_ACCESS_PERM         0x00020000      /* access event in a permissions hook */
> +#define FS_MODIFY_DIR          0x00040000      /* directory changed (rename/unlink/...) */
>
>  #define FS_EXCL_UNLINK         0x04000000      /* do not send events if object is unlinked */
>  #define FS_ISDIR               0x40000000      /* event occurred against dir */
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index 030508d195d3..f14e048d492a 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -15,6 +15,8 @@
>  #define FAN_OPEN_PERM          0x00010000      /* File open in perm check */
>  #define FAN_ACCESS_PERM                0x00020000      /* File accessed in perm check */
>
> +#define FAN_MODIFY_DIR         0x00040000      /* directory changed (rename/unlink/...) */
> +
>  #define FAN_ONDIR              0x40000000      /* event occurred against dir */
>
>  #define FAN_EVENT_ON_CHILD     0x08000000      /* interested in child events */
> @@ -67,7 +69,8 @@
>  #define FAN_ALL_EVENTS (FAN_ACCESS |\
>                         FAN_MODIFY |\
>                         FAN_CLOSE |\
> -                       FAN_OPEN)
> +                       FAN_OPEN |\
> +                       FAN_MODIFY_DIR)
>
>  /*
>   * All events which require a permission response from userspace
> --
> 2.11.1
>

[toc] | [prev] | [next] | [standalone]

#1600292

From	Filip Štědronský <r.lkml@regnarg.cz>
Date	2017-03-14 13:50 +0100
Message-ID	<tkU3L-8W-7@gated-at.bofh.it>
In reply to	#1600192

Hi,

On Tue, Mar 14, 2017 at 12:11:40PM +0200, Amir Goldstein wrote:
> >   - file system indexers / desktop search tools
> >   - file synchronization tools (like Dropbox, Nextcloud, etc.),
> >     online backup tools
> 
> This last one is the use case of my employer, Ctera Networks.
> Out of curiosity, what is the use case that you are focusing on?

I'm working on a file synchronization tool as part of my bachelor
thesis at Charles University.

When I started (now over a year ago ... long story), there were AFIK no
attempts at solving the recursive inotify issue, only a lot of
complaints, so I cobbled up together something simple (I'm not a kernel
developer by trade, this was my first patch) that would allow me to
work on the userspace parts, which are the main bulk.

I try to focus on algorithmic and implementation efficiency as opposed
to fancy GUIs and similar. I want it to be fast with 100k directories
and 10M files in home dir. But it's very WIP so we'll see how that turns
out.

> I had the feeling that all recursive inotify users are hiding in the shadows,
> but was missing more concrete evidence.

Yes, even Dropbox uses inotify. They have articles in their help on how
to increase inotify.max_user_watches: https://www.dropbox.com/help/145.
That's not good PR. ;-) (I'm not affiliated with DB, just pointing that
out.)

> About the argument of not having to change in-kernel framework,
> I don't think it should be a consideration at all.

Understood. I tried to stay conservative and non-controversial because I
imagined that radical framework changes would take months of discussions
(look at the history of statx) and this issue seems to be pressing for
quite a lot of people.  But rushing is of course not the best strategy
either, there are always compromises.

> If you don't specify FAN_EVENT_INFO_NAME, you can get filename events
> FAN_MOVE|FAN_CREATE|FAN_DELETE without the name.
> 
> What you do get is the file descriptor of the parent. sounds familiar? ;-)

I didn't notice this bit. That sounds like a win-win.

Filip

[toc] | [prev] | [next] | [standalone]

#1600435

From	Amir Goldstein <amir73il@gmail.com>
Date	2017-03-14 15:00 +0100
Message-ID	<tkV9x-Uf-27@gated-at.bofh.it>
In reply to	#1600292

On Tue, Mar 14, 2017 at 2:41 PM, Filip Štědronský <r.lkml@regnarg.cz> wrote:
> Hi,
>
> On Tue, Mar 14, 2017 at 12:11:40PM +0200, Amir Goldstein wrote:
>> >   - file system indexers / desktop search tools
>> >   - file synchronization tools (like Dropbox, Nextcloud, etc.),
>> >     online backup tools
>>
>> This last one is the use case of my employer, Ctera Networks.
>> Out of curiosity, what is the use case that you are focusing on?
>
> I'm working on a file synchronization tool as part of my bachelor
> thesis at Charles University.
>
> When I started (now over a year ago ... long story), there were AFIK no
> attempts at solving the recursive inotify issue, only a lot of
> complaints, so I cobbled up together something simple (I'm not a kernel
> developer by trade, this was my first patch) that would allow me to
> work on the userspace parts, which are the main bulk.
>

That sounds very useful. I was actually looking for an open userspace
implementation were I could demonstrate my patches, but all the projects
I looked at (beagle etc..) seems to have died from desperation waiting
for intotify scalabiltiy to be fixed...

> I try to focus on algorithmic and implementation efficiency as opposed
> to fancy GUIs and similar. I want it to be fast with 100k directories
> and 10M files in home dir. But it's very WIP so we'll see how that turns
> out.
>

I did some basic tests of my super block watch with 1M directories,
and currently the code is under integration testing in our company.

I prepared a drop-in replacement for our fs change listener class
that was previously implemented with inotify.
Currently, the userspace code still "thinks" it is adding watches on directories
recursively, but what it really does is just gets all their file handles.
Then, the code that maps an event to "watch descriptor" actually matches
the fhandle in the event to the directory fhandle (much like in your example).

If you are not married to using your kernel patch, you can just go a head and
try my patch set.
You can fetch it directly from:
https://github.com/amir73il/linux/tree/fanotify_sb

It has been recently rebased over v4.11-rc1.

There are also ready ports for stable kernels v4.1 v4.4 and v4.9
in my github if that serves you better.

Marko Rauhamaa [CCed] has also tested my patches to his satisfaction.

>> I had the feeling that all recursive inotify users are hiding in the shadows,
>> but was missing more concrete evidence.
>
> Yes, even Dropbox uses inotify. They have articles in their help on how
> to increase inotify.max_user_watches: https://www.dropbox.com/help/145.
> That's not good PR. ;-) (I'm not affiliated with DB, just pointing that
> out.)
>
>> About the argument of not having to change in-kernel framework,
>> I don't think it should be a consideration at all.
>
> Understood. I tried to stay conservative and non-controversial because I
> imagined that radical framework changes would take months of discussions
> (look at the history of statx) and this issue seems to be pressing for
> quite a lot of people.  But rushing is of course not the best strategy
> either, there are always compromises.
>

Don't get me wrong, I certainly do *understand* why you would want to stay
away from making radical framework changes.
But I believe it is the best interest of the community to choose the
best solution
going forward while eliminating the fear-of-change factor.
This is why I say that fear of change SHOULD not be a consideration.

>> If you don't specify FAN_EVENT_INFO_NAME, you can get filename events
>> FAN_MOVE|FAN_CREATE|FAN_DELETE without the name.
>>
>> What you do get is the file descriptor of the parent. sounds familiar? ;-)
>
> I didn't notice this bit. That sounds like a win-win.
>

Well, I may me giving you half the truth here.
With current patches not specifying FAN_EVENT_INFO_NAME
will mask out the filename events from the mark, but
1. The implementation could be easily changed
2. You can set FAN_EVENT_INFO_NAME and ignore the
    filename info in userspace. You still get the parent fd
    as your program expects and you get a bonus fhandle
    of the parent for free with the event, see my test program:

github.com/amir73il/fsnotify-utils/blob/master/src/test/fanotify_demo.c#L101

Please let me know if that is sufficient for your needs
or if you need me to prepare a version that delivers filename events
without filename info, therefore allowing to merge filename events.

Sounds to me like if you get an event FAN_DELETE, "aaa",
your implementation can update the db directly without having
to scan the directory, so it should be useful.
For your consideration.

Amir.

[toc] | [prev] | [next] | [standalone]

#1600538

From	Filip Štědronský <r.lkml@regnarg.cz>
Date	2017-03-14 16:00 +0100
Message-ID	<tkW5B-1Ad-47@gated-at.bofh.it>
In reply to	#1600435

Hi,

On Tue, Mar 14, 2017 at 03:55:19PM +0200, Amir Goldstein wrote:
> Please let me know if that is sufficient for your needs
> or if you need me to prepare a version that delivers filename events
> without filename info, therefore allowing to merge filename events.
> 
> Sounds to me like if you get an event FAN_DELETE, "aaa",
> your implementation can update the db directly without having
> to scan the directory, so it should be useful.
> For your consideration.

both approaches might be useful and there are tradeoffs. Direct updates
save us from rescanning but give more events and more context switches.

On the other hand, with an unlimited queue and well-mergable events I
could e.g. sleep for five seconds each time after emptying the queue,
thus giving the events more potential to merge and reducing context
switches.

But one risks blowing up the queue. However, I have some ideas.
Basically we could implement some kind of "bulk read" mode for fanotify
that would pass events to userspace only when
(a) a given event count thresold is hit (e.g. half the queue limit
    if queue is limited), or
(b) the oldest event is older than a specified maximum latency. That
    might vary from 1 second to 5 minutes depending on specific
    application needs (e.g. background backup does not need to be
    low-latency).

This would greatly reduce extra context switches when watching
a large portion (or whole) of the file system. All of this presumably
could be implemented on top of your suggested patches.

Either way, I suggest that implementing the nameless filename events
is a good idea. I do not know whether they will be the best choice
for my application but they probably will be useful for some
applications.

Filip

[toc] | [prev] | [next] | [standalone]

#1600940

From	Amir Goldstein <amir73il@gmail.com>
Date	2017-03-14 23:40 +0100
Message-ID	<tl3gK-6Qt-5@gated-at.bofh.it>
In reply to	#1600538

On Tue, Mar 14, 2017 at 4:48 PM, Filip Štědronský <r.lkml@regnarg.cz> wrote:
> Hi,
>
> On Tue, Mar 14, 2017 at 03:55:19PM +0200, Amir Goldstein wrote:
>> Please let me know if that is sufficient for your needs
>> or if you need me to prepare a version that delivers filename events
>> without filename info, therefore allowing to merge filename events.
>>
>> Sounds to me like if you get an event FAN_DELETE, "aaa",
>> your implementation can update the db directly without having
>> to scan the directory, so it should be useful.
>> For your consideration.
>
> both approaches might be useful and there are tradeoffs. Direct updates
> save us from rescanning but give more events and more context switches.
>
> On the other hand, with an unlimited queue and well-mergable events I
> could e.g. sleep for five seconds each time after emptying the queue,
> thus giving the events more potential to merge and reducing context
> switches.
>
> But one risks blowing up the queue. However, I have some ideas.
> Basically we could implement some kind of "bulk read" mode for fanotify
> that would pass events to userspace only when
> (a) a given event count thresold is hit (e.g. half the queue limit
>     if queue is limited), or
> (b) the oldest event is older than a specified maximum latency. That
>     might vary from 1 second to 5 minutes depending on specific
>     application needs (e.g. background backup does not need to be
>     low-latency).
>
> This would greatly reduce extra context switches when watching
> a large portion (or whole) of the file system. All of this presumably
> could be implemented on top of your suggested patches.
>
> Either way, I suggest that implementing the nameless filename events
> is a good idea. I do not know whether they will be the best choice
> for my application but they probably will be useful for some
> applications.
>

Done and pushed all branches.
Added 2 new patches to the fanotify_dentry set:
ce0c223 fanotify: allow merge of filename events
cd872d1 fanotify: make filename info optional for filename events

You should test with branch fanotify_sb, using super block watch,
because filename events are not supported in mount scope.
You can follow user API used by my fanotify_demo.c program.

Tested with my fanotify_demo test
both without FAN_EVENT_INFO_NAME and without FAN_EVENT_INFO_FH.

Without FAN_EVENT_INFO_NAME more filename events are merged,
but in my test that runs fs ops by a script, most events do not get merged
because of different pid value.

I guess that for the filesync/indexer use case, we would need to add a new
FAN_EVENT_INFO_NOPID flag to loose the irrelevant pid information
in order to be able to merge a lot more filename event on a directory.

I also plan to add FAN_EVENT_INFO_NOFD, to get rid of redundant
fd (when INFO_FH is available) and then be able to drop the refcount
on the victim object.

Cheers,
Amir.

[toc] | [prev] | [next] | [standalone]

#1601427

From	Jan Kara <jack@suse.cz>
Date	2017-03-15 15:10 +0100
Message-ID	<tlhMJ-gZ-1@gated-at.bofh.it>
In reply to	#1600192

Hello,

On Tue 14-03-17 12:11:40, Amir Goldstein wrote:
> > Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the
> > contents of a directory change (a directory entry is added, removed or
> > renamed). This covers all the currently missing events: rename, unlink,
> > mknod, mkdir, rmdir, etc.
> >
> > Included with the event is a file descriptor to the modified directory
> > but no further info. The userspace application is expected to rescan the
> > whole directory and update its model accordingly. This needs to be done
> > carefully to prevent race conditions. A cross-directory rename generates
> > two FAN_MODIFY_DIR events.
> >
> 
> Your approach is interesting and I am glad you shared it with us.
> I do like it and it gives me an idea, I am going to prepare a branch
> with a subset
> of my patches, so you can try them with your userspace sample program.
> 
> In comments to your patches I am going to argue that, as a matter of fact,
> you can take a small sub set of my patches and get the same functionality
> of FAN_MODIFY_DIR, with code that is *less* complex then what you propose.
> So please treat my comments as technical comments how FAN_MODIFY_DIR
> should be implemented IMO and not as an attempt to reject an opposing proposal.

Yeah, so I have yet to read both your patch sets in detail (good news is
that this is getting towards top of my TODO stack and I think I'll do this
when flying to LSF/MM this weekend). With Filip's design I like the
minimalistic approach of adding just DIR_CHANGED event with (albeit
optional) messing with names. That just seems to fit well with the fanotify
design.

> > This minimalistic approach has several advantages:
> >   * Does not require changes to the fanotify userspace API or the
> >     fsnotify in-kernel framework, apart from adding a new event.
> 
> About the argument of not having to change in-kernel framework,
> I don't think it should be a consideration at all.
> I claim that my 1st fsnotify cleanup series is an improvement of the framework
> (https://github.com/amir73il/linux/commits/fsnotify_dentry)
> regardless of additional functionality.
> So changing the in-kernel framework by adding complexity may be bad,
> but changing it by simplifying and making code more readable should not
> be an argument against the "non-minimalistic" approach.
> 
> About the change to usespace API, if you strip off the *added* functionality
> of super block watch from my patches, your proposal for user API looks
> like this:
> 
>     fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT, \
>                            FAN_MODIFY_DIR|FAN_ONDIR,
> 
> And my proposal for user API looks like this:
> 
>     fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT, \
>                            FAN_MOVE|FAN_CREATE|FAN_DELETE,
> 
> You can see why my proposal for user API is not any less minimalistic
> and it is fully compatible with existing intotify API for the same events.
> 
> I understand why it is hard to see this behind my added functionality,
> so I will post a minimalistic branch and test program as an example.
> 
> >     Especially doesn't complicate it by adding string fields.
> 
> So the string fields in my proposal are optional.
> userspace opts-in to get them by specifying the  FAN_EVENT_INFO_NAME flag:
> 
>     fanotify_init(FAN_CLOEXEC | FAN_CLASS_NOTIF |
>                        FAN_EVENT_INFO_PARENT | FAN_EVENT_INFO_NAME,
> 
> If you don't specify FAN_EVENT_INFO_NAME, you can get filename events
> FAN_MOVE|FAN_CREATE|FAN_DELETE without the name.

So when thinking about this again, I'm again concerned that the names need
not tell userspace what it thinks. I know we already discussed this but
in our last discussion I think I forgot to point out that inotify directory
events (and fanotify would be the same AFAICT) may come out of order compared
to real filesystem changes. E.g. sequence:

Task 1		Task 2

mv a b
		mv b c

may come out of inotify as:

IN_MOVED_FROM "b" COOKIE 1
IN_MOVED_TO   "c" COOKIE 1
IN_MOVED_FROM "a" COOKIE 2
IN_MOVED_TO   "b" COOKIE 2

and if user program just blindly belives this sequence its internal
representation of the filesystem will be different from the real state of
the filesystem.

So for now I'm more inclined towards the trivial approach (possibly using
your patches and stripping additional functionality from them). But I'll
leave final decision to when I'll be able to read everything in detail.

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

[toc] | [prev] | [next] | [standalone]

#1601448

From	Amir Goldstein <amir73il@gmail.com>
Date	2017-03-15 15:40 +0100
Message-ID	<tlifM-sV-9@gated-at.bofh.it>
In reply to	#1601427

On Wed, Mar 15, 2017 at 4:05 PM, Jan Kara <jack@suse.cz> wrote:
> Hello,
>
> On Tue 14-03-17 12:11:40, Amir Goldstein wrote:
>> > Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the
>> > contents of a directory change (a directory entry is added, removed or
>> > renamed). This covers all the currently missing events: rename, unlink,
>> > mknod, mkdir, rmdir, etc.
>> >
>> > Included with the event is a file descriptor to the modified directory
>> > but no further info. The userspace application is expected to rescan the
>> > whole directory and update its model accordingly. This needs to be done
>> > carefully to prevent race conditions. A cross-directory rename generates
>> > two FAN_MODIFY_DIR events.
>> >
>>
>> Your approach is interesting and I am glad you shared it with us.
>> I do like it and it gives me an idea, I am going to prepare a branch
>> with a subset
>> of my patches, so you can try them with your userspace sample program.
>>
>> In comments to your patches I am going to argue that, as a matter of fact,
>> you can take a small sub set of my patches and get the same functionality
>> of FAN_MODIFY_DIR, with code that is *less* complex then what you propose.
>> So please treat my comments as technical comments how FAN_MODIFY_DIR
>> should be implemented IMO and not as an attempt to reject an opposing proposal.
>
> Yeah, so I have yet to read both your patch sets in detail (good news is
> that this is getting towards top of my TODO stack and I think I'll do this
> when flying to LSF/MM this weekend). With Filip's design I like the
> minimalistic approach of adding just DIR_CHANGED event with (albeit
> optional) messing with names. That just seems to fit well with the fanotify
> design.
>
>> > This minimalistic approach has several advantages:
>> >   * Does not require changes to the fanotify userspace API or the
>> >     fsnotify in-kernel framework, apart from adding a new event.
>>
>> About the argument of not having to change in-kernel framework,
>> I don't think it should be a consideration at all.
>> I claim that my 1st fsnotify cleanup series is an improvement of the framework
>> (https://github.com/amir73il/linux/commits/fsnotify_dentry)
>> regardless of additional functionality.
>> So changing the in-kernel framework by adding complexity may be bad,
>> but changing it by simplifying and making code more readable should not
>> be an argument against the "non-minimalistic" approach.
>>
>> About the change to usespace API, if you strip off the *added* functionality
>> of super block watch from my patches, your proposal for user API looks
>> like this:
>>
>>     fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT, \
>>                            FAN_MODIFY_DIR|FAN_ONDIR,
>>
>> And my proposal for user API looks like this:
>>
>>     fanotify_mark(fan_fd, FAN_MARK_ADD|FAN_MARK_MOUNT, \
>>                            FAN_MOVE|FAN_CREATE|FAN_DELETE,
>>
>> You can see why my proposal for user API is not any less minimalistic
>> and it is fully compatible with existing intotify API for the same events.
>>
>> I understand why it is hard to see this behind my added functionality,
>> so I will post a minimalistic branch and test program as an example.
>>
>> >     Especially doesn't complicate it by adding string fields.
>>
>> So the string fields in my proposal are optional.
>> userspace opts-in to get them by specifying the  FAN_EVENT_INFO_NAME flag:
>>
>>     fanotify_init(FAN_CLOEXEC | FAN_CLASS_NOTIF |
>>                        FAN_EVENT_INFO_PARENT | FAN_EVENT_INFO_NAME,
>>
>> If you don't specify FAN_EVENT_INFO_NAME, you can get filename events
>> FAN_MOVE|FAN_CREATE|FAN_DELETE without the name.
>
> So when thinking about this again, I'm again concerned that the names need
> not tell userspace what it thinks. I know we already discussed this but
> in our last discussion I think I forgot to point out that inotify directory
> events (and fanotify would be the same AFAICT) may come out of order compared
> to real filesystem changes. E.g. sequence:
>
> Task 1          Task 2
>
> mv a b
>                 mv b c
>
> may come out of inotify as:
>
> IN_MOVED_FROM "b" COOKIE 1
> IN_MOVED_TO   "c" COOKIE 1
> IN_MOVED_FROM "a" COOKIE 2
> IN_MOVED_TO   "b" COOKIE 2
>

Really? How come?

fsnotify_move() inside vfs_rename() is serialized with lock_rename()

I should use this opportunity to note that fanotify event does not have
a cookie field and that I did not add one to my implementation, so that
is a problem.

> and if user program just blindly belives this sequence its internal
> representation of the filesystem will be different from the real state of
> the filesystem.
>
> So for now I'm more inclined towards the trivial approach (possibly using
> your patches and stripping additional functionality from them). But I'll
> leave final decision to when I'll be able to read everything in detail.
>

The addition of filename info is completely optional and the relevant patch
("fanotify: pass filename info for filename events")
Can be yanked out of the series and pushed back to the 'maybe' pile.

Amir.

[toc] | [prev] | [next] | [standalone]

#1602164

From	Jan Kara <jack@suse.cz>
Date	2017-03-16 11:40 +0100
Message-ID	<tlAZ3-5nr-5@gated-at.bofh.it>
In reply to	#1601448

On Wed 15-03-17 16:34:11, Amir Goldstein wrote:
> On Wed, Mar 15, 2017 at 4:05 PM, Jan Kara <jack@suse.cz> wrote:
> >> So the string fields in my proposal are optional.
> >> userspace opts-in to get them by specifying the  FAN_EVENT_INFO_NAME flag:
> >>
> >>     fanotify_init(FAN_CLOEXEC | FAN_CLASS_NOTIF |
> >>                        FAN_EVENT_INFO_PARENT | FAN_EVENT_INFO_NAME,
> >>
> >> If you don't specify FAN_EVENT_INFO_NAME, you can get filename events
> >> FAN_MOVE|FAN_CREATE|FAN_DELETE without the name.
> >
> > So when thinking about this again, I'm again concerned that the names need
> > not tell userspace what it thinks. I know we already discussed this but
> > in our last discussion I think I forgot to point out that inotify directory
> > events (and fanotify would be the same AFAICT) may come out of order compared
> > to real filesystem changes. E.g. sequence:
> >
> > Task 1          Task 2
> >
> > mv a b
> >                 mv b c
> >
> > may come out of inotify as:
> >
> > IN_MOVED_FROM "b" COOKIE 1
> > IN_MOVED_TO   "c" COOKIE 1
> > IN_MOVED_FROM "a" COOKIE 2
> > IN_MOVED_TO   "b" COOKIE 2
> >
> 
> Really? How come?
> 
> fsnotify_move() inside vfs_rename() is serialized with lock_rename()

Sorry, I got confused by the unlocking in vfs_rename() however that unlocks
only the moved file / directory but not the directory in which the rename
is happening. Taking my objection back.

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

[toc] | [prev] | [next] | [standalone]

#1601053

From	Michael Kerrisk <mtk.manpages@gmail.com>
Date	2017-03-15 06:00 +0100
Message-ID	<tl9cu-2sb-9@gated-at.bofh.it>
In reply to	#1599909

[CC += linux-api@vger.kernel.org]

Filip,

Since this is a kernel-user-space API change, please CC linux-api@
(and on future iterations of this patch). The kernel source file
Documentation/SubmitChecklist notes that all Linux kernel patches that
change userspace interfaces should be CCed to
linux-api@vger.kernel.org, so that the various parties who are
interested in API changes are informed. For further information, see
https://www.kernel.org/doc/man-pages/linux-api-ml.html

Thanks,

Michael



On Tue, Mar 14, 2017 at 12:02 AM, Filip Štědronský <r.lklm@regnarg.cz> wrote:
> Fanotify currently does not report directory modification events
> (rename, unlink, etc.). But these events are essential for about half of
> concievable fanotify use cases, especially:
>
>   - file system indexers / desktop search tools
>   - file synchronization tools (like Dropbox, Nextcloud, etc.),
>     online backup tools
>
> and pretty much any app that needs to maintain and up-to-date internal
> representation of current contents of the file system.
>
> All applications of the above classes that I'm aware of currently use
> recursive inotify watches, which do not scale (my home dir has ~200k
> directories, creating all the watches takes ~2min and eats several tens
> of MB of kernel memory).
>
> There have been many calls for such a feature, pretty much since the
> creation of fanotify in 2009:
>   * By GNOME developers:
>     https://wiki.gnome.org/BastienNocera/KernelWishlist#VFS.2C_filesystems
>   * By KDE developers:
>     http://lkml.kernel.org/r/201211011352.42476.Martin%40lichtvoll.de
>     'Better support for (desktop) file search / indexing applications'
>   * And others:
>     http://lkml.kernel.org/r/AANLkTi=owK=WZW4oNtpm5WpAZhqCQUdTR2K5gzJ_MqZ+%40mail.gmail.com
>     'Fanotify mv/rename?'
>     http://lkml.kernel.org/r/1238158043.23703.20.camel%40fatty
>     'Issues with using fanotify for a filesystem indexer'
>
> Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the
> contents of a directory change (a directory entry is added, removed or
> renamed). This covers all the currently missing events: rename, unlink,
> mknod, mkdir, rmdir, etc.
>
> Included with the event is a file descriptor to the modified directory
> but no further info. The userspace application is expected to rescan the
> whole directory and update its model accordingly. This needs to be done
> carefully to prevent race conditions. A cross-directory rename generates
> two FAN_MODIFY_DIR events.
>
> This minimalistic approach has several advantages:
>   * Does not require changes to the fanotify userspace API or the
>     fsnotify in-kernel framework, apart from adding a new event.
>     Especially doesn't complicate it by adding string fields.
>   * Has simple and clear semantics, even with multiple renames occurring
>     in parallel etc. In case of any inconsistencies, one can simply wait
>     for a while and rescan again.
>   * FAN_MODIFY_DIR events are easily merged in case of multiple
>     operations on a directory (e.g. deleting all files).
>
> Signed-off-by: Filip Štědronský <r.lkml@regnarg.cz>
>
> ---
>
> An alternative might be to create wrapper functions like
> vfs_path_(rename|unlink|...). They could also take care of calling
> security_path_(rename|unlink|...), which is currently also up to
> the indvidual callers (possibly with a flag because it might not
> be always desired).
>
> An alternative was proposed by Amir Goldstein in several long series of
> patches that add superblock-scoped (as opposed to vfsmount-scoped)
> fanotify watches and specific dentry manipulation events with filenames:
>
>     http://lkml.kernel.org/r/1481984434-13293-1-git-send-email-amir73il%40gmail.com
>     http://lkml.kernel.org/r/1482247207-4424-1-git-send-email-amir73il%40gmail.com
>     http://lkml.kernel.org/r/1476126784-12520-1-git-send-email-amir73il%40gmail.com
>     http://lkml.kernel.org/r/1489411223-12081-1-git-send-email-amir73il%40gmail.com
>
> There is large but not complete overlap between that proposal and
> mine (which is was originally created over a year ago, before Amir's
> work, but never posted).
>
> I think the superblock watch idea is very interesting because it might
> in the future allow reporing fsnotify events from remote and virtual
> filesystems. So I'm posting this more as a potential source of more
> ideas for discussion, or a fallback proposal in case Amir's patches
> don't make it.
> ---
>  fs/notify/fanotify/fanotify.c    |  1 +
>  include/linux/fsnotify.h         | 17 +++++++++++++++++
>  include/linux/fsnotify_backend.h |  1 +
>  include/uapi/linux/fanotify.h    |  5 ++++-
>  4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index bbc175d4213d..5178b06c338c 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -186,6 +186,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>
>         BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
>         BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
> +       BUILD_BUG_ON(FAN_MODIFY_DIR != FS_MODIFY_DIR);
>         BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
>         BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
>         BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index b43d3f5bd9ea..00fb87c975d6 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -208,6 +208,23 @@ static inline void fsnotify_modify(struct file *file)
>  }
>
>  /*
> + * fsnotify_modifydir - directory contents were changed
> + * (as a result of rename, creat, unlink, etc.)
> + */
> +static inline void fsnotify_modify_dir(struct path *path)
> +{
> +       struct inode *inode = path->dentry->d_inode;
> +       __u32 mask = FS_MODIFY_DIR;
> +
> +       if (S_ISDIR(inode->i_mode))
> +               mask |= FS_ISDIR;
> +       else
> +               return;
> +
> +       fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
> +}
> +
> +/*
>   * fsnotify_open - file was opened
>   */
>  static inline void fsnotify_open(struct file *file)
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 487246546ebe..7751b337ec31 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -42,6 +42,7 @@
>
>  #define FS_OPEN_PERM           0x00010000      /* open event in an permission hook */
>  #define FS_ACCESS_PERM         0x00020000      /* access event in a permissions hook */
> +#define FS_MODIFY_DIR          0x00040000      /* directory changed (rename/unlink/...) */
>
>  #define FS_EXCL_UNLINK         0x04000000      /* do not send events if object is unlinked */
>  #define FS_ISDIR               0x40000000      /* event occurred against dir */
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index 030508d195d3..f14e048d492a 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -15,6 +15,8 @@
>  #define FAN_OPEN_PERM          0x00010000      /* File open in perm check */
>  #define FAN_ACCESS_PERM                0x00020000      /* File accessed in perm check */
>
> +#define FAN_MODIFY_DIR         0x00040000      /* directory changed (rename/unlink/...) */
> +
>  #define FAN_ONDIR              0x40000000      /* event occurred against dir */
>
>  #define FAN_EVENT_ON_CHILD     0x08000000      /* interested in child events */
> @@ -67,7 +69,8 @@
>  #define FAN_ALL_EVENTS (FAN_ACCESS |\
>                         FAN_MODIFY |\
>                         FAN_CLOSE |\
> -                       FAN_OPEN)
> +                       FAN_OPEN |\
> +                       FAN_MODIFY_DIR)
>
>  /*
>   * All events which require a permission response from userspace
> --
> 2.11.1
>



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

csiph-web

[RFC 1/2] fanotify: new event FAN_MODIFY_DIR

Contents

#1605804 — Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes

#1605856 — Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes

#1600192

#1600292

#1600435

#1600538

#1600940

#1601427

#1601448

#1602164

#1601053