From: "Michael S. Tsirkin" <mst@redhat.com> To: Peter Zijlstra <peterz@infradead.org> Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Jason Wang <jasowang@redhat.com>, Rusty Russell <rusty@rustcorp.com.au>, qemu-devel@nongnu.org, Alexander Duyck <alexander.duyck@gmail.com>, linux-kernel@vger.kernel.org, David Vrabel <david.vrabel@citrix.com>, xen-devel@lists.xenproject.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>, virtualization@lists.linux-foundation.org, Arnd Bergmann <arnd@arndb.de>, linux-arch@vger.kernel.org Subject: Re: [Xen-devel] new barrier type for paravirt (was Re: [PATCH] virtio_ring: use smp_store_mb) Date: Mon, 21 Dec 2015 09:10:52 +0200 [thread overview] Message-ID: <20151221085947-mutt-send-email-mst@redhat.com> (raw) In-Reply-To: <20151220195944.GT6344@twins.programming.kicks-ass.net> On Sun, Dec 20, 2015 at 08:59:44PM +0100, Peter Zijlstra wrote: > On Sun, Dec 20, 2015 at 05:07:19PM +0000, Andrew Cooper wrote: > > > > Very much +1 for fixing this. > > > > Those names would be fine, but they do add yet another set of options in > > an already-complicated area. > > > > An alternative might be to have the regular smp_{w,r,}mb() not revert > > back to nops if CONFIG_PARAVIRT, or perhaps if pvops have detected a > > non-native environment. (I don't know how feasible this suggestion is, > > however.) > > So a regular SMP kernel emits the LOCK prefix and will patch it out with > a DS prefix (iirc) when it finds but a single CPU. So for those you > could easily do this. > > However an UP kernel will not emit the LOCK and do no patching. > > So if you're willing to make CONFIG_PARAVIRT depend on CONFIG_SMP or > similar, this is doable. One of the uses for virtio is to allow testing an existing kernel on kvm just by loading a module, and this will break this usecase. > I don't see people going to allow emitting the LOCK prefix (and growing > the kernel text size) for UP kernels. Thinking about this more, maybe __smp_*mb is a better set of names. The nice thing about it is that we can then have generic code that does basically #ifdef CONFIG_SMP #define smp_mb() __smp_mb() #else #define smp_mb() barrier() #endif and reuse this on all architectures. So instead of a maintainance burden, we are actually removing code duplication. -- MST
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com> To: Peter Zijlstra <peterz@infradead.org> Cc: linux-arch@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>, Andrew Cooper <andrew.cooper3@citrix.com>, Jason Wang <jasowang@redhat.com>, Rusty Russell <rusty@rustcorp.com.au>, qemu-devel@nongnu.org, Alexander Duyck <alexander.duyck@gmail.com>, linux-kernel@vger.kernel.org, David Vrabel <david.vrabel@citrix.com>, xen-devel@lists.xenproject.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>, virtualization@lists.linux-foundation.org Subject: Re: [Qemu-devel] [Xen-devel] new barrier type for paravirt (was Re: [PATCH] virtio_ring: use smp_store_mb) Date: Mon, 21 Dec 2015 09:10:52 +0200 [thread overview] Message-ID: <20151221085947-mutt-send-email-mst@redhat.com> (raw) In-Reply-To: <20151220195944.GT6344@twins.programming.kicks-ass.net> On Sun, Dec 20, 2015 at 08:59:44PM +0100, Peter Zijlstra wrote: > On Sun, Dec 20, 2015 at 05:07:19PM +0000, Andrew Cooper wrote: > > > > Very much +1 for fixing this. > > > > Those names would be fine, but they do add yet another set of options in > > an already-complicated area. > > > > An alternative might be to have the regular smp_{w,r,}mb() not revert > > back to nops if CONFIG_PARAVIRT, or perhaps if pvops have detected a > > non-native environment. (I don't know how feasible this suggestion is, > > however.) > > So a regular SMP kernel emits the LOCK prefix and will patch it out with > a DS prefix (iirc) when it finds but a single CPU. So for those you > could easily do this. > > However an UP kernel will not emit the LOCK and do no patching. > > So if you're willing to make CONFIG_PARAVIRT depend on CONFIG_SMP or > similar, this is doable. One of the uses for virtio is to allow testing an existing kernel on kvm just by loading a module, and this will break this usecase. > I don't see people going to allow emitting the LOCK prefix (and growing > the kernel text size) for UP kernels. Thinking about this more, maybe __smp_*mb is a better set of names. The nice thing about it is that we can then have generic code that does basically #ifdef CONFIG_SMP #define smp_mb() __smp_mb() #else #define smp_mb() barrier() #endif and reuse this on all architectures. So instead of a maintainance burden, we are actually removing code duplication. -- MST
next prev parent reply other threads:[~2015-12-21 7:11 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-12-17 10:32 [PATCH] virtio_ring: use smp_store_mb Michael S. Tsirkin 2015-12-17 10:32 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-17 10:52 ` Peter Zijlstra 2015-12-17 10:52 ` Peter Zijlstra 2015-12-17 10:52 ` [Qemu-devel] " Peter Zijlstra 2015-12-17 13:16 ` Michael S. Tsirkin 2015-12-17 13:16 ` Michael S. Tsirkin 2015-12-17 13:16 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-17 13:57 ` Peter Zijlstra 2015-12-17 13:57 ` Peter Zijlstra 2015-12-17 13:57 ` [Qemu-devel] " Peter Zijlstra 2015-12-17 14:33 ` Michael S. Tsirkin 2015-12-17 14:33 ` Michael S. Tsirkin 2015-12-17 14:33 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-17 14:39 ` Peter Zijlstra 2015-12-17 14:39 ` Peter Zijlstra 2015-12-17 14:39 ` [Qemu-devel] " Peter Zijlstra 2015-12-17 14:43 ` Michael S. Tsirkin 2015-12-17 14:43 ` Michael S. Tsirkin 2015-12-17 14:43 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-20 9:25 ` new barrier type for paravirt (was Re: [PATCH] virtio_ring: use smp_store_mb) Michael S. Tsirkin 2015-12-20 9:25 ` Michael S. Tsirkin 2015-12-20 9:25 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-20 17:07 ` Andrew Cooper 2015-12-20 17:07 ` [Xen-devel] " Andrew Cooper 2015-12-20 17:07 ` Andrew Cooper 2015-12-20 17:07 ` [Qemu-devel] " Andrew Cooper 2015-12-20 19:59 ` Peter Zijlstra 2015-12-20 19:59 ` Peter Zijlstra 2015-12-20 19:59 ` [Qemu-devel] " Peter Zijlstra 2015-12-21 7:10 ` Michael S. Tsirkin 2015-12-21 7:10 ` [Xen-devel] " Michael S. Tsirkin 2015-12-21 7:10 ` Michael S. Tsirkin [this message] 2015-12-21 7:10 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-21 7:22 ` [PATCH RFC] smp_store_mb should use smp_mb Michael S. Tsirkin 2015-12-21 7:22 ` Michael S. Tsirkin 2015-12-21 7:22 ` Michael S. Tsirkin 2015-12-21 7:22 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-20 19:59 ` new barrier type for paravirt (was Re: [PATCH] virtio_ring: use smp_store_mb) Peter Zijlstra 2015-12-21 10:47 ` David Vrabel 2015-12-21 10:47 ` [Xen-devel] " David Vrabel 2015-12-21 10:47 ` David Vrabel 2015-12-21 10:47 ` [Qemu-devel] " David Vrabel 2015-12-21 11:52 ` Michael S. Tsirkin 2015-12-21 11:52 ` [Xen-devel] " Michael S. Tsirkin 2015-12-21 11:52 ` Michael S. Tsirkin 2015-12-21 11:52 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-21 14:50 ` Stefano Stabellini 2015-12-21 14:50 ` Stefano Stabellini 2015-12-21 14:50 ` Stefano Stabellini 2015-12-21 14:50 ` Stefano Stabellini 2015-12-21 14:50 ` [Qemu-devel] " Stefano Stabellini 2015-12-20 9:25 ` Michael S. Tsirkin 2015-12-17 11:22 ` [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra 2015-12-17 11:22 ` [Qemu-devel] " Peter Zijlstra 2015-12-17 13:26 ` Michael S. Tsirkin 2015-12-17 13:26 ` Michael S. Tsirkin 2015-12-17 13:26 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-17 14:02 ` Peter Zijlstra 2015-12-17 14:02 ` Peter Zijlstra 2015-12-17 14:02 ` [Qemu-devel] " Peter Zijlstra 2015-12-17 14:34 ` Michael S. Tsirkin 2015-12-17 14:34 ` Michael S. Tsirkin 2015-12-17 14:34 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-17 15:09 ` Peter Zijlstra 2015-12-17 15:09 ` Peter Zijlstra 2015-12-17 15:09 ` [Qemu-devel] " Peter Zijlstra 2015-12-17 15:52 ` Will Deacon 2015-12-17 15:52 ` Will Deacon 2015-12-17 15:52 ` [Qemu-devel] " Will Deacon 2015-12-17 19:21 ` Michael S. Tsirkin 2015-12-17 19:21 ` Michael S. Tsirkin 2015-12-17 19:21 ` [Qemu-devel] " Michael S. Tsirkin 2015-12-17 11:22 ` Peter Zijlstra
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20151221085947-mutt-send-email-mst@redhat.com \ --to=mst@redhat.com \ --cc=alexander.duyck@gmail.com \ --cc=andrew.cooper3@citrix.com \ --cc=arnd@arndb.de \ --cc=boris.ostrovsky@oracle.com \ --cc=david.vrabel@citrix.com \ --cc=jasowang@redhat.com \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=peterz@infradead.org \ --cc=qemu-devel@nongnu.org \ --cc=rusty@rustcorp.com.au \ --cc=virtualization@lists.linux-foundation.org \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.