On Thu, 12 Sep 2024 03:38:12 -0400, "Michael S. Tsirkin" [off-list ref] wrote:

On Thu, Sep 12, 2024 at 02:55:38PM +0800, Xuan Zhuo wrote:

quoted

On Wed, 11 Sep 2024 07:28:36 -0400, "Michael S. Tsirkin" [off-list ref] wrote:

quoted

As gcc luckily noted:

On Tue, Aug 20, 2024 at 03:33:20PM +0800, Xuan Zhuo wrote:

quoted

@@ -1617,23 +1617,24 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
 	}

 	if (vq->indirect) {
+		struct vring_desc_extra *extra;
 		u32 len;

 		/* Free the indirect table, if any, now that it's unmapped. */
-		desc = state->indir_desc;
-		if (!desc)

desc is no longer initialized here

Will fix.

quoted

quoted

+		extra = state->indir;
+		if (!extra)
 			return;

 		if (vring_need_unmap_buffer(vq)) {
 			len = vq->packed.desc_extra[id].len;
 			for (i = 0; i < len / sizeof(struct vring_packed_desc);
 					i++)
-				vring_unmap_desc_packed(vq, &desc[i]);
+				vring_unmap_extra_packed(vq, &extra[i]);
 		}
 		kfree(desc);

but freed here

quoted

-		state->indir_desc = NULL;
+		state->indir = NULL;
 	} else if (ctx) {
-		*ctx = state->indir_desc;
+		*ctx = state->indir;
 	}
 }

It seems unlikely this was always 0 on all paths with even
a small amount of stress, so now I question how this was tested.
Besides, do not ignore compiler warnings, and do not tweak code
to just make compiler shut up - they are your friend.

I agree.

Normally I do this by make W=12, but we have too many message,
so I missed this.

	make W=12 drivers/net/virtio_net.o drivers/virtio/virtio_ring.o

If not W=12, then I did not get any warning message.
How do you get the message quickly?

Thanks.

If you stress test this for a long enough time, and with
debug enabled, you will see a crash.

I only stress tested the split ring. For the packed ring, I
just performed a simple verification.

I will street test for two mode in next version.

Thanks.

quoted

quoted

quoted

--
2.32.0.3.g01195cf9f