On Fri, Sep 04, 2020 at 09:41:42AM +0200, pet...@infradead.org wrote: > On Thu, Aug 27, 2020 at 01:40:42PM +0200, Ahmed S. Darwish wrote: > > > __always_inline void cyc2ns_read_begin(struct cyc2ns_data *data) > > { > > + seqcount_latch_t *seqcount; > > int seq, idx; > > > > preempt_disable_notrace(); > > > > + seqcount = &this_cpu_ptr(&cyc2ns)->seq; > > do { > > - seq = this_cpu_read(cyc2ns.seq.sequence); > > + seq = raw_read_seqcount_latch(seqcount); > > idx = seq & 1; > > > > data->cyc2ns_offset = > > this_cpu_read(cyc2ns.data[idx].cyc2ns_offset); > > data->cyc2ns_mul = > > this_cpu_read(cyc2ns.data[idx].cyc2ns_mul); > > data->cyc2ns_shift = > > this_cpu_read(cyc2ns.data[idx].cyc2ns_shift); > > > > - } while (unlikely(seq != this_cpu_read(cyc2ns.seq.sequence))); > > + } while (read_seqcount_latch_retry(seqcount, seq)); > > } > > So I worried about this change, it obviously generates worse code. But I > was not expecting this: > > Before: > > 196: 0000000000000110 189 FUNC GLOBAL DEFAULT 1 native_sched_clock > > After: > > 195: 0000000000000110 399 FUNC GLOBAL DEFAULT 1 native_sched_clock > > That's _210_ bytes extra!! > > If you look at the disassembly of the thing after it's a complete > trainwreck.
The below delta fixes it again. --- --- a/arch/x86/kernel/tsc.c +++ b/arch/x86/kernel/tsc.c @@ -68,21 +68,19 @@ early_param("tsc_early_khz", tsc_early_k __always_inline void cyc2ns_read_begin(struct cyc2ns_data *data) { - seqcount_latch_t *seqcount; int seq, idx; preempt_disable_notrace(); - seqcount = &this_cpu_ptr(&cyc2ns)->seq; do { - seq = raw_read_seqcount_latch(seqcount); + seq = this_cpu_read(cyc2ns.seq.seqcount.sequence); idx = seq & 1; data->cyc2ns_offset = this_cpu_read(cyc2ns.data[idx].cyc2ns_offset); data->cyc2ns_mul = this_cpu_read(cyc2ns.data[idx].cyc2ns_mul); data->cyc2ns_shift = this_cpu_read(cyc2ns.data[idx].cyc2ns_shift); - } while (read_seqcount_latch_retry(seqcount, seq)); + } while (unlikely(seq != this_cpu_read(cyc2ns.seq.seqcount.sequence))); } __always_inline void cyc2ns_read_end(void)