linux-kernel - Re: [PATCH] virtio_ring: use smp_store

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151217131554-mutt-send-email-mst@redhat.com>
Date:	Thu, 17 Dec 2015 15:16:20 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Jason Wang <jasowang@...hat.com>, qemu-devel@...gnu.org
Subject: Re: [PATCH] virtio_ring: use smp_store_mb

On Thu, Dec 17, 2015 at 11:52:38AM +0100, Peter Zijlstra wrote:
> On Thu, Dec 17, 2015 at 12:32:53PM +0200, Michael S. Tsirkin wrote:
> > +static inline void virtio_store_mb(bool weak_barriers,
> > +				   __virtio16 *p, __virtio16 v)
> > +{
> > +#ifdef CONFIG_SMP
> > +	if (weak_barriers)
> > +		smp_store_mb(*p, v);
> > +	else
> > +#endif
> > +	{
> > +		WRITE_ONCE(*p, v);
> > +		mb();
> > +	}
> > +}
> 
> This is a different barrier depending on SMP, that seems wrong.

Of course it's wrong in the sense that it's
suboptimal on UP. What we would really like is to
have, on UP, exactly the same barrier as on SMP.
This is because a UP guest can run on an SMP host.

But Linux doesn't provide this ability: if CONFIG_SMP is
not defined is optimizes most barriers out to a
compiler barrier.

Consider for example x86: what we want is xchg (NOT
mfence - see below for why) but if built without CONFIG_SMP
smp_store_mb does not include this.

> smp_mb(), as (should be) used by smp_store_mb() does not provide a
> barrier against IO. mb() otoh does.
> 
> Since this is virtIO I would expect you always want mb().

No because it's VIRTio not real io :) It just switches to the hyprevisor
mode - kind of like a function call really.
The weak_barriers flag is cleared for when it's used
with real devices with real IO.

All this is explained in some detail at the top of
include/linux/virtio.h

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/