From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5377BC433E1 for ; Thu, 16 Jul 2020 10:06:56 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2870D2074B for ; Thu, 16 Jul 2020 10:06:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2870D2074B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jw0mN-00043P-Hq; Thu, 16 Jul 2020 10:06:15 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jw0mM-00043K-Gh for xen-devel@lists.xenproject.org; Thu, 16 Jul 2020 10:06:14 +0000 X-Inumbo-ID: fa918f08-c74b-11ea-b7bb-bc764e2007e4 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id fa918f08-c74b-11ea-b7bb-bc764e2007e4; Thu, 16 Jul 2020 10:06:13 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3D9B3B69F; Thu, 16 Jul 2020 10:06:16 +0000 (UTC) Subject: Re: [PATCH v3 1/2] x86: restore pv_rtc_handler() invocation To: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= References: <5426dd6f-50cd-dc23-5c6b-0ab631d98d38@suse.com> <7dd4b668-06ca-807a-9cc1-77430b2376a8@suse.com> <20200715121347.GY7191@Air-de-Roger> <2b9de0fd-5973-ed66-868c-ffadca83edf3@suse.com> <20200715133217.GZ7191@Air-de-Roger> <20200715145144.GA7191@Air-de-Roger> From: Jan Beulich Message-ID: Date: Thu, 16 Jul 2020 12:06:14 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20200715145144.GA7191@Air-de-Roger> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: "xen-devel@lists.xenproject.org" , Paul Durrant , Wei Liu , Andrew Cooper Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" On 15.07.2020 16:51, Roger Pau Monné wrote: > On Wed, Jul 15, 2020 at 03:51:17PM +0200, Jan Beulich wrote: >> On 15.07.2020 15:32, Roger Pau Monné wrote: >>> Feel free to change to ACCESS_ONCE or barrier if you think it's >>> clearer. >> >> I did so (also on the writer side), not the least based on guessing >> what Andrew would presumably have preferred. > > Thanks! Sorry I might be pedantic, but is the ACCESS_ONCE on the write > side actually required? I'm not sure I see what ACCESS_ONCE protects > against in handle_rtc_once. Well, this is all sort of a mess, I think. We have this mixture of ACCESS_ONCE() and read_atomic() / write_atomic(), but I don't think we use them consistently, and I'm not sure either is suitable to deal with all (theoretical) corner cases. read_atomic() / write_atomic() guarantee a single insn to be used to access a piece of data. I'm uncertain whether they also guarantee single access (i.e. that the compiler won't replicate the asm()-s). The wording in gcc doc is pretty precise, but not quite enough imo to be entirely certain. ACCESS_ONCE() guarantees single access, but doesn't guarantee that the compiler wouldn't split this single access into multiple insns. (It's just, like elsewhere, that it would be pretty silly of it if it did.) Yesterday, as said, I tried to in particular do what I expect/guess Andrew would have wanted done. This is despite me not being entirely convinced this is the right thing to do here, i.e. personally I would have preferred read_atomic() / write_atomic(), as I think the intention of what the gcc doc is saying is what we want (taking into consideration both uses of "volatile" in these helpers). Jan