* [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
@ 2026-07-23 11:58 Michael S. Tsirkin
2026-07-23 15:42 ` Eric Auger
2026-07-23 16:38 ` Eric Auger
0 siblings, 2 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2026-07-23 11:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Auger, Jean-Philippe Brucker, Paolo Bonzini, Fam Zheng
Currently, within virtio-iommu, handle_command processes the command vq
without any limits on the number of entries processed.
This can easily and repeatedly enable/disable multiple memory regions.
Within the memory code, this causes an accumulation of an
unbounded number of RCU-deferred FlatViews - each of these
is supposed to be freed with call_rcu, but that never happens
because the main thread never returns to the main loop.
Given FlatView is big, it's easy to have this balloon out to multiple
Gigabytes of memory.
Limit the loop defer any remaining work to a timer.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/virtio-iommu.h | 1 +
hw/virtio/virtio-iommu.c | 29 +++++++++++++++++++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 3b86050f2c..1f265540ad 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -65,6 +65,7 @@ struct VirtIOIOMMU {
GTree *domains;
QemuRecMutex mutex;
GTree *endpoints;
+ QEMUTimer *cmd_timer;
bool boot_bypass;
Notifier machine_done;
bool granule_frozen;
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 08f7e8b783..533bd5073f 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
return ret ? ret : virtio_iommu_probe(s, &req, buf);
}
+static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
+
+static void virtio_iommu_handle_command_timer(void *opaque)
+{
+ VirtIOIOMMU *s = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
+ virtio_iommu_handle_command(vdev, s->req_vq);
+ }
+}
+
static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
@@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
struct iovec *iov;
void *buf = NULL;
size_t sz;
+ unsigned int batch = 0;
for (;;) {
size_t output_size = sizeof(tail);
+ if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
+ timer_mod(s->cmd_timer,
+ qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
+ break;
+ }
+
elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
if (!elem) {
return;
@@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
virtio_iommu_handle_command);
s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
+ s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
+ virtio_iommu_handle_command_timer, s);
/*
* config.bypass is needed to get initial address space early, such as
@@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
qemu_rec_mutex_destroy(&s->mutex);
+ timer_free(s->cmd_timer);
virtio_delete_queue(s->req_vq);
virtio_delete_queue(s->event_vq);
virtio_cleanup(vdev);
@@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
trace_virtio_iommu_device_reset_exit();
+ timer_del(s->cmd_timer);
+
if (s->domains) {
g_tree_destroy(s->domains);
}
@@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
* still correct.
*/
virtio_iommu_switch_address_space_all(s);
+
+ if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
+ timer_mod(s->cmd_timer,
+ qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
+ }
return 0;
}
--
MST
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
2026-07-23 11:58 [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu Michael S. Tsirkin
@ 2026-07-23 15:42 ` Eric Auger
2026-07-23 16:01 ` Michael S. Tsirkin
2026-07-23 16:38 ` Eric Auger
1 sibling, 1 reply; 5+ messages in thread
From: Eric Auger @ 2026-07-23 15:42 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Jean-Philippe Brucker, Paolo Bonzini, Fam Zheng
Hi Michael,
On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
> Currently, within virtio-iommu, handle_command processes the command vq
> without any limits on the number of entries processed.
> This can easily and repeatedly enable/disable multiple memory regions.
> Within the memory code, this causes an accumulation of an
> unbounded number of RCU-deferred FlatViews - each of these
> is supposed to be freed with call_rcu, but that never happens
> because the main thread never returns to the main loop.
>
> Given FlatView is big, it's easy to have this balloon out to multiple
> Gigabytes of memory.
>
> Limit the loop defer any remaining work to a timer.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/hw/virtio/virtio-iommu.h | 1 +
> hw/virtio/virtio-iommu.c | 29 +++++++++++++++++++++++++++++
> 3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 3b86050f2c..1f265540ad 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
> GTree *domains;
> QemuRecMutex mutex;
> GTree *endpoints;
> + QEMUTimer *cmd_timer;
> bool boot_bypass;
> Notifier machine_done;
> bool granule_frozen;
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 08f7e8b783..533bd5073f 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
> return ret ? ret : virtio_iommu_probe(s, &req, buf);
> }
>
> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
> +
> +static void virtio_iommu_handle_command_timer(void *opaque)
> +{
> + VirtIOIOMMU *s = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> + if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
> + virtio_iommu_handle_command(vdev, s->req_vq);
So what does happen in terms of scheduling. Can't this run concurrently
with the usual direct virtio_iommu_handle_command() path?
> + }
> +}
> +
> static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> {
> VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> struct iovec *iov;
> void *buf = NULL;
> size_t sz;
> + unsigned int batch = 0;
>
> for (;;) {
> size_t output_size = sizeof(tail);
>
> + if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
how did you chose that condition? is it always safe wrt oom situation?
> + timer_mod(s->cmd_timer,
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> + break;
> + }
> +
> elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> if (!elem) {
> return;
> @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
> virtio_iommu_handle_command);
> s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> + s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
> + virtio_iommu_handle_command_timer, s);
>
> /*
> * config.bypass is needed to get initial address space early, such as
> @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
>
> qemu_rec_mutex_destroy(&s->mutex);
>
> + timer_free(s->cmd_timer);
> virtio_delete_queue(s->req_vq);
> virtio_delete_queue(s->event_vq);
> virtio_cleanup(vdev);
> @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
>
> trace_virtio_iommu_device_reset_exit();
>
> + timer_del(s->cmd_timer);
> +
> if (s->domains) {
> g_tree_destroy(s->domains);
> }
> @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
> * still correct.
> */
> virtio_iommu_switch_address_space_all(s);
> +
> + if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
> + timer_mod(s->cmd_timer,
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> + }
> return 0;
> }
>
Thanks
Eric
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
2026-07-23 15:42 ` Eric Auger
@ 2026-07-23 16:01 ` Michael S. Tsirkin
2026-07-23 16:36 ` Eric Auger
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2026-07-23 16:01 UTC (permalink / raw)
To: Eric Auger; +Cc: qemu-devel, Jean-Philippe Brucker, Paolo Bonzini, Fam Zheng
On Thu, Jul 23, 2026 at 05:42:02PM +0200, Eric Auger wrote:
> Hi Michael,
>
> On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
> > Currently, within virtio-iommu, handle_command processes the command vq
> > without any limits on the number of entries processed.
> > This can easily and repeatedly enable/disable multiple memory regions.
> > Within the memory code, this causes an accumulation of an
> > unbounded number of RCU-deferred FlatViews - each of these
> > is supposed to be freed with call_rcu, but that never happens
> > because the main thread never returns to the main loop.
> >
> > Given FlatView is big, it's easy to have this balloon out to multiple
> > Gigabytes of memory.
> >
> > Limit the loop defer any remaining work to a timer.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
> > Cc: Eric Auger <eric.auger@redhat.com>
> > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > include/hw/virtio/virtio-iommu.h | 1 +
> > hw/virtio/virtio-iommu.c | 29 +++++++++++++++++++++++++++++
> > 3 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> > index 3b86050f2c..1f265540ad 100644
> > --- a/include/hw/virtio/virtio-iommu.h
> > +++ b/include/hw/virtio/virtio-iommu.h
> > @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
> > GTree *domains;
> > QemuRecMutex mutex;
> > GTree *endpoints;
> > + QEMUTimer *cmd_timer;
> > bool boot_bypass;
> > Notifier machine_done;
> > bool granule_frozen;
> > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> > index 08f7e8b783..533bd5073f 100644
> > --- a/hw/virtio/virtio-iommu.c
> > +++ b/hw/virtio/virtio-iommu.c
> > @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
> > return ret ? ret : virtio_iommu_probe(s, &req, buf);
> > }
> >
> > +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
> > +
> > +static void virtio_iommu_handle_command_timer(void *opaque)
> > +{
> > + VirtIOIOMMU *s = opaque;
> > + VirtIODevice *vdev = VIRTIO_DEVICE(s);
> > +
> > + if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
> > + virtio_iommu_handle_command(vdev, s->req_vq);
> So what does happen in terms of scheduling. Can't this run concurrently
> with the usual direct virtio_iommu_handle_command() path?
BQL prevents that.
> > + }
> > +}
> > +
> > static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> > {
> > VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> > @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> > struct iovec *iov;
> > void *buf = NULL;
> > size_t sz;
> > + unsigned int batch = 0;
> >
> > for (;;) {
> > size_t output_size = sizeof(tail);
> >
> > + if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
> how did you chose that condition? is it always safe wrt oom situation?
It's kinda random, but the idea is that queue size already gives us
a point on the memory/speed curve.
There's nothing that is "safe wrt oom" in all configs - qemu makes
no promises and even a single allocation can in theory push you
into oom if memory is tight.
But what this prevents is unbounded memory allocations which will trigger
oom at some point for sure.
> > + timer_mod(s->cmd_timer,
> > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> > + break;
> > + }
> > +
> > elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> > if (!elem) {
> > return;
> > @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> > s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
> > virtio_iommu_handle_command);
> > s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> > + s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
> > + virtio_iommu_handle_command_timer, s);
> >
> > /*
> > * config.bypass is needed to get initial address space early, such as
> > @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
> >
> > qemu_rec_mutex_destroy(&s->mutex);
> >
> > + timer_free(s->cmd_timer);
> > virtio_delete_queue(s->req_vq);
> > virtio_delete_queue(s->event_vq);
> > virtio_cleanup(vdev);
> > @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
> >
> > trace_virtio_iommu_device_reset_exit();
> >
> > + timer_del(s->cmd_timer);
> > +
> > if (s->domains) {
> > g_tree_destroy(s->domains);
> > }
> > @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
> > * still correct.
> > */
> > virtio_iommu_switch_address_space_all(s);
> > +
> > + if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
> > + timer_mod(s->cmd_timer,
> > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> > + }
> > return 0;
> > }
> >
> Thanks
>
> Eric
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
2026-07-23 16:01 ` Michael S. Tsirkin
@ 2026-07-23 16:36 ` Eric Auger
0 siblings, 0 replies; 5+ messages in thread
From: Eric Auger @ 2026-07-23 16:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jean-Philippe Brucker, Paolo Bonzini, Fam Zheng
On 7/23/26 6:01 PM, Michael S. Tsirkin wrote:
> On Thu, Jul 23, 2026 at 05:42:02PM +0200, Eric Auger wrote:
>> Hi Michael,
>>
>> On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
>>> Currently, within virtio-iommu, handle_command processes the command vq
>>> without any limits on the number of entries processed.
>>> This can easily and repeatedly enable/disable multiple memory regions.
>>> Within the memory code, this causes an accumulation of an
>>> unbounded number of RCU-deferred FlatViews - each of these
>>> is supposed to be freed with call_rcu, but that never happens
>>> because the main thread never returns to the main loop.
>>>
>>> Given FlatView is big, it's easy to have this balloon out to multiple
>>> Gigabytes of memory.
>>>
>>> Limit the loop defer any remaining work to a timer.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
>>> Cc: Eric Auger <eric.auger@redhat.com>
>>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>> include/hw/virtio/virtio-iommu.h | 1 +
>>> hw/virtio/virtio-iommu.c | 29 +++++++++++++++++++++++++++++
>>> 3 files changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>>> index 3b86050f2c..1f265540ad 100644
>>> --- a/include/hw/virtio/virtio-iommu.h
>>> +++ b/include/hw/virtio/virtio-iommu.h
>>> @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
>>> GTree *domains;
>>> QemuRecMutex mutex;
>>> GTree *endpoints;
>>> + QEMUTimer *cmd_timer;
>>> bool boot_bypass;
>>> Notifier machine_done;
>>> bool granule_frozen;
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 08f7e8b783..533bd5073f 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
>>> return ret ? ret : virtio_iommu_probe(s, &req, buf);
>>> }
>>>
>>> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
>>> +
>>> +static void virtio_iommu_handle_command_timer(void *opaque)
>>> +{
>>> + VirtIOIOMMU *s = opaque;
>>> + VirtIODevice *vdev = VIRTIO_DEVICE(s);
>>> +
>>> + if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
>>> + virtio_iommu_handle_command(vdev, s->req_vq);
>> So what does happen in terms of scheduling. Can't this run concurrently
>> with the usual direct virtio_iommu_handle_command() path?
> BQL prevents that.
OK
>
>
>>> + }
>>> +}
>>> +
>>> static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>>> {
>>> VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
>>> @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>>> struct iovec *iov;
>>> void *buf = NULL;
>>> size_t sz;
>>> + unsigned int batch = 0;
>>>
>>> for (;;) {
>>> size_t output_size = sizeof(tail);
>>>
>>> + if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
>> how did you chose that condition? is it always safe wrt oom situation?
> It's kinda random, but the idea is that queue size already gives us
> a point on the memory/speed curve.
>
> There's nothing that is "safe wrt oom" in all configs - qemu makes
> no promises and even a single allocation can in theory push you
> into oom if memory is tight.
>
> But what this prevents is unbounded memory allocations which will trigger
> oom at some point for sure.
OK thanks for the explanation
Eric
>
>
>
>>> + timer_mod(s->cmd_timer,
>>> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
>>> + break;
>>> + }
>>> +
>>> elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>>> if (!elem) {
>>> return;
>>> @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>> s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>>> virtio_iommu_handle_command);
>>> s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
>>> + s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
>>> + virtio_iommu_handle_command_timer, s);
>>>
>>> /*
>>> * config.bypass is needed to get initial address space early, such as
>>> @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
>>>
>>> qemu_rec_mutex_destroy(&s->mutex);
>>>
>>> + timer_free(s->cmd_timer);
>>> virtio_delete_queue(s->req_vq);
>>> virtio_delete_queue(s->event_vq);
>>> virtio_cleanup(vdev);
>>> @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
>>>
>>> trace_virtio_iommu_device_reset_exit();
>>>
>>> + timer_del(s->cmd_timer);
>>> +
>>> if (s->domains) {
>>> g_tree_destroy(s->domains);
>>> }
>>> @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
>>> * still correct.
>>> */
>>> virtio_iommu_switch_address_space_all(s);
>>> +
>>> + if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
>>> + timer_mod(s->cmd_timer,
>>> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
>>> + }
>>> return 0;
>>> }
>>>
>> Thanks
>>
>> Eric
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
2026-07-23 11:58 [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu Michael S. Tsirkin
2026-07-23 15:42 ` Eric Auger
@ 2026-07-23 16:38 ` Eric Auger
1 sibling, 0 replies; 5+ messages in thread
From: Eric Auger @ 2026-07-23 16:38 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Jean-Philippe Brucker, Paolo Bonzini, Fam Zheng
Hi Michael,
On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
> Currently, within virtio-iommu, handle_command processes the command vq
> without any limits on the number of entries processed.
> This can easily and repeatedly enable/disable multiple memory regions.
> Within the memory code, this causes an accumulation of an
> unbounded number of RCU-deferred FlatViews - each of these
> is supposed to be freed with call_rcu, but that never happens
> because the main thread never returns to the main loop.
>
> Given FlatView is big, it's easy to have this balloon out to multiple
> Gigabytes of memory.
>
> Limit the loop defer any remaining work to a timer.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
test migration, reset and obviously normal execution with lowered threshold.
Thanks!
Eric
> ---
> include/hw/virtio/virtio-iommu.h | 1 +
> hw/virtio/virtio-iommu.c | 29 +++++++++++++++++++++++++++++
> 3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 3b86050f2c..1f265540ad 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
> GTree *domains;
> QemuRecMutex mutex;
> GTree *endpoints;
> + QEMUTimer *cmd_timer;
> bool boot_bypass;
> Notifier machine_done;
> bool granule_frozen;
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 08f7e8b783..533bd5073f 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
> return ret ? ret : virtio_iommu_probe(s, &req, buf);
> }
>
> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
> +
> +static void virtio_iommu_handle_command_timer(void *opaque)
> +{
> + VirtIOIOMMU *s = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> + if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
> + virtio_iommu_handle_command(vdev, s->req_vq);
> + }
> +}
> +
> static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> {
> VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> struct iovec *iov;
> void *buf = NULL;
> size_t sz;
> + unsigned int batch = 0;
>
> for (;;) {
> size_t output_size = sizeof(tail);
>
> + if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
> + timer_mod(s->cmd_timer,
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> + break;
> + }
> +
> elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> if (!elem) {
> return;
> @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
> virtio_iommu_handle_command);
> s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> + s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
> + virtio_iommu_handle_command_timer, s);
>
> /*
> * config.bypass is needed to get initial address space early, such as
> @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
>
> qemu_rec_mutex_destroy(&s->mutex);
>
> + timer_free(s->cmd_timer);
> virtio_delete_queue(s->req_vq);
> virtio_delete_queue(s->event_vq);
> virtio_cleanup(vdev);
> @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
>
> trace_virtio_iommu_device_reset_exit();
>
> + timer_del(s->cmd_timer);
> +
> if (s->domains) {
> g_tree_destroy(s->domains);
> }
> @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
> * still correct.
> */
> virtio_iommu_switch_address_space_all(s);
> +
> + if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
> + timer_mod(s->cmd_timer,
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> + }
> return 0;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-23 16:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-23 11:58 [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu Michael S. Tsirkin
2026-07-23 15:42 ` Eric Auger
2026-07-23 16:01 ` Michael S. Tsirkin
2026-07-23 16:36 ` Eric Auger
2026-07-23 16:38 ` Eric Auger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.