From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH RFC 3/4] barriers: convert a control to a data dependency Date: Mon, 7 Jan 2019 08:36:36 -0500 Message-ID: <20190107082223-mutt-send-email-mst@kernel.org> References: <20190102205715.14054-1-mst@redhat.com> <20190102205715.14054-4-mst@redhat.com> <86023cbe-d1ae-a0d6-7b75-26556f1a0c1f@redhat.com> <20190106231756-mutt-send-email-mst@kernel.org> <20190107094610.GA2861@worktop.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20190107094610.GA2861@worktop.programming.kicks-ass.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Peter Zijlstra Cc: Andrea Parri , linux-doc@vger.kernel.org, Akira Yokosawa , Will Deacon , virtualization@lists.linux-foundation.org, David Howells , linux-arch@vger.kernel.org, Jonathan Corbet , linux-sparse@vger.kernel.org, Alan Stern , Matt Turner , "Paul E. McKenney" , Boqun Feng , Arnd Bergmann , Daniel Lustig , Nicholas Piggin , Ivan Kokshaysky , Luc Maranget , Richard Henderson , Jade Alglave , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org, Luc Van Oostenryck List-Id: linux-arch.vger.kernel.org T24gTW9uLCBKYW4gMDcsIDIwMTkgYXQgMTA6NDY6MTBBTSArMDEwMCwgUGV0ZXIgWmlqbHN0cmEg d3JvdGU6Cj4gT24gU3VuLCBKYW4gMDYsIDIwMTkgYXQgMTE6MjM6MDdQTSAtMDUwMCwgTWljaGFl bCBTLiBUc2lya2luIHdyb3RlOgo+ID4gT24gTW9uLCBKYW4gMDcsIDIwMTkgYXQgMTE6NTg6MjNB TSArMDgwMCwgSmFzb24gV2FuZyB3cm90ZToKPiA+ID4gT24gMjAxOS8xLzMg5LiK5Y2INDo1Nywg TWljaGFlbCBTLiBUc2lya2luIHdyb3RlOgo+IAo+ID4gPiA+ICsjaWYgZGVmaW5lZChDT01QSUxF Ul9IQVNfT1BUSU1JWkVSX0hJREVfVkFSKSAmJiBcCj4gPiA+ID4gKwkhZGVmaW5lZChBUkNIX05F RURTX1JFQURfQkFSUklFUl9ERVBFTkRTKQo+ID4gPiA+ICsKPiA+ID4gPiArI2RlZmluZSBkZXBl bmRlbnRfcHRyX21iKHB0ciwgdmFsKSAoewkJCQkJXAo+ID4gPiA+ICsJbG9uZyBkZXBlbmRlbnRf cHRyX21iX3ZhbCA9IChsb25nKSh2YWwpOwkJCVwKPiA+ID4gPiArCWxvbmcgZGVwZW5kZW50X3B0 cl9tYl9wdHIgPSAobG9uZykocHRyKSAtIGRlcGVuZGVudF9wdHJfbWJfdmFsOwlcCj4gPiA+ID4g KwkJCQkJCQkJCVwKPiA+ID4gPiArCUJVSUxEX0JVR19PTihzaXplb2YodmFsKSA+IHNpemVvZihs b25nKSk7CQkJXAo+ID4gPiA+ICsJT1BUSU1JWkVSX0hJREVfVkFSKGRlcGVuZGVudF9wdHJfbWJf dmFsKTsJCQlcCj4gPiA+ID4gKwkodHlwZW9mKHB0cikpKGRlcGVuZGVudF9wdHJfbWJfcHRyICsg ZGVwZW5kZW50X3B0cl9tYl92YWwpOwlcCj4gPiA+ID4gK30pCj4gPiA+ID4gKwo+ID4gPiA+ICsj ZWxzZQo+ID4gPiA+ICsKPiA+ID4gPiArI2RlZmluZSBkZXBlbmRlbnRfcHRyX21iKHB0ciwgdmFs KSAoeyBtYigpOyAocHRyKTsgfSkKPiA+ID4gCj4gPiA+IAo+ID4gPiBTbyBmb3IgdGhlIGV4YW1w bGUgb2YgcGF0Y2ggNCwgd2UnZCBiZXR0ZXIgZmFsbCBiYWNrIHRvIHJtYigpIG9yIG5lZWQgYQo+ ID4gPiBkZXBlbmRlbnRfcHRyX3JtYigpPwo+ID4gPiAKPiA+ID4gVGhhbmtzCj4gPiAKPiA+IFlv dSBtZWFuIGZvciBzdHJvbmdseSBvcmRlcmVkIGFyY2hpdGVjdHVyZXMgbGlrZSBJbnRlbD8KPiA+ IFllcywgbWF5YmUgaXQgbWFrZXMgc2Vuc2UgdG8gaGF2ZSBkZXBlbmRlbnRfcHRyX3NtcF9ybWIs Cj4gPiBkZXBlbmRlbnRfcHRyX2RtYV9ybWIgYW5kIGRlcGVuZGVudF9wdHJfdmlydF9ybWIuCj4g PiAKPiA+IG1iIHZhcmlhbnQgaXMgdW51c2VkIHJpZ2h0IG5vdyBzbyBJJ2xsIHJlbW92ZSBpdC4K PiAKPiBIb3cgYWJvdXQgbmFtaW5nIHRoZSB0aGluZzogZGVwZW5kZW50X3B0cigpID8gVGhhdCBp cyB3aXRob3V0IGFueSAociltYgo+IGltcGxpY2F0aW9ucyBhdCBhbGwuIFRoZSBhZGRyZXNzIGRl cGVuZGVuY3kgaXMgc3RyaWN0bHkgd2Vha2VyIHRoYW4gYW4KPiBybWIgaW4gdGhhdCBpdCB3aWxs IG9ubHkgb3JkZXIgdGhlIHR3byBsb2FkcyBpbiBxZXN0aW9uIGFuZCBub3QsIGxpa2UKPiBybWIs IGFueSBwcmlvciB0byBhbnkgbGF0ZXIgbG9hZC4KClNvIEknbSBmaW5lIHdpdGggdGhpcyBhcyBp dCdzIGVub3VnaCBmb3IgdmlydGlvLCBidXQgSSB3b3VsZCBsaWtlIHRvIHBvaW50IG91dCB0d28g dGhpbmdzOgoKMS4gRS5nLiBvbiB4ODYgYm90aCBTTVAgYW5kIERNQSB2YXJpYW50cyBjYW4gYmUg Tk9QcyBidXQKdGhlIG1hZGF0b3J5IG9uZSBjYW4ndCwgc28gYXNzdW1pbmcgd2UgZG8gbm90IHdh bnQKaXQgdG8gYmUgc3Ryb25nZXIgdGhhbiBybXAgdGhlbiBlaXRoZXIgd2Ugd2FudApzbXBfZGVw ZW5kZW50X3B0cigpLCBkbWFfZGVwZW5kZW50X3B0cigpLCBkZXBlbmRlbnRfcHRyKCkKb3Igd2Ug anVzdCB3aWxsIHNwZWNpZnkgdGhhdCBkZXBlbmRlbnRfcHRyKCkgd29ya3MgZm9yCmJvdGggRE1B IGFuZCBTTVAuCgoyLiBEb3duIHRoZSByb2FkLCBzb21lb25lIG1pZ2h0IHdhbnQgdG8gb3JkZXIg YSBzdG9yZSBhZnRlciBhIGxvYWQuCkFkZHJlc3MgZGVwZW5kZW5jeSBkb2VzIHRoYXQgZm9yIHVz IHRvby4gQXNzdW1pbmcgd2UgbWFrZQpkZXBlbmRlbnRfcHRyIGEgTk9QIG9uIHg4Niwgd2Ugd2ls bCB3YW50IGFuIG1iIHZhcmlhbnQKd2hpY2ggaXNuJ3QgYSBOT1Agb24geDg2LiBXaWxsIHdlIHdh bnQgdG8gcmVuYW1lCmRlcGVuZGVudF9wdHIgdG8gZGVwZW5kZW50X3B0cl9ybWIgYXQgdGhhdCBw b2ludD8KClRoYW5rcywKCi0tIApNU1QKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KVmlydHVhbGl6YXRpb24gbWFpbGluZyBsaXN0ClZpcnR1YWxpemF0aW9u QGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnCmh0dHBzOi8vbGlzdHMubGludXhmb3VuZGF0aW9u Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL3ZpcnR1YWxpemF0aW9u From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:35098 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726886AbfAGNgo (ORCPT ); Mon, 7 Jan 2019 08:36:44 -0500 Date: Mon, 7 Jan 2019 08:36:36 -0500 From: "Michael S. Tsirkin" Subject: Re: [PATCH RFC 3/4] barriers: convert a control to a data dependency Message-ID: <20190107082223-mutt-send-email-mst@kernel.org> References: <20190102205715.14054-1-mst@redhat.com> <20190102205715.14054-4-mst@redhat.com> <86023cbe-d1ae-a0d6-7b75-26556f1a0c1f@redhat.com> <20190106231756-mutt-send-email-mst@kernel.org> <20190107094610.GA2861@worktop.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190107094610.GA2861@worktop.programming.kicks-ass.net> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Peter Zijlstra Cc: Jason Wang , linux-kernel@vger.kernel.org, Alan Stern , Andrea Parri , Will Deacon , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , linux-arch@vger.kernel.org, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, Jonathan Corbet , Richard Henderson , Ivan Kokshaysky , Matt Turner , Arnd Bergmann , Luc Van Oostenryck , linux-doc@vger.kernel.org, linux-alpha@vger.kernel.org, linux-sparse@vger.kernel.org Message-ID: <20190107133636.0jRyXF2Hhz5JRHZWXOcT0NbdCm12q4oQ9_hzshJB_io@z> On Mon, Jan 07, 2019 at 10:46:10AM +0100, Peter Zijlstra wrote: > On Sun, Jan 06, 2019 at 11:23:07PM -0500, Michael S. Tsirkin wrote: > > On Mon, Jan 07, 2019 at 11:58:23AM +0800, Jason Wang wrote: > > > On 2019/1/3 上午4:57, Michael S. Tsirkin wrote: > > > > > +#if defined(COMPILER_HAS_OPTIMIZER_HIDE_VAR) && \ > > > > + !defined(ARCH_NEEDS_READ_BARRIER_DEPENDS) > > > > + > > > > +#define dependent_ptr_mb(ptr, val) ({ \ > > > > + long dependent_ptr_mb_val = (long)(val); \ > > > > + long dependent_ptr_mb_ptr = (long)(ptr) - dependent_ptr_mb_val; \ > > > > + \ > > > > + BUILD_BUG_ON(sizeof(val) > sizeof(long)); \ > > > > + OPTIMIZER_HIDE_VAR(dependent_ptr_mb_val); \ > > > > + (typeof(ptr))(dependent_ptr_mb_ptr + dependent_ptr_mb_val); \ > > > > +}) > > > > + > > > > +#else > > > > + > > > > +#define dependent_ptr_mb(ptr, val) ({ mb(); (ptr); }) > > > > > > > > > So for the example of patch 4, we'd better fall back to rmb() or need a > > > dependent_ptr_rmb()? > > > > > > Thanks > > > > You mean for strongly ordered architectures like Intel? > > Yes, maybe it makes sense to have dependent_ptr_smp_rmb, > > dependent_ptr_dma_rmb and dependent_ptr_virt_rmb. > > > > mb variant is unused right now so I'll remove it. > > How about naming the thing: dependent_ptr() ? That is without any (r)mb > implications at all. The address dependency is strictly weaker than an > rmb in that it will only order the two loads in qestion and not, like > rmb, any prior to any later load. So I'm fine with this as it's enough for virtio, but I would like to point out two things: 1. E.g. on x86 both SMP and DMA variants can be NOPs but the madatory one can't, so assuming we do not want it to be stronger than rmp then either we want smp_dependent_ptr(), dma_dependent_ptr(), dependent_ptr() or we just will specify that dependent_ptr() works for both DMA and SMP. 2. Down the road, someone might want to order a store after a load. Address dependency does that for us too. Assuming we make dependent_ptr a NOP on x86, we will want an mb variant which isn't a NOP on x86. Will we want to rename dependent_ptr to dependent_ptr_rmb at that point? Thanks, -- MST