From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751981AbbFEJ2E (ORCPT ); Fri, 5 Jun 2015 05:28:04 -0400 Received: from forward18h.cmail.yandex.net ([87.250.230.160]:57858 "EHLO forward18h.cmail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751149AbbFEJ2A (ORCPT ); Fri, 5 Jun 2015 05:28:00 -0400 From: Kirill Tkhai To: Peter Zijlstra Cc: "umgwanakikbuti@gmail.com" , "mingo@elte.hu" , "ktkhai@parallels.com" , "rostedt@goodmis.org" , "tglx@linutronix.de" , "juri.lelli@gmail.com" , "pang.xunlei@linaro.org" , "oleg@redhat.com" , "wanpeng.li@linux.intel.com" , "linux-kernel@vger.kernel.org" In-Reply-To: <20150605091027.GE19282@twins.programming.kicks-ass.net> References: <20150603132903.203333087@infradead.org> <20150603134023.156059118@infradead.org> <214021433348760@web25g.yandex.ru> <20150603211324.GC3644@twins.programming.kicks-ass.net> <2134411433408823@web8j.yandex.ru> <20150604104902.GH3644@twins.programming.kicks-ass.net> <5883311433494921@web27h.yandex.ru> <20150605091027.GE19282@twins.programming.kicks-ass.net> Subject: Re: [PATCH 8/9] hrtimer: Allow hrtimer::function() to free the timer MIME-Version: 1.0 Message-Id: <6034121433496470@web27h.yandex.ru> X-Mailer: Yamail [ http://yandex.ru ] 5.0 Date: Fri, 05 Jun 2015 12:27:50 +0300 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=koi8-r Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 05.06.2015, 12:10, "Peter Zijlstra" : > On Fri, Jun 05, 2015 at 12:02:01PM +0300, Kirill Tkhai wrote: >> šYeah, it's safe for now, but it may happen difficulties with a support >> šin the future, because barrier logic is not easy to review. But it seems >> šwe may simplify it a little bit. Please, see the comments below. >>> š@@ -394,8 +399,24 @@ extern u64 hrtimer_get_next_event(void); >>> ššš*/ >>> ššstatic inline int hrtimer_active(const struct hrtimer *timer) >>> šš{ >>> š+ struct hrtimer_cpu_base *cpu_base; >>> š+ unsigned int seq; >>> š+ bool active; >>> š+ >>> š+ do { >>> š+ active = false; >>> š+ cpu_base = READ_ONCE(timer->base->cpu_base); >>> š+ seqcount_lockdep_reader_access(&cpu_base->seq); >>> š+ seq = raw_read_seqcount(&cpu_base->seq); >>> š+ >>> š+ if (timer->state != HRTIMER_STATE_INACTIVE || >>> š+ šššcpu_base->running == timer) >>> š+ active = true; >>> š+ >>> š+ } while (read_seqcount_retry(&cpu_base->seq, seq) || >>> š+ cpu_base != READ_ONCE(timer->base->cpu_base)); >>> š+ >>> š+ return active; >>> šš} >> šThis may race with migrate_hrtimer_list(), so it needs write seqcounter >> štoo. > > Let me go stare at that. >> šMy suggestion is do not use seqcounters for long parts of code, and implement >> šshort primitives for changing timer state and cpu_base running timer. Something >> šlike this: >> >> šstatic inline void hrtimer_set_state(struct hrtimer *timer, unsigned long state) >> š{ >> šššššššššstruct hrtimer_cpu_base *cpu_base = timer->base->cpu_base; >> >> šššššššššlockdep_assert_held(&cpu_base->lock); >> >> šššššššššwrite_seqcount_begin(&cpu_base->seq); >> ššššššššštimer->state = state; >> šššššššššwrite_seqcount_end(&cpu_base->seq); >> š} >> >> šstatic inline void cpu_base_set_running(struct hrtimer_cpu_base *cpu_base, >> šššššššššššššššššššššššššššššššššššššššššstruct hrtimer *timer) >> š{ >> šššššššššlockdep_assert_held(&cpu_base->lock); >> >> šššššššššwrite_seqcount_begin(&cpu_base->seq); >> šššššššššcpu_base->running = timer; >> šššššššššwrite_seqcount_end(&cpu_base->seq); >> š} >> >> šImplemented this, we may less think about right barrier order, because >> šall changes are being made under seqcount. > > The problem with that is that it generates far more memory barriers, > while on x86 these are 'free', this is very much not so for other archs > like ARM / PPC etc. Ok, thanks. One more way is to take write_seqcount every time we're taking base spin_lock, thus we may group several smp_wmb() in a single. But this increases write seqlocked code areas, and worsens the speed of hrtimer_active(), so it's not good too.