On Thu, 2015-03-05 at 16:35 +0100, Frederic Weisbecker wrote: > On Mon, Mar 02, 2015 at 10:42:11AM -0800, Jason Low wrote:
> > +/* Sample thread_group_cputimer values in "cputimer", copy results to > > "times" */ > > +static inline void sample_group_cputimer(struct task_cputime *times, > > + struct thread_group_cputimer *cputimer) > > +{ > > + times->utime = atomic64_read(&cputimer->utime); > > + times->stime = atomic64_read(&cputimer->stime); > > + times->sum_exec_runtime = > > atomic64_read(&cputimer->sum_exec_runtime); > > So, in the case we are calling that right after setting cputimer->running, I > guess we are fine > because we just updated cputimer with the freshest values. > > But if we are reading this a while after, say several ticks further, there is > a chance that > we read stale values since we don't lock anymore. > > I don't know if it matters or not, I guess it depends how stale it can be and > how much precision > we expect from posix cpu timers. It probably doesn't matter. > > But just in case, atomic64_read_return(&cputimer->utime, 0) would make sure > we get the freshest > value because it performs a full barrier, at the cost of more overhead of > course. (Assuming that is atomic64_add_return :)) Yeah, there aren't any guarantees that we read the freshest value, but since the lock isn't used to serialize subsequent accesses of times->utime, ect..., the values can potentially become stale by the time they get used anyway, even when we have the locking. So I'm not sure if atomic64_add_return(&time, 0) for the reads would really provide much of a benefit when we factor in the extra overhead. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/