Messages in this thread | | | Date | Wed, 6 Jan 2016 09:28:09 -0800 | Subject | Re: [PATCH 1/2] kernel, timekeeping, add trylock option to ktime_get_with_offset() | From | John Stultz <> |
| |
On Wed, Jan 6, 2016 at 5:00 AM, Prarit Bhargava <prarit@redhat.com> wrote: > -ktime_t ktime_get_with_offset(enum tk_offsets offs) > +ktime_t ktime_get_with_offset(enum tk_offsets offs, int trylock) > { > struct timekeeper *tk = &tk_core.timekeeper; > unsigned int seq; > ktime_t base, *offset = offsets[offs]; > s64 nsecs; > + unsigned long flags = 0; > + > + if (unlikely(!timekeeping_initialized)) > + return ktime_set(0, 0); > > WARN_ON(timekeeping_suspended); > > + if (trylock && !raw_spin_trylock_irqsave(&timekeeper_lock, flags)) > + return ktime_set(KTIME_MAX, 0);
Wait.. this doesn't make sense. The timekeeper lock is only for reading.
What I was suggesting to you off line is to have something that avoids spinning on the seqcounter should if a bug occurs and we IPI all the cpus, that we don't deadlock or block any printk messages.
> + > do { > seq = read_seqcount_begin(&tk_core.seq); > base = ktime_add(tk->tkr_mono.base, *offset); > @@ -721,6 +730,9 @@ ktime_t ktime_get_with_offset(enum tk_offsets offs) > > } while (read_seqcount_retry(&tk_core.seq, seq));
So instead of the do/while() loop above... Something closer to:
int __ktime_get_with_offset(enum tk_offsets offs, ktime_t* base, s64 *nsec) { unsigned int seq;
seq = read_seqcount_begin(&tk_core.seq); *base = ktime_add(tk->tkr_mono.base, *offset); *nsecs = timekeeping_get_ns(&tk->tkr_mono);
return read_seqcount_retry(&tk_core.seq, seq); }
Then ktime_get_with_offset() would just call: ... while(__ktime_get_with_offset(offs, &base, &nsecs)) /*spin*/; return ktime_add_ns(base,nsecs);
Then you add a simple: int ktime_try_get_with_offset(enum tk_offsets offs, ktime_t* ret) { ... if (__ktime_get_with_offset(offs, &base, &nsecs)) return -EAGAIN; *ret = ktime_add_ns(base,nsecs); return 0; }
thanks -john
| |