Messages in this thread | | | Date | Fri, 14 Jan 2005 17:57:21 -0500 (EST) | Subject | Re: [PATCH 3/4] relayfs for 2.6.10: locking/lockless implementation | From | Robert Wisniewski <> |
| |
Greg, There are a couple variables used throughout relayfs code that could be modified at any point "simultaneously" by different processes. These variables were not declared volatile, thus when we modify them we need to tell the compiler to refetch from memory as another process could have changed out from under the current stream of execution since the last time there were accessed in the function. An alternative would be to mark the variables that we care about as volatile. I am not sure how best to make that tradeoff (i.e., always forcing a refetch by marking a variable volatile or only at points were we know we need to by memory clobbering) or on what side the Linux community comes down on. We certainly would be happy to go either way with the relayfs code, i.e., mark them variable and used the standard atomic operations. That explains compare_and_store, atomic_add, and atomic_sub. It does not explain the memory clobbering around the atomic set operation, which I'm guessing was there just to be consistent with the other operations, and could, I believe, be removed. Hopefully that helps answer the question. If it doesn't please feel free to ask more. Thanks.
-bob
Robert Wisniewski The K42 MP OS Project Advanced Operating Systems Scalable Parallel Systems IBM T.J. Watson Research Center 914-945-3181 http://www.research.ibm.com/K42/ bob@watson.ibm.com
----
On Thu, Jan 13, 2005 at 10:04:33PM -0500, Karim Yaghmour wrote: > +/** > + * compare_and_store_volatile - self-explicit > + * @ptr: ptr to the word that will receive the new value > + * @oval: the value we think is currently in *ptr > + * @nval: the value *ptr will get if we were right > + */ > +inline int > +compare_and_store_volatile(volatile u32 *ptr, > + u32 oval, > + u32 nval) > +{ > + u32 prev; > + > + barrier(); > + prev = cmpxchg(ptr, oval, nval); > + barrier(); > + > + return (prev == oval); > +}
Why is this function needed? What's wrong with the "normal" cmpxchg?
> +/** > + * atomic_set_volatile - atomically set the value in ptr to nval. > + * @ptr: ptr to the word that will receive the new value > + * @nval: the new value > + */ > +inline void > +atomic_set_volatile(atomic_t *ptr, > + u32 nval) > +{ > + barrier(); > + atomic_set(ptr, (int)nval); > + barrier(); > +}
Same here, what's wrong with the normal atomic_set()?
Same question also goes for the other functions like this in this file.
thanks,
greg k-h - 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/
--------------080309080704060007080707-- - 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/
| |