public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Paris <eparis@redhat.com>
To: "C. Scott Ananian" <cscott@laptop.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH -v4 00/14] fsnotify, dnotify, and inotify
Date: Sun, 21 Dec 2008 22:22:06 -0500	[thread overview]
Message-ID: <1229916126.29604.47.camel@localhost.localdomain> (raw)
In-Reply-To: <c6d9bea0812181536p2a3aaeacmbd6af0474279b7b@mail.gmail.com>

On Thu, 2008-12-18 at 18:36 -0500, C. Scott Ananian wrote:
> On Fri, Dec 12, 2008 at 4:51 PM, Eric Paris <eparis@redhat.com> wrote:
> > The following series implements a new generic in kernel filesystem
> > notification system, fsnotify.  On top of fsnotify I reimplement dnotify and
> > inotify.  I have not finished with the change from inotify although I think
> > inotify_user should be completed.  In kernel inotify users (aka audit) still
> > (until I get positive feedback) relay on the old inotify backend.  This can be
> > 'easily' fixed.
> > All of this is in preperation for fanotify and using fanotify as an on access
> > file scanner.   So you better know it's coming.
> > Why is this useful?  Because I actually shrank the struct inode.  That's
> > right, my code is smaller and faster.  Eat that.
> 
> As a desktop-search-and-indexing developer, it doesn't seem like
> fanotify is going to give me anything I want.  The inotify/dnotify
> restructuring to fsnotify seems reasonable, but not exciting (to me).
> 
> From a desktop search perspective, my wishlist reads like this:
>  1) An 'autoadd' option added to inotify directory watches, so that
> newly-created subdirectories get atomically added to the watch.  That
> would prevent missed IN_MOVED_FROM and IN_MOVED_TO in a newly created
> directory.
>  2) A reasonable interface to map inode #s to "the current path to
> this inode" -- or failing that, an iopen or ilink syscall.  This would
> allow the search index to maintain inode #s instead of path names for
> files, which saves a lot of IN_MOVE processing and races (especially
> for directory moves, which require recursive path renaming).
>  3) Dream case: in-kernel dirty bits.  I don't *really* want to know
> all the details; I just want to know which inotify watches are dirty,
> so I can rescan them.  To avoid races, the query should return a dirty
> watch and atomically clear its dirty flag, so that if it is updated
> again during my indexing, I'd be told to scan it again.
> 
> From the indexing perspective, dealing with a sequence of operations like:
>    1) mkdir -p abc/def
>    2) echo "foo" > abc/def/ghi
>    3) mv abc xyz
>    4) mv xyz/def/ghi jkl
> is entirely too much "fun".  Depending on the races between kernel and
> user space, I might get only:
>    CREATE abc
>    IN_MOVED_TO jkl
> and if I do get my watches put in place after step (1) I've still got
> to deal with a recursive rename in step (3), the possibility of
> getting notification of (2) after the rename in (3) (so abc/def/ghi no
> longer exists), and other delights.
> 
> As far as I can tell, fanotify only helps with the 'recursive watch'
> problem (which could be solved with 'autoadd' or just using the
> algorithm in http://mail.gnome.org/archives/dashboard-hackers/2004-October/msg00022.html
> ), and doesn't give me any tools to deal with the actual hard races or
> path-maintenance problems.

You are absolutely correct that fanotify doesn't help with object
movement or path maintenance.  Neither had been requested, but
notification (that an inode moved) shouldn't be impossible (although the
hooks are going to be a lot more involved and will probably take some
fighting with the VFS people, my current fanotify hooks use what is
already being handed to fsnotify_* today)  To directly answer you
requests

1) autoadd isn't really what I'm looking at, but maybe someday I could
take a peek, at first glance it doesn't seem unreasonable an idea, but I
don't see how the userspace interface could work.  Without the call the
inotify_init to get the watch descriptor how can userspace know what
these new events are?  Only possibility I see for this is if inotify got
an extensible userspace interface.  In any case I'd be hard pressed to
call it a high priority since it's already possible to get this and the
intention of the addition is to make userspace code easier.

2) major vfs and every FS redesign me thinks.

3) What you want is IN_MODIFY from every inode but you want them to all
coallese until you grab one instead of only merging the same event type
if they are the last 2 in the notification queue.  Not sure of a
particularly clean/fast way to implement that right offhand, we'd have
to run the entire notification queue every time before we add an event
to the end, but at least this is doable with the constraints of the
inotify user interface.

Can't this already be done in userspace just by draining the queue,
matching events, throwing duplicates away, and then processing whatever
is left?  You know there is atomicity since removal of an event and the
addition of an event both hold the inotify_dev->ev_mutex.

In any case, I'm going to let your thoughts rattle around in my brain
while I'm still trying to rewrite inotify and dnotify to a better base.
My first inclination is to stop using inotify and start using fanotify.
Abandon filenames and start using device/inode pairs and you get
everything you need.  But I'm certain that isn't that case    :)

-Eric


  reply	other threads:[~2008-12-22  3:22 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-12 21:51 [RFC PATCH -v4 00/14] fsnotify, dnotify, and inotify Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 01/14] filesystem notification: create fs/notify to contain all fs notification Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 02/14] fsnotify: pass a file instead of an inode to open, read, and write Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 03/14] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 04/14] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
2008-12-13 15:29   ` Christoph Hellwig
2008-12-12 21:51 ` [RFC PATCH -v4 05/14] fsnotify: unified filesystem notification backend Eric Paris
2008-12-13  2:54   ` Evgeniy Polyakov
2008-12-13 15:01     ` Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 06/14] fsnotify: add group priorities Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 07/14] fsnotify: add in inode fsnotify markings Eric Paris
2008-12-13  3:07   ` Evgeniy Polyakov
2008-12-13 16:35     ` Eric Paris
2008-12-22 13:43       ` Al Viro
2008-12-22 14:45         ` Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 08/14] fsnotify: parent event notification Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 09/14] dnotify: reimplement dnotify using fsnotify Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 10/14] fsnotify: generic notification queue and waitq Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 11/14] fsnotify: include pathnames with entries when possible Eric Paris
2008-12-13  3:19   ` Evgeniy Polyakov
2008-12-13 16:42     ` Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 12/14] fsnotify: add correlations between events Eric Paris
2008-12-18 22:28   ` C. Scott Ananian
2008-12-22  2:40     ` Eric Paris
2008-12-22  9:01       ` Evgeniy Polyakov
2008-12-22 20:06       ` C. Scott Ananian
2008-12-12 21:52 ` [RFC PATCH -v4 13/14] inotify: reimplement inotify using fsnotify Eric Paris
2008-12-13  3:22   ` Evgeniy Polyakov
2008-12-13 16:44     ` Eric Paris
2008-12-15 15:48       ` Evgeniy Polyakov
2008-12-12 21:52 ` [RFC PATCH -v4 14/14] shit on top for debugging Eric Paris
2008-12-14 22:40   ` James Morris
2008-12-14 22:47     ` Eric Paris
2008-12-18 23:36 ` [RFC PATCH -v4 00/14] fsnotify, dnotify, and inotify C. Scott Ananian
2008-12-22  3:22   ` Eric Paris [this message]
2008-12-22 10:58     ` Niraj Kumar
2008-12-22 19:59     ` C. Scott Ananian
2008-12-22 20:53       ` Eric Paris
2008-12-29 18:19         ` C. Scott Ananian
2008-12-22 21:04       ` Al Viro
2008-12-22 23:08         ` C. Scott Ananian
2008-12-22 23:20           ` Al Viro
2008-12-22 23:21           ` Christoph Hellwig
2008-12-25 18:17             ` C. Scott Ananian
2008-12-25 20:33               ` Al Viro
2008-12-26  0:58                 ` C. Scott Ananian
2008-12-26  1:44                   ` Al Viro
2008-12-27 21:23                     ` C. Scott Ananian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1229916126.29604.47.camel@localhost.localdomain \
    --to=eparis@redhat.com \
    --cc=cscott@laptop.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox