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 Content-Length: 2858 Lines: 75 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/