lkml.org 
[lkml]   [2004]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC/PATCH] inotify -- a dnotify replacement
    From
    On Tue, May 11, 2004 at 01:46:47PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
    > On Tue, May 11, 2004 at 08:20:01AM -0400, John McCutchan wrote:
    >
    > > Inotify will support watching a hierarchy. The reason it was not
    > > implemented yet is because the one app that I really care about is
    > > nautilus and the maintainer of it says he doesn't care.
    >
    > How are you going to implement that?

    From a quick glance at someone elses implementation of it, I plan on
    walking up the dentries and checking at each level if a watcher on that
    level is interested in events from subdirectories. Is this good practice in
    the kernel?

    > > The big feature that inotify is trying to provide is not having to keep
    > > a file open (So that unmounting is not affected). I asked for some
    > > guidance from people more familiar with the kernel so that I can
    > > implement this feature, it requires changes made to the inode cache, and
    > > how unmounting is done.
    >
    > Bzzert. First of all, on quite a few filesystems inumbers are stable
    > only when object is pinned down. What's more, if it's not pinned down
    > you've got nothing even remotely resembling a reliable way to tell if
    > two events had happened to the same object - inumbers can be reused.

    The inode will be pinned down, I haven't implemented this yet but I am
    going to change the inode cache (is this the right place? )
    so that if inode->watcher_count > 0 the inode stays pinned. Then when
    the filesystem is unmounted, we will kick off all the watchers on
    each inode.

    >
    > Besides, your "doesn't pin down" is racy as hell - not to mention the
    > way you manage the lists, pretty much every function is an exploitable
    > hole. Hell, just take a look at your "find inode" stuff - you grab
    > superblock, find an inode by inumber (great idea, that - especially
    > since on a bunch of filesystems it will get you BUG() or equivalent)
    > then drop refernce to superblock (at which point it can be destroyed by
    > umount()) _and_ do iput() (which will do lovely, lovely things if that
    > umount did happen). Moreover, you return a pointer to inode, even
    > though there's nothing to hold it alive anymore. And dereference that
    > pointer later on, not caring if it had been freed/reused/whatever.

    Like I said above,as long as an inode has a watcher it will be pinned. As
    for the races, I plan on implementing locking around all of the list operations.
    Perhaps I wasn't very clear that this is very much a WIP and lots of work is
    needed.

    >
    > Overall: hopeless crap. And that's a direct result of your main feature -
    > it's really broken by design.

    Having directory event notification without needing to keep a file open on the
    device is not broken by design. It is the only reasonable solution to
    a problem that needs fixing. You can't simply say that a file manager
    needing to be notified when directories change is broken. How would
    you solve this problem?

    John
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:03    [W:3.423 / U:1.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site