Messages in this thread | | | Date | Fri, 17 Jun 2005 11:30:29 +1000 | From | Nick Piggin <> | Subject | Re: [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 http://au.messenger.yahoo.com
- 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/
| |