From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 4/5] xen: use masking operation instead of test_bit for VPF bits Date: Mon, 5 Oct 2015 14:45:40 +0100 Message-ID: <56127F04.4060300@citrix.com> References: <1443760830-29095-1-git-send-email-jgross@suse.com> <1443760830-29095-5-git-send-email-jgross@suse.com> <561278C2.3010107@citrix.com> <5612990402000078000A8230@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5612990402000078000A8230@prv-mh.provo.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: Juergen Gross , keir@xen.org, george.dunlap@eu.citrix.com, andrew.cooper3@citrix.com, dario.faggioli@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/10/15 14:36, Jan Beulich wrote: >>>> On 05.10.15 at 15:18, wrote: >> On 02/10/15 05:40, Juergen Gross wrote: >>> Use a bit mask for testing of a set bit instead of test_bit in case no >>> atomic operation is needed, as this will lead to smaller and more >>> effective code. >>> >>> Signed-off-by: Juergen Gross >> >> I'm a bit confused here -- exactly when is an atomic operation needed or >> not needed? Isn't it the case that we always need to do an atomic read >> if we ever do an atomic write without a lock held? > > First of all - what is an atomic read from CPU perspective other than > just a read? Since we talk about individual bits here, we don't care > about the granularity the compiler may convert the read to, and even > if the compiler chose to do multiple reads the result would still be > correct, as only one of those reads can possibly have read the bit > in question. > > And then, the old mechanism was in no way "atomic", all it added was > a kind of compiler barrier (due to the cast to volatile). Yet in none of > the cases changed I was able to spot a need for such a barrier. OK, so the key thing about test_bit isn't that it's atomic, so much that it's an implicit memory barrier. So as long as you're not doing a lockless careful-ordering sort of thing, then a simple memory read should be fine. Is that correct? In that case, it's likely that the patch is correct (though I'll take a closer look just to be sure). -George