[lkml]   [2005]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [patch] inotify.
    Robert Love wrote:
    > On Thu, 2005-06-16 at 10:52 -0700, Zach Brown wrote:

    >>>+ if (likely(!atomic_read(&inode->inotify_watch_count)))
    >>>+ return;
    >>Are there still platforms that implement atomic_read() with locks? I
    >>wonder if there isn't a cheaper way for inodes to find out that they're
    >>not involved in inotify.. maybe an inode function pointer that is only
    >>set to queue_event when watchers are around?
    > I don't know what esoteric architectures are doing, but the solution
    > needs to be atomic (or we need to say "we don't care about races"--but
    > its hard not to care about a pointer race). On x86, at least, an
    > atomic_read() is trivial.
    > I actually would not mind being racey (in a safe way) or finding a
    > cheaper solution, especially if we could remove
    > inode->inotify_watch_count altogether (and not replace it with
    > anything).
    > But the overhead here is not biting us (we just went through some
    > off-list benchmarking that led to the inclusion of this check, in fact).

    What we could do is just check list_empty(&inode->inotify_watchers)
    and remove the atomic count completely.

    We don't actually care about getting an exact count at all, just
    whether or not it is empty, and in that case using list_empty is
    no more racy than checking an atomic count, both are done outside
    any locks.

    It is basically just a lock avoidance heuristic. But I think count
    is superfluous - off with its head!

    SUSE Labs, Novell Inc.

    Send instant messages to your online friends

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-06-17 03:34    [W:0.021 / U:108.368 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site