qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: peterx@redhat.com, Juraj Marcin <jmarcin@redhat.com>,
	Julia Suvorova <jusual@redhat.com>,
	Prasad Pandit <ppandit@redhat.com>,
	Fabiano Rosas <farosas@suse.de>
Subject: [PATCH 08/16] migration: Adjust locking in migration_maybe_pause()
Date: Tue, 14 Jan 2025 18:07:38 -0500	[thread overview]
Message-ID: <20250114230746.3268797-9-peterx@redhat.com> (raw)
In-Reply-To: <20250114230746.3268797-1-peterx@redhat.com>

In migration_maybe_pause() QEMU may yield BQL before waiting for a
semaphore.  However it yields the BQL too early, which logically gives it
chance for the main thread to quickly take the BQL and modify the state to
CANCELLING.

To avoid such race condition from happening at all, always update the
migration states within the BQL.  It'll make sure no concurrent
cancellation can ever happen.

With that, IIUC there's chance we can remove the extra parameter in
migration_maybe_pause() to update active state, but that'll be done
separately later.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 13b7df0d5b..5c688059de 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2701,14 +2701,14 @@ static int migration_maybe_pause(MigrationState *s,
      * wait for the 'pause_sem' semaphore.
      */
     if (s->state != MIGRATION_STATUS_CANCELLING) {
-        bql_unlock();
         migrate_set_state(&s->state, *current_active_state,
                           MIGRATION_STATUS_PRE_SWITCHOVER);
+        bql_unlock();
         qemu_sem_wait(&s->pause_sem);
+        bql_lock();
         migrate_set_state(&s->state, MIGRATION_STATUS_PRE_SWITCHOVER,
                           new_state);
         *current_active_state = new_state;
-        bql_lock();
     }
 
     return s->state == new_state ? 0 : -EINVAL;
-- 
2.47.0



  parent reply	other threads:[~2025-01-14 23:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-14 23:07 [PATCH 00/16] migration: Switchover phase refactoring Peter Xu
2025-01-14 23:07 ` [PATCH 01/16] migration: Remove postcopy implications in should_send_vmdesc() Peter Xu
2025-01-14 23:07 ` [PATCH 02/16] migration: Do not construct JSON description if suppressed Peter Xu
2025-01-14 23:07 ` [PATCH 03/16] migration: Optimize postcopy on downtime by avoiding JSON writer Peter Xu
2025-01-14 23:07 ` [PATCH 04/16] migration: Avoid two src-downtime-end tracepoints for postcopy Peter Xu
2025-01-14 23:07 ` [PATCH 05/16] migration: Drop inactivate_disk param in qemu_savevm_state_complete* Peter Xu
2025-01-14 23:07 ` [PATCH 06/16] migration: Synchronize all CPU states only for non-iterable dump Peter Xu
2025-01-14 23:07 ` [PATCH 07/16] migration: Adjust postcopy bandwidth during switchover Peter Xu
2025-01-14 23:07 ` Peter Xu [this message]
2025-01-14 23:07 ` [PATCH 09/16] migration: Drop cached migration state in migration_maybe_pause() Peter Xu
2025-01-14 23:07 ` [PATCH 10/16] migration: Take BQL slightly longer in postcopy_start() Peter Xu
2025-01-14 23:07 ` [PATCH 11/16] migration: Notify COMPLETE once for postcopy Peter Xu
2025-01-14 23:07 ` [PATCH 12/16] migration: Unwrap qemu_savevm_state_complete_precopy() in postcopy Peter Xu
2025-01-14 23:07 ` [PATCH 13/16] migration: Cleanup qemu_savevm_state_complete_precopy() Peter Xu
2025-01-14 23:07 ` [PATCH 14/16] migration: Always set DEVICE state Peter Xu
2025-01-14 23:07 ` [PATCH 15/16] migration: Merge precopy/postcopy on switchover start Peter Xu
2025-01-14 23:07 ` [PATCH 16/16] migration: Trivial cleanup on JSON writer of vmstate_save() Peter Xu
2025-01-15  9:12 ` [PATCH 00/16] migration: Switchover phase refactoring Jiri Denemark
2025-01-15 12:55   ` Peter Xu
2025-01-15 16:13 ` Juraj Marcin
2025-01-15 16:49 ` Fabiano Rosas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250114230746.3268797-9-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=farosas@suse.de \
    --cc=jmarcin@redhat.com \
    --cc=jusual@redhat.com \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).