From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/2] x86: suppress event check IPI to MWAITing CPUs Date: Thu, 11 Sep 2014 11:09:51 +0100 Message-ID: <541174EF.4040506@citrix.com> References: <541189280200007800033BB5@mail.emea.novell.com> <54118A190200007800033BC9@mail.emea.novell.com> <5411734E.8010303@citrix.com> <5411908F0200007800033C1D@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XS1K8-0006cQ-Vq for xen-devel@lists.xenproject.org; Thu, 11 Sep 2014 10:09:57 +0000 In-Reply-To: <5411908F0200007800033C1D@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Ian Campbell , xen-devel , Keir Fraser , IanJackson , Tim Deegan List-Id: xen-devel@lists.xenproject.org On 11/09/14 11:07, Jan Beulich wrote: >>>> On 11.09.14 at 12:02, wrote: >> On 11/09/14 10:40, Jan Beulich wrote: >>> @@ -349,7 +359,7 @@ void mwait_idle_with_hints(unsigned int >>> * Timer deadline passing is the event on which we will be woken via >>> * cpuidle_mwait_wakeup. So check it now that the location is armed. >>> */ >>> - if ( expires > NOW() || expires == 0 ) >>> + if ( (expires > NOW() || expires == 0) && !softirq_pending(cpu) ) >>> { >>> cpumask_set_cpu(cpu, &cpuidle_mwait_flags); >> Don't you need a smp_wmb() or better here for the results of >> cpumask_set_cpu() to be guaranteed to be externally visible? mwait does >> not appear to be a serialising instruction, and doesn't appear to have >> any ordering guarantees in the manual. > I think cpumask_set_cpu() using a LOCKed r-m-w instruction should > provide all the needed ordering. > > Jan > Ah yes, and you haven't gotten around to changing the cpumask_* functions wrt atomicity yet. Reviewed-by: Andrew Cooper