linux-kernel - Re: [PATCH v3] virtio_ring: Add READ_ONCE annotations for device-writable fields

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] [day] [month] [year] [list]

Message-ID: <20260203170514.4cce4498@pumpkin>
Date: Tue, 3 Feb 2026 17:05:14 +0000
From: David Laight <david.laight.linux@...il.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Johannes Thumshirn
 <johannes.thumshirn@....com>, Alexander Graf <graf@...zon.com>, Jason Wang
 <jasowang@...hat.com>, Eugenio Pérez <eperezma@...hat.com>,
 "open list:VIRTIO CORE" <virtualization@...ts.linux.dev>, open list
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] virtio_ring: Add READ_ONCE annotations for
 device-writable fields

On Tue, 3 Feb 2026 07:02:23 -0500
"Michael S. Tsirkin" <mst@...hat.com> wrote:

...
> > > +/*
> > > + * Accessors for device-writable fields in virtio rings.
> > > + * These fields are concurrently written by the device and read by the driver.
> > > + * Use READ_ONCE() to prevent compiler optimizations, document the
> > > + * intentional data race and prevent KCSAN warnings.
> > > + */
> > > +static inline u16 vring_read_split_used_idx(const struct vring_virtqueue *vq)  
> > 
> > "inline" is not recommended in *.c files.  
> 
> why would it be? it's a compiler hint. given this is the hottest path,
> it makes sense.

The compiler will almost always inline trivial functions regardless
of whether are marked inline or not.
So it is unlikely that adding 'inline' to any of this block of functions
makes any difference at all.

Adding inline to the wrong functions just bloats the code and can make the
code run slower if there are two calls near enough to each other that the
code would still be in the i-cache [1].

There are cases where the code would be smaller and faster if a function
is inlined - but the compiler chooses not to.
Those need always_inline.
Apart from some quite bug functions that shouldn't be marked inline at all
it would actually make sense to #define inline always_inline since that is
what most kernel code actually wants to do.
But a lot of the time the compiler gets it right - which is where the
guidance comes from.

[1] gcc has another way of breaking this by generating multiple copies
of a function for different constant parameters.
Sometimes that can make a big difference to the amount of code - then
it can make sense, but sometimes it just means you have two almost
identical copies to fill memory and the i-cache.

	David