Re: [PATCH] virtio: avoid cost of -ftrivial-auto-var-init in hot path

[Kevin Wolf <kwolf@redhat.com>]

Am 05.06.2025 um 20:54 hat Daniel P. Berrangé geschrieben:
> On Wed, Jun 04, 2025 at 03:18:43PM -0400, Stefan Hajnoczi wrote:
> > Since commit 7ff9ff039380 ("meson: mitigate against use of uninitialize
> > stack for exploits") the -ftrivial-auto-var-init=zero compiler option is
> > used to zero local variables. While this reduces security risks
> > associated with uninitialized stack data, it introduced a measurable
> > bottleneck in the virtqueue_split_pop() and virtqueue_packed_pop()
> > functions.
> > 
> > These virtqueue functions are in the hot path. They are called for each
> > element (request) that is popped from a VIRTIO device's virtqueue. Using
> > __attribute__((uninitialized)) on large stack variables in these
> > functions improves fio randread bs=4k iodepth=64 performance from 304k
> > to 332k IOPS (+9%).
> > 
> > This issue was found using perf-top(1). virtqueue_split_pop() was one of
> > the top CPU consumers and the "annotate" feature showed that the memory
> > zeroing instructions at the beginning of the functions were hot.
> 
> I'm concerned we have other issues lurking, so I built qemu
> with -Wframe-larger-than=8192 and looked at every source
> file location it reported. I've not done performance testing
> but I've found a decent number of locations that look like
> they are in the I/O path, so likely hot paths. It seems I
> was too naive when introducing -ftrivial-auto-var-init=zero
> wrt possible perf hits.
> 
> The results are as follow, and show some areas we should
> likely proactively marked with "QEMU_UNINITIALIZED' even
> without checking perf results:
> 
> 
> ../block/linux-aio.c:342:1: warning: the frame size of 8208 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> 
>   - ioq_submit - struct iocb *iocbs[MAX_EVENTS];

This is certainly a hot path. We should probably try and measure it.

> ../hw/virtio/vhost-user.c:886:1: warning: the frame size of 21760 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> 
>   - Several arrays of VHOST_USER_MAX_RAM_SLOTS (512)
>     Unclear if vhost_user_add_remove_regions is a hot path

Pretty sure it's not, you don't reconfigure your memory all the time.
(If you did, vhost-user performance would be destroyed anyway.)

> ../hw/virtio/virtio.c:1827:1: warning: the frame size of 24784 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> ../hw/virtio/virtio.c:1827:1: warning: the frame size of 24800 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> ../hw/virtio/virtio.c:1977:1: warning: the frame size of 24816 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> 
>   - Several arrays of VIRTQUEUE_MAX_SIZE (1k)
>     Hot path - Stefan's patch fixes
> 
> 
> ../hw/virtio/virtio.c:2156:1: warning: the frame size of 49184 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> ../hw/virtio/virtio.c:2193:1: warning: the frame size of 49184 bytes is larger than 8192 bytes [-Wframe-larger-than=]
> 
>   - Struct VirtQueueElementOld containing several arrays of VIRTQUEUE_MAX_SIZE (1k)
>     Unclear if qemu_{put,get}_virtqueue_element are a hot path

These are for (de)serialising state for migration, not a hot path.

Kevin