Message-ID: <55B66F83.7040105@hp.com>
Date: Mon, 27 Jul 2015 13:50:59 -0400
From: Waiman Long <waiman.long@hp.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12
MIME-Version: 1.0
To: Davidlohr Bueso <dave@stgolabs.net>
CC: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>, "H. Peter Anvin" <hpa@zytor.com>,
        x86@kernel.org, linux-kernel@vger.kernel.org,
        Scott J Norton <scott.norton@hp.com>,
        Douglas Hatch <doug.hatch@hp.com>
Subject: Re: [PATCH v3 1/7] locking/pvqspinlock: Unconditional PV kick with
 _Q_SLOW_VAL
References: <1437595962-21472-1-git-send-email-Waiman.Long@hp.com>  <1437595962-21472-2-git-send-email-Waiman.Long@hp.com>  <1437863509.3298.71.camel@stgolabs.net> <1437961615.25997.36.camel@stgolabs.net>
In-Reply-To: <1437961615.25997.36.camel@stgolabs.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1870
Lines: 40

On 07/26/2015 09:46 PM, Davidlohr Bueso wrote:
> On Sat, 2015-07-25 at 15:31 -0700, Davidlohr Bueso wrote:
>> On Wed, 2015-07-22 at 16:12 -0400, Waiman Long wrote:
>>> The smp_store_release() is not a full barrier. In order to avoid missed
>>> wakeup, we may need to add memory barrier around locked and cpu state
>>> variables adding to complexity. As the chance of spurious wakeup is very
>>> low, it is easier and safer to just do an unconditional kick at unlock
>>> time.
>> Although I guess if SPIN_THRESHOLD is ever enlarged, the chances of
>> spurious wakeups would be greater.
>>
>>> Signed-off-by: Waiman Long<Waiman.Long@hp.com>
>> Reviewed-by: Davidlohr Bueso<dave@stgolabs.net>
> Thinking about this some more, as good practice, could you please add a
> comment in the code explicitly mentioning the spurious wakeup side
> effect? Perhaps even having something more generic for the entire kernel
> might be added/created to Documentation/spurious-wakeups.txt?
>
> Thanks,
> Davidlohr
>

The if conditional check was added with the intention to save an 
unneeded PV kick when the vCPU was running. Doing an unconditional kick 
doesn't do any harm other than the additional latency of the PV kick. I 
will add a comment when I update the patch.

As for the spurious-wakeups.txt, I saw there are spurious wakeup 
occasionally. However, I am not totally sure of the mechanism that 
causes it. Also the spurious wakeup here refers to the vmenter of the 
vCPU which is different from spurious wakeup of a sleeping thread. I 
don't think I have enough data and information to write a document file yet.

Cheers,
Longman
--
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/