From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751529AbcHLAb2 (ORCPT ); Thu, 11 Aug 2016 20:31:28 -0400 Received: from mga11.intel.com ([192.55.52.93]:14613 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbcHLAb1 (ORCPT ); Thu, 11 Aug 2016 20:31:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,507,1464678000"; d="scan'208";a="154537776" Subject: Re: [RESEND PATCH v4] x86/hpet: Reduce HPET counter read contention To: Waiman Long References: <1470853770-37625-1-git-send-email-Waiman.Long@hpe.com> <57ACD2DE.6080306@intel.com> <57AD0898.7030506@hpe.com> Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , linux-kernel@vger.kernel.org, x86@kernel.org, Jiang Liu , Borislav Petkov , Andy Lutomirski , Prarit Bhargava , Scott J Norton , Douglas Hatch , Randy Wright , John Stultz From: Dave Hansen Message-ID: <57AD18D1.1050107@intel.com> Date: Thu, 11 Aug 2016 17:31:13 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <57AD0898.7030506@hpe.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/11/2016 04:22 PM, Waiman Long wrote: > On 08/11/2016 03:32 PM, Dave Hansen wrote: >> It's a real bummer that this all has to be open-coded. I have to wonder >> if there were any alternatives that you tried that were simpler. > > What do you mean by "open-coded"? Do you mean the function can be inlined? I just mean that it's implementing its own locking instead of being able to use spinlocks or seqlocks, or some other existing primitive. >> Is READ_ONCE()/smp_store_release() really strong enough here? It >> guarantees ordering, but you need ordering *and* a guarantee that your >> write is visible to the reader. Don't you need actual barriers for >> that? Otherwise, you might be seeing a stale HPET value, and the spin >> loop that you did waiting for it to be up-to-date was worthless. The >> seqlock code, uses barriers, btw. > > The cmpxchg() and smp_store_release() act as the lock/unlock sequence > with the proper barriers. Another important point is that the hpet value > is visible to the other readers before the sequence number. This is > what the smp_store_release() is providing. cmpxchg is an actual barrier, > even though smp_store_release() is not. However, the x86 architecture > will guarantee the writes are in order, I think. The contended case (where HPET_SEQ_LOCKED(seq)) doesn't do the cmpxchg. So it's entirely relying on the READ_ONCE() on the "reader" side and the cmpxchg/smp_store_release() on the "writer". This probably works in practice, but I'm not sure it's guaranteed behavior.