From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752650AbbE0JTg (ORCPT ); Wed, 27 May 2015 05:19:36 -0400 Received: from forward-corp1f.mail.yandex.net ([95.108.130.40]:48457 "EHLO forward-corp1f.mail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752064AbbE0JTe (ORCPT ); Wed, 27 May 2015 05:19:34 -0400 Authentication-Results: smtpcorp1m.mail.yandex.net; dkim=pass header.i=@yandex-team.ru Message-ID: <55658C21.7040102@yandex-team.ru> Date: Wed, 27 May 2015 12:19:29 +0300 From: Konstantin Khlebnikov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Peter Zijlstra CC: Ingo Molnar , linux-kernel@vger.kernel.org, x86@kernel.org, Linus Torvalds , Oleg Nesterov , Steven Rostedt Subject: Re: [PATCH RESEND] sched/preempt: fix cond_resched_lock() and cond_resched_softirq() References: <20150514162044.27876.20374.stgit@buzz> <20150527090922.GW3644@twins.programming.kicks-ass.net> In-Reply-To: <20150527090922.GW3644@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27.05.2015 12:09, Peter Zijlstra wrote: > On Thu, May 14, 2015 at 07:23:27PM +0300, Konstantin Khlebnikov wrote: >> These functions check should_resched() before unlocking spinlock/bh-enable: >> preempt_count always non-zero => should_resched() always returns false. >> cond_resched_lock() works iff spin_needbreak is set. >> >> This patch adds argument "preempt_offset" to should_resched() add >> rearranges preempt_count offset constants for that: >> >> PREEMPT_OFFSET - offset after preempt_disable() (0 if CONFIG_PREEMPT_COUNT=n) >> PREEMPT_LOCK_OFFSET - offset after spin_lock() (alias for PREEMPT_OFFSET) >> SOFTIRQ_DISABLE_OFFSET - offset after local_bh_distable() >> SOFTIRQ_LOCK_OFFSET - offset after spin_lock_bh() >> >> Signed-off-by: Konstantin Khlebnikov > > Sorry, but it doesn't apply anymore because of that whole > pagefault_disable() muck we merged. No problem. I'll dig into this stuff again later. This bug is harmless and it's here for ages. > >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >> index 48d3c5d2ecc9..6e73b74c0c60 100644 >> --- a/arch/powerpc/kvm/book3s_hv.c >> +++ b/arch/powerpc/kvm/book3s_hv.c >> @@ -2177,7 +2177,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) >> vc->runner = vcpu; >> if (n_ceded == vc->n_runnable) { >> kvmppc_vcore_blocked(vc); >> - } else if (should_resched()) { >> + } else if (should_resched(PREEMPT_LOCK_OFFSET)) { > > I'm thinking this wants to be: need_resched() ? > >> vc->vcore_state = VCORE_PREEMPT; >> /* Let something else run */ >> cond_resched_lock(&vc->lock); > >> diff --git a/include/asm-generic/preempt.h b/include/asm-generic/preempt.h >> index eb6f9e6c3075..e91fb799a6da 100644 >> --- a/include/asm-generic/preempt.h >> +++ b/include/asm-generic/preempt.h >> @@ -71,9 +71,9 @@ static __always_inline bool __preempt_count_dec_and_test(void) >> /* >> * Returns true when we need to resched and can (barring IRQ state). >> */ >> -static __always_inline bool should_resched(void) >> +static __always_inline bool should_resched(int offset) >> { >> - return unlikely(!preempt_count() && tif_need_resched()); >> + return unlikely(preempt_count() == offset && tif_need_resched()); >> } >> >> #ifdef CONFIG_PREEMPT > > So the reason I held off on this patch for a wee bit is because I don't > like the should_resched() change you did; although I fully understand > why you did it. > > That said, I could not come up with anything better either and I suppose > that once we fix that ppc-kvm user, there really isn't a user left > outside of core code and thus we can deal with a slightly dangerous > function. > > I did not really look, but it would be good if we could also get rid of > the Xen usage. > -- Konstantin