From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761502AbdADV5i convert rfc822-to-8bit (ORCPT ); Wed, 4 Jan 2017 16:57:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42912 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751732AbdADV5f (ORCPT ); Wed, 4 Jan 2017 16:57:35 -0500 Subject: Re: [RFC PATCH 6/7] locking/rtqspinlock: Voluntarily yield CPU when need_sched() To: Boqun Feng References: <1483466430-8028-1-git-send-email-longman@redhat.com> <1483466430-8028-7-git-send-email-longman@redhat.com> <20170104100720.GA4985@tardis.cn.ibm.com> Cc: Peter Zijlstra , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , linux-kernel@vger.kernel.org From: Waiman Long Organization: Red Hat Message-ID: Date: Wed, 4 Jan 2017 16:57:16 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170104100720.GA4985@tardis.cn.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8BIT X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 04 Jan 2017 21:57:17 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/04/2017 05:07 AM, Boqun Feng wrote: > On Tue, Jan 03, 2017 at 01:00:29PM -0500, Waiman Long wrote: >> Ideally we want the CPU to be preemptible even when inside or waiting >> for a lock. We cannot make it preemptible when inside a lock critical >> section, but we can try to make the task voluntarily yield the CPU >> when waiting for a lock. >> >> This patch checks the need_sched() flag and yields the CPU when the >> preemption count is 1. IOW, the spin_lock() call isn't done in a >> region that doesn't allow preemption. Otherwise, it will just perform >> RT spinning with a minimum priority of 1. >> >> Signed-off-by: Waiman Long >> --- >> kernel/locking/qspinlock_rt.h | 68 +++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 65 insertions(+), 3 deletions(-) >> >> diff --git a/kernel/locking/qspinlock_rt.h b/kernel/locking/qspinlock_rt.h >> index 0c4d051..18ec1f8 100644 >> --- a/kernel/locking/qspinlock_rt.h >> +++ b/kernel/locking/qspinlock_rt.h >> @@ -43,6 +43,16 @@ >> * it will have to break out of the MCS wait queue just like what is done >> * in the OSQ lock. Then it has to retry RT spinning if it has been boosted >> * to RT priority. >> + * >> + * Another RT requirement is that the CPU need to be preemptible even when >> + * waiting for a spinlock. If the task has already acquired the lock, we >> + * will let it run to completion to release the lock and reenable preemption. >> + * For non-nested spinlock, a spinlock waiter will periodically check >> + * need_resched flag to see if it should break out of the waiting loop and >> + * yield the CPU as long as the preemption count indicates just one >> + * preempt_disabled(). For nested spinlock with outer lock acquired, it will >> + * boost its priority to the highest RT priority level to try to acquire the >> + * inner lock, finish up its work, release the locks and reenable preemption. >> */ >> #include >> >> @@ -51,6 +61,15 @@ >> #endif >> >> /* >> + * Rescheduling is only needed when it is in the task context, the >> + * PREEMPT_NEED_RESCHED flag is set and the preemption count is one. >> + * If only the TIF_NEED_RESCHED flag is set, it will be moved to RT >> + * spinning with a minimum priority of 1. >> + */ >> +#define rt_should_resched() (preempt_count() == \ >> + (PREEMPT_OFFSET | PREEMPT_NEED_RESCHED)) >> + > Maybe I am missing something... but > > On x86, PREEMPT_NEED_RESCHED is used in an inverting style, i.e. 0 > indicates "need to reschedule" and preempt_count() masks away this very > bit, which makes rt_should_resched() always false. So... > > Regards, > Boqun You are right. I think I misunderstood what the preemption code is doing. I think I need to revise the code to fix that. Thanks for spotting this. Cheers, Longman