On Sat, 12 Jul 2014, Mathieu Desnoyers wrote:
> I'm perhaps missing something here, but what happens with the
> following scenario ?
>
> Initial conditions:
>
> tkf->seq = 0
> tkf->base[0] and tkf->base[1] are initialized.
>
> CPU 0 CPU 1
> ------------ ----------------
> update:
> tkf->seq++
> smb_wmb()
> tkf->seq++ (reordered before update)
> reader:
> seq = tkf->seq (reads 2)
> smp_rmb()
> idx = seq & 0x01
> now = now(tkf->base[idx] (reads
> base[0])
> update(tkf->base[0], tk) (racy concurrent update)
> smp_rmb()
> while (seq != tkf->seq) (they are
> equal)
>
> So AFAIU, we end up returning a corrupted value. Adding a
> smp_wmb() between update of base[0] and increment of seq,
> as well as between update of base[1] and the _following_
> increment of seq (next update call) would fix this.
>
> Thoughts ?
Well, the actual implementation does:
+ /* Force readers off to base[1] */
+ raw_write_seqcount_begin(&tkf->seq);
+
+ /* Update base[0] */
+ base->clock = clk;
+ base->cycle_last = clk->cycle_last;
+ base->base = tbase;
+ base->shift = shift;
+ base->mult = mult;
+
+ /* Force readers back to base[0] */
+ raw_write_seqcount_end(&tkf->seq);
+
+ /* Update base[1] */
+ base++;
+ base->clock = clk;
+ base->cycle_last = clk->cycle_last;
+ base->base = tbase;
+ base->shift = shift;
+ base->mult = mult;
Where raw_write_seqcount_begin/raw_write_seqcount_end provides the
required memory barriers.
Sure I should update the documentaion ....
Thanks,
tglx
--
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/
