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

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

[Johannes Thumshirn]

From: Alexander Graf <graf@amazon.com>

KCSAN reports data races when accessing virtio ring fields that are
concurrently written by the device (host). These are legitimate
concurrent accesses where the CPU reads fields that the device updates
via DMA-like mechanisms.

Add accessor functions that use READ_ONCE() to properly annotate these
device-writable fields and prevent compiler optimizations that could in
theory break the code. This also serves as documentation showing which
fields are shared with the device.

The affected fields are:
- Split ring: used->idx, used->ring[].id, used->ring[].len
- Packed ring: desc[].flags, desc[].id, desc[].len

This patch was partially written using the help of Kiro, an
AI coding assistant, to automate the mechanical work of generating the
inline function definition.

Signed-off-by: Alexander Graf <graf@amazon.com>
[jth: Add READ_ONCE in virtqueue_kick_prepare_split ]
Co-developed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Alexander Graf <graf@amazon.com>
---
Changes to v2:
- Add AI statement (agraf)
- Add R-b from agraf
- Update comment (mst)
- Add split to function names handling split rings (mst)
- Add vring_read_split_avail_event() (mst)

Changes to v1:
- Updated comments (mst, agraf)
- Moved _read suffix to prefix in newly introduced functions (mst)
- Update my minor contribution to Co-developed-by (agraf)
- Add "in theory" to changelog
---
 drivers/virtio/virtio_ring.c | 72 +++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 14 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index ddab68959671..53d5334576bc 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -222,6 +222,55 @@ struct vring_virtqueue {
 #endif
 };
 
+/*
+ * 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)
+{
+	return virtio16_to_cpu(vq->vq.vdev,
+			       READ_ONCE(vq->split.vring.used->idx));
+}
+
+static inline u32 vring_read_split_used_id(const struct vring_virtqueue *vq,
+					   u16 idx)
+{
+	return virtio32_to_cpu(vq->vq.vdev,
+			       READ_ONCE(vq->split.vring.used->ring[idx].id));
+}
+
+static inline u32 vring_read_split_used_len(const struct vring_virtqueue *vq, u16 idx)
+{
+	return virtio32_to_cpu(vq->vq.vdev,
+			       READ_ONCE(vq->split.vring.used->ring[idx].len));
+}
+
+static inline u16 vring_read_split_avail_event(const struct vring_virtqueue *vq)
+{
+	return virtio16_to_cpu(vq->vq.vdev,
+			       READ_ONCE(vring_avail_event(&vq->split.vring)));
+}
+
+static inline u16 vring_read_packed_desc_flags(const struct vring_virtqueue *vq,
+					       u16 idx)
+{
+	return le16_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].flags));
+}
+
+static inline u16 vring_read_packed_desc_id(const struct vring_virtqueue *vq,
+				            u16 idx)
+{
+	return le16_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].id));
+}
+
+static inline u32 vring_read_packed_desc_len(const struct vring_virtqueue *vq,
+				             u16 idx)
+{
+	return le32_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].len));
+}
+
 static struct vring_desc_extra *vring_alloc_desc_extra(unsigned int num);
 static void vring_free(struct virtqueue *_vq);
 
@@ -736,8 +785,7 @@ static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
 	LAST_ADD_TIME_INVALID(vq);
 
 	if (vq->event) {
-		needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev,
-					vring_avail_event(&vq->split.vring)),
+		needs_kick = vring_need_event(vring_read_split_avail_event(vq),
 					      new, old);
 	} else {
 		needs_kick = !(vq->split.vring.used->flags &
@@ -808,8 +856,7 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
 
 static bool more_used_split(const struct vring_virtqueue *vq)
 {
-	return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
-			vq->split.vring.used->idx);
+	return vq->last_used_idx != vring_read_split_used_idx(vq);
 }
 
 static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
@@ -838,10 +885,8 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
 	virtio_rmb(vq->weak_barriers);
 
 	last_used = (vq->last_used_idx & (vq->split.vring.num - 1));
-	i = virtio32_to_cpu(_vq->vdev,
-			vq->split.vring.used->ring[last_used].id);
-	*len = virtio32_to_cpu(_vq->vdev,
-			vq->split.vring.used->ring[last_used].len);
+	i = vring_read_split_used_id(vq, last_used);
+	*len = vring_read_split_used_len(vq, last_used);
 
 	if (unlikely(i >= vq->split.vring.num)) {
 		BAD_RING(vq, "id %u out of range\n", i);
@@ -923,8 +968,7 @@ static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned int last_used_i
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
-	return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev,
-			vq->split.vring.used->idx);
+	return (u16)last_used_idx != vring_read_split_used_idx(vq);
 }
 
 static bool virtqueue_enable_cb_delayed_split(struct virtqueue *_vq)
@@ -1701,10 +1745,10 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
 static inline bool is_used_desc_packed(const struct vring_virtqueue *vq,
 				       u16 idx, bool used_wrap_counter)
 {
-	bool avail, used;
 	u16 flags;
+	bool avail, used;
 
-	flags = le16_to_cpu(vq->packed.vring.desc[idx].flags);
+	flags = vring_read_packed_desc_flags(vq, idx);
 	avail = !!(flags & (1 << VRING_PACKED_DESC_F_AVAIL));
 	used = !!(flags & (1 << VRING_PACKED_DESC_F_USED));
 
@@ -1751,8 +1795,8 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
 	last_used_idx = READ_ONCE(vq->last_used_idx);
 	used_wrap_counter = packed_used_wrap_counter(last_used_idx);
 	last_used = packed_last_used(last_used_idx);
-	id = le16_to_cpu(vq->packed.vring.desc[last_used].id);
-	*len = le32_to_cpu(vq->packed.vring.desc[last_used].len);
+	id = vring_read_packed_desc_id(vq, last_used);
+	*len = vring_read_packed_desc_len(vq, last_used);
 
 	if (unlikely(id >= vq->packed.vring.num)) {
 		BAD_RING(vq, "id %u out of range\n", id);
-- 
2.52.0

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

[Xuan Zhuo]

On Sat, 31 Jan 2026 11:28:09 +0100, Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote:
> From: Alexander Graf <graf@amazon.com>
>
> KCSAN reports data races when accessing virtio ring fields that are
> concurrently written by the device (host). These are legitimate
> concurrent accesses where the CPU reads fields that the device updates
> via DMA-like mechanisms.
>
> Add accessor functions that use READ_ONCE() to properly annotate these
> device-writable fields and prevent compiler optimizations that could in
> theory break the code. This also serves as documentation showing which
> fields are shared with the device.
>
> The affected fields are:
> - Split ring: used->idx, used->ring[].id, used->ring[].len
> - Packed ring: desc[].flags, desc[].id, desc[].len
>
> This patch was partially written using the help of Kiro, an
> AI coding assistant, to automate the mechanical work of generating the
> inline function definition.
>
> Signed-off-by: Alexander Graf <graf@amazon.com>
> [jth: Add READ_ONCE in virtqueue_kick_prepare_split ]
> Co-developed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Reviewed-by: Alexander Graf <graf@amazon.com>
> ---
> Changes to v2:
> - Add AI statement (agraf)
> - Add R-b from agraf
> - Update comment (mst)
> - Add split to function names handling split rings (mst)
> - Add vring_read_split_avail_event() (mst)
>
> Changes to v1:
> - Updated comments (mst, agraf)
> - Moved _read suffix to prefix in newly introduced functions (mst)
> - Update my minor contribution to Co-developed-by (agraf)
> - Add "in theory" to changelog
> ---
>  drivers/virtio/virtio_ring.c | 72 +++++++++++++++++++++++++++++-------
>  1 file changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index ddab68959671..53d5334576bc 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -222,6 +222,55 @@ struct vring_virtqueue {
>  #endif
>  };
>
> +/*
> + * 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.

Ohters LGTM.

Thanks.


> +{
> +	return virtio16_to_cpu(vq->vq.vdev,
> +			       READ_ONCE(vq->split.vring.used->idx));
> +}
> +
> +static inline u32 vring_read_split_used_id(const struct vring_virtqueue *vq,
> +					   u16 idx)
> +{
> +	return virtio32_to_cpu(vq->vq.vdev,
> +			       READ_ONCE(vq->split.vring.used->ring[idx].id));
> +}
> +
> +static inline u32 vring_read_split_used_len(const struct vring_virtqueue *vq, u16 idx)
> +{
> +	return virtio32_to_cpu(vq->vq.vdev,
> +			       READ_ONCE(vq->split.vring.used->ring[idx].len));
> +}
> +
> +static inline u16 vring_read_split_avail_event(const struct vring_virtqueue *vq)
> +{
> +	return virtio16_to_cpu(vq->vq.vdev,
> +			       READ_ONCE(vring_avail_event(&vq->split.vring)));
> +}
> +
> +static inline u16 vring_read_packed_desc_flags(const struct vring_virtqueue *vq,
> +					       u16 idx)
> +{
> +	return le16_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].flags));
> +}
> +
> +static inline u16 vring_read_packed_desc_id(const struct vring_virtqueue *vq,
> +				            u16 idx)
> +{
> +	return le16_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].id));
> +}
> +
> +static inline u32 vring_read_packed_desc_len(const struct vring_virtqueue *vq,
> +				             u16 idx)
> +{
> +	return le32_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].len));
> +}
> +
>  static struct vring_desc_extra *vring_alloc_desc_extra(unsigned int num);
>  static void vring_free(struct virtqueue *_vq);
>
> @@ -736,8 +785,7 @@ static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
>  	LAST_ADD_TIME_INVALID(vq);
>
>  	if (vq->event) {
> -		needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev,
> -					vring_avail_event(&vq->split.vring)),
> +		needs_kick = vring_need_event(vring_read_split_avail_event(vq),
>  					      new, old);
>  	} else {
>  		needs_kick = !(vq->split.vring.used->flags &
> @@ -808,8 +856,7 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
>
>  static bool more_used_split(const struct vring_virtqueue *vq)
>  {
> -	return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
> -			vq->split.vring.used->idx);
> +	return vq->last_used_idx != vring_read_split_used_idx(vq);
>  }
>
>  static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
> @@ -838,10 +885,8 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
>  	virtio_rmb(vq->weak_barriers);
>
>  	last_used = (vq->last_used_idx & (vq->split.vring.num - 1));
> -	i = virtio32_to_cpu(_vq->vdev,
> -			vq->split.vring.used->ring[last_used].id);
> -	*len = virtio32_to_cpu(_vq->vdev,
> -			vq->split.vring.used->ring[last_used].len);
> +	i = vring_read_split_used_id(vq, last_used);
> +	*len = vring_read_split_used_len(vq, last_used);
>
>  	if (unlikely(i >= vq->split.vring.num)) {
>  		BAD_RING(vq, "id %u out of range\n", i);
> @@ -923,8 +968,7 @@ static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned int last_used_i
>  {
>  	struct vring_virtqueue *vq = to_vvq(_vq);
>
> -	return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev,
> -			vq->split.vring.used->idx);
> +	return (u16)last_used_idx != vring_read_split_used_idx(vq);
>  }
>
>  static bool virtqueue_enable_cb_delayed_split(struct virtqueue *_vq)
> @@ -1701,10 +1745,10 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
>  static inline bool is_used_desc_packed(const struct vring_virtqueue *vq,
>  				       u16 idx, bool used_wrap_counter)
>  {
> -	bool avail, used;
>  	u16 flags;
> +	bool avail, used;
>
> -	flags = le16_to_cpu(vq->packed.vring.desc[idx].flags);
> +	flags = vring_read_packed_desc_flags(vq, idx);
>  	avail = !!(flags & (1 << VRING_PACKED_DESC_F_AVAIL));
>  	used = !!(flags & (1 << VRING_PACKED_DESC_F_USED));
>
> @@ -1751,8 +1795,8 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
>  	last_used_idx = READ_ONCE(vq->last_used_idx);
>  	used_wrap_counter = packed_used_wrap_counter(last_used_idx);
>  	last_used = packed_last_used(last_used_idx);
> -	id = le16_to_cpu(vq->packed.vring.desc[last_used].id);
> -	*len = le32_to_cpu(vq->packed.vring.desc[last_used].len);
> +	id = vring_read_packed_desc_id(vq, last_used);
> +	*len = vring_read_packed_desc_len(vq, last_used);
>
>  	if (unlikely(id >= vq->packed.vring.num)) {
>  		BAD_RING(vq, "id %u out of range\n", id);
> --
> 2.52.0
>

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

[Johannes Thumshirn]

On 2/3/26 9:29 AM, Xuan Zhuo 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.
>
> Ohters LGTM.
>
> Thanks.

Is it?

johannes@neo:~/src/linux (master)$ git grep -E "\bstatic inline\b" 
{arch,drivers,fs,mm}/**.c | wc -l
18295

That's new to me, sorry. But if there's consensus I can re-send the 
patch without inline.

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

[Michael S. Tsirkin]

On Tue, Feb 03, 2026 at 04:27:24PM +0800, Xuan Zhuo wrote:
> On Sat, 31 Jan 2026 11:28:09 +0100, Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote:
> > From: Alexander Graf <graf@amazon.com>
> >
> > KCSAN reports data races when accessing virtio ring fields that are
> > concurrently written by the device (host). These are legitimate
> > concurrent accesses where the CPU reads fields that the device updates
> > via DMA-like mechanisms.
> >
> > Add accessor functions that use READ_ONCE() to properly annotate these
> > device-writable fields and prevent compiler optimizations that could in
> > theory break the code. This also serves as documentation showing which
> > fields are shared with the device.
> >
> > The affected fields are:
> > - Split ring: used->idx, used->ring[].id, used->ring[].len
> > - Packed ring: desc[].flags, desc[].id, desc[].len
> >
> > This patch was partially written using the help of Kiro, an
> > AI coding assistant, to automate the mechanical work of generating the
> > inline function definition.
> >
> > Signed-off-by: Alexander Graf <graf@amazon.com>
> > [jth: Add READ_ONCE in virtqueue_kick_prepare_split ]
> > Co-developed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> > Reviewed-by: Alexander Graf <graf@amazon.com>
> > ---
> > Changes to v2:
> > - Add AI statement (agraf)
> > - Add R-b from agraf
> > - Update comment (mst)
> > - Add split to function names handling split rings (mst)
> > - Add vring_read_split_avail_event() (mst)
> >
> > Changes to v1:
> > - Updated comments (mst, agraf)
> > - Moved _read suffix to prefix in newly introduced functions (mst)
> > - Update my minor contribution to Co-developed-by (agraf)
> > - Add "in theory" to changelog
> > ---
> >  drivers/virtio/virtio_ring.c | 72 +++++++++++++++++++++++++++++-------
> >  1 file changed, 58 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index ddab68959671..53d5334576bc 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -222,6 +222,55 @@ struct vring_virtqueue {
> >  #endif
> >  };
> >
> > +/*
> > + * 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.


> Ohters LGTM.
> 
> Thanks.




> 
> > +{
> > +	return virtio16_to_cpu(vq->vq.vdev,
> > +			       READ_ONCE(vq->split.vring.used->idx));
> > +}
> > +
> > +static inline u32 vring_read_split_used_id(const struct vring_virtqueue *vq,
> > +					   u16 idx)
> > +{
> > +	return virtio32_to_cpu(vq->vq.vdev,
> > +			       READ_ONCE(vq->split.vring.used->ring[idx].id));
> > +}
> > +
> > +static inline u32 vring_read_split_used_len(const struct vring_virtqueue *vq, u16 idx)
> > +{
> > +	return virtio32_to_cpu(vq->vq.vdev,
> > +			       READ_ONCE(vq->split.vring.used->ring[idx].len));
> > +}
> > +
> > +static inline u16 vring_read_split_avail_event(const struct vring_virtqueue *vq)
> > +{
> > +	return virtio16_to_cpu(vq->vq.vdev,
> > +			       READ_ONCE(vring_avail_event(&vq->split.vring)));
> > +}
> > +
> > +static inline u16 vring_read_packed_desc_flags(const struct vring_virtqueue *vq,
> > +					       u16 idx)
> > +{
> > +	return le16_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].flags));
> > +}
> > +
> > +static inline u16 vring_read_packed_desc_id(const struct vring_virtqueue *vq,
> > +				            u16 idx)
> > +{
> > +	return le16_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].id));
> > +}
> > +
> > +static inline u32 vring_read_packed_desc_len(const struct vring_virtqueue *vq,
> > +				             u16 idx)
> > +{
> > +	return le32_to_cpu(READ_ONCE(vq->packed.vring.desc[idx].len));
> > +}
> > +
> >  static struct vring_desc_extra *vring_alloc_desc_extra(unsigned int num);
> >  static void vring_free(struct virtqueue *_vq);
> >
> > @@ -736,8 +785,7 @@ static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
> >  	LAST_ADD_TIME_INVALID(vq);
> >
> >  	if (vq->event) {
> > -		needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev,
> > -					vring_avail_event(&vq->split.vring)),
> > +		needs_kick = vring_need_event(vring_read_split_avail_event(vq),
> >  					      new, old);
> >  	} else {
> >  		needs_kick = !(vq->split.vring.used->flags &
> > @@ -808,8 +856,7 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
> >
> >  static bool more_used_split(const struct vring_virtqueue *vq)
> >  {
> > -	return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
> > -			vq->split.vring.used->idx);
> > +	return vq->last_used_idx != vring_read_split_used_idx(vq);
> >  }
> >
> >  static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
> > @@ -838,10 +885,8 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
> >  	virtio_rmb(vq->weak_barriers);
> >
> >  	last_used = (vq->last_used_idx & (vq->split.vring.num - 1));
> > -	i = virtio32_to_cpu(_vq->vdev,
> > -			vq->split.vring.used->ring[last_used].id);
> > -	*len = virtio32_to_cpu(_vq->vdev,
> > -			vq->split.vring.used->ring[last_used].len);
> > +	i = vring_read_split_used_id(vq, last_used);
> > +	*len = vring_read_split_used_len(vq, last_used);
> >
> >  	if (unlikely(i >= vq->split.vring.num)) {
> >  		BAD_RING(vq, "id %u out of range\n", i);
> > @@ -923,8 +968,7 @@ static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned int last_used_i
> >  {
> >  	struct vring_virtqueue *vq = to_vvq(_vq);
> >
> > -	return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev,
> > -			vq->split.vring.used->idx);
> > +	return (u16)last_used_idx != vring_read_split_used_idx(vq);
> >  }
> >
> >  static bool virtqueue_enable_cb_delayed_split(struct virtqueue *_vq)
> > @@ -1701,10 +1745,10 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
> >  static inline bool is_used_desc_packed(const struct vring_virtqueue *vq,
> >  				       u16 idx, bool used_wrap_counter)
> >  {
> > -	bool avail, used;
> >  	u16 flags;
> > +	bool avail, used;
> >
> > -	flags = le16_to_cpu(vq->packed.vring.desc[idx].flags);
> > +	flags = vring_read_packed_desc_flags(vq, idx);
> >  	avail = !!(flags & (1 << VRING_PACKED_DESC_F_AVAIL));
> >  	used = !!(flags & (1 << VRING_PACKED_DESC_F_USED));
> >
> > @@ -1751,8 +1795,8 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
> >  	last_used_idx = READ_ONCE(vq->last_used_idx);
> >  	used_wrap_counter = packed_used_wrap_counter(last_used_idx);
> >  	last_used = packed_last_used(last_used_idx);
> > -	id = le16_to_cpu(vq->packed.vring.desc[last_used].id);
> > -	*len = le32_to_cpu(vq->packed.vring.desc[last_used].len);
> > +	id = vring_read_packed_desc_id(vq, last_used);
> > +	*len = vring_read_packed_desc_len(vq, last_used);
> >
> >  	if (unlikely(id >= vq->packed.vring.num)) {
> >  		BAD_RING(vq, "id %u out of range\n", id);
> > --
> > 2.52.0
> >

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

[David Laight]

On Tue, 3 Feb 2026 07:02:23 -0500
"Michael S. Tsirkin" <mst@redhat.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