Messages in this thread | | | Date | Wed, 08 Mar 2006 19:22:50 +1100 | From | Nick Piggin <> | Subject | Re: [PATCH 6/6] Optimise d_find_alias() [try #6] |
| |
David Howells wrote: > Nick Piggin <nickpiggin@yahoo.com.au> wrote: > > >>Having the smp_rmb() here implies there is some sort of memory barrier >>based synchronisation protocol at a higher level (than this function), >>because you don't actually do anything before them smp_rmb() here. >> >>So can you comment what that is? > > > Hmmm... That probably shouldn't be there. I don't think Andrew Morton and I > actually came to an agreement as to whether it's necessary. I don't think it > is, he thought that it was, though he may have changed his mind. >
I think this is easy to argue that the optimisation is OK _if_ the condition being moved outside the locks is atomic.
The reasoning: if there is _no_ higher synchronisation which guarantees the condition may be flipped one way or the other, then it is equally uncertain of whether the spinlock will be taken before or after any critical sections that modify the value. If there _is_ some higher synchronisation, then the value is going to be correct anyway -- *unless* you are using some consistency based lockless synchronisation (ie. if this value is 10 then that must be false), which is what an rmb could be covering.
Now in this case, the condition isn't exactly atomic but I think that's still OK, because it shouldn't be able to get a false negative or positive during some transient operation (ie. if the list was _never_ empty within our known synchronisation, it will return false, if it was _always_ empty, true).
So I guess the only question is whether some code relies on another value to "know" the result ( if (inode->nr_aliases > 0) {d_find_alias(inode);} ), in which case I _think_ an rmb there can get you out of trouble. But you'd need a comment.
Also: I might be talking complete crap here, so anyone feel free to ridicule me if I'm wrong.
-- 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/
| |