From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752986AbbJOUos (ORCPT ); Thu, 15 Oct 2015 16:44:48 -0400 Received: from g2t4622.austin.hp.com ([15.73.212.79]:41986 "EHLO g2t4622.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751268AbbJOUoq (ORCPT ); Thu, 15 Oct 2015 16:44:46 -0400 Message-ID: <5620103B.2050002@hpe.com> Date: Thu, 15 Oct 2015 16:44:43 -0400 From: Waiman Long User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12 MIME-Version: 1.0 To: Peter Zijlstra CC: Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Scott J Norton , Douglas Hatch , Davidlohr Bueso Subject: Re: [PATCH v7 4/5] locking/pvqspinlock: Allow 1 lock stealing attempt References: <1442955044-43895-1-git-send-email-Waiman.Long@hpe.com> <1442955044-43895-5-git-send-email-Waiman.Long@hpe.com> <20151013182338.GW3816@twins.programming.kicks-ass.net> <561D6C85.9030508@hpe.com> <20151013204421.GN17308@twins.programming.kicks-ass.net> In-Reply-To: <20151013204421.GN17308@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/13/2015 04:44 PM, Peter Zijlstra wrote: > On Tue, Oct 13, 2015 at 04:41:41PM -0400, Waiman Long wrote: >> On 10/13/2015 02:23 PM, Peter Zijlstra wrote: >>> On Tue, Sep 22, 2015 at 04:50:43PM -0400, Waiman Long wrote: >>>> for (;; waitcnt++) { >>>> + loop = SPIN_THRESHOLD; >>>> + while (loop) { >>>> + /* >>>> + * Spin until the lock is free >>>> + */ >>>> + for (; loop&& READ_ONCE(l->locked); loop--) >>>> + cpu_relax(); >>>> + /* >>>> + * Seeing the lock is free, this queue head vCPU is >>>> + * the rightful next owner of the lock. However, the >>>> + * lock may have just been stolen by another task which >>>> + * has entered the slowpath. So we need to use atomic >>>> + * operation to make sure that we really get the lock. >>>> + * Otherwise, we have to wait again. >>>> + */ >>>> + if (cmpxchg(&l->locked, 0, _Q_LOCKED_VAL) == 0) >>>> + goto gotlock; >>>> } >>> for (loop = SPIN_THRESHOLD; loop; --loop) { >>> if (!READ_ONCE(l->locked)&& >>> cmpxchg(&l->locked, 0, _Q_LOCKED_VA) == 0) >>> goto gotlock; >>> >>> cpu_relax(); >>> } >>> >> This was the code that I used in my original patch, but it seems to confuse >> you about doing too many lock stealing. So I separated it out to make my >> intention more explicit. I will change it back to the old code. > Code should be compact; its the purpose of Changelogs and comments to > explain it if its subtle. > > Here you made weird code and the comments still don't explain how its > starvation proof and the Changelog is almost empty of useful. You are right. The current patch can't guarantee that there will be no lock starvation. I will make some code changes to make sure that lock starvation won't happen. I will also try to clean up the code and the comments at the same time. Cheers, Longman