Re: [patch 54/55] timekeeping: Provide fast and NMI safe access to CLOCK_MONOTONIC[_RAW]

[Thomas Gleixner]

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/