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 15:31:23 +0100 Message-ID: <561289BB.6040705@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> <56127F04.4060300@citrix.com> <56129FC602000078000A8290@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: <56129FC602000078000A8290@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 15:05, Jan Beulich wrote: >>>> On 05.10.15 at 15:45, wrote: >> 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? > > Yes. > >> In that case, it's likely that the patch is correct (though I'll take a >> closer look just to be sure). > > Thanks. OK, I've looked through again and don't see anything that looks racy: Reviewed-by: George Dunlap