[v3] Fix race condition in throttle_group_restart_queue()

[PATCH v3 0/1] Fix race condition in throttle_group_restart_queue()

Alberto Garcia posted 1 patch 3 weeks, 5 days ago

Diff against v2
Download series mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cover.1773316842.git.berto@igalia.com

Maintainers: Alberto Garcia <berto@igalia.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>

block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 19 deletions(-)

Expand all Fold all

[PATCH v3 0/1] Fix race condition in throttle_group_restart_queue()

Posted by Alberto Garcia 3 weeks, 5 days ago

Hi,

In the previous patch I forgot to relax the assertion for case 2.

If we're restarting the queue when another tgm has a timer armed I
don't think we can guarantee that when the timer fires it won't set a
new timer on our own tgm before we reach the assertion.

I think that the assertion should depend on reset_timer_armed:

- If it's true, no one else can arm a timer so the assertion holds.
- If it's false, it is possible for the other tgm to arm our timer.
  The assertion would fail in this case, but I think it's harmless
  because even if it would fire at the same time the logic in
  throttle_group_restart_queue_entry() is protected by the lock.

Berto

Alberto Garcia (1):
  throttle-group: Fix race condition in throttle_group_restart_queue()

 block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 19 deletions(-)

-- 
2.47.3

Re: [PATCH v3 0/1] Fix race condition in throttle_group_restart_queue()

Posted by Hanna Czenczek 3 weeks ago

On 12.03.26 13:11, Alberto Garcia wrote:
> Hi,
>
> In the previous patch I forgot to relax the assertion for case 2.
>
> If we're restarting the queue when another tgm has a timer armed I
> don't think we can guarantee that when the timer fires it won't set a
> new timer on our own tgm before we reach the assertion.
>
> I think that the assertion should depend on reset_timer_armed:
>
> - If it's true, no one else can arm a timer so the assertion holds.
> - If it's false, it is possible for the other tgm to arm our timer.
>    The assertion would fail in this case, but I think it's harmless
>    because even if it would fire at the same time the logic in
>    throttle_group_restart_queue_entry() is protected by the lock.
>
> Berto
>
> Alberto Garcia (1):
>    throttle-group: Fix race condition in throttle_group_restart_queue()
>
>   block/throttle-groups.c | 79 +++++++++++++++++++++++++++++++----------
>   1 file changed, 60 insertions(+), 19 deletions(-)

Thanks, applied to my block branch:

https://gitlab.com/hreitz/qemu/-/commits/block

Hanna