@samitouri / QOSamiQemu / commits / dadd06e727

system/physmem: Synchronize ram_list accesses

Alex Bennée reported a ThreadSanitizer warning about a plain concurrent access to ram_list [1]. Ensure the concurrent accesses to ram_list are properly synchronized with atomic accesses, mutexes, or RCU. First, the plain assignments of ram_list.mru_block are replaced with qatomic_set(). A comment in qemu_get_ram_block() explains why the ordering requirement is relaxed, but it still needs to be atomically accessed. include/qemu/atomic.h says: > The C11 memory model says that variables that are accessed from > different threads should at least be done with __ATOMIC_RELAXED > primitives or the result is undefined. Generally this has little to > no effect on the generated code but not using the atomic primitives > will get flagged by sanitizers as a violation. Second, ram_list.version accesses are replaced with atomic operations or protected with a mutex. Unlike ram_list.mru_block, ram_list.version has tighter ordering requirements for one of its goals: ensuring that the reader-held rs->last_seen_block value is invalidated whenever a RAM block is reclaimed between two RCU reader critical sections. Below are steps a reader and an updater follow: Reader: R-1. Enter the first RCU read-side critical section: R-1-1. rs->last_version = qatomic_load_acquire(&ram_list.version) R-1-2. rs->last_seen_block = an element of ram_list.blocks R-2. Enter the second RCU read-side critical section: R-2-1. if (qatomic_read(&ram_list.version) != rs->last_version) R-2-2. rs->last_seen_block = NULL Updater: W-1. Enter a ram_list.mutex critical section W-1-1. Update ram_list.blocks W-1-2. qatomic_store_release(&ram_list.version, ram_list.version + 1) W-2. Enter another ram_list.mutex critical section W-2-1. QLIST_REMOVE_RCU(block, next) W-2-2. qatomic_store_release(&ram_list.version, ram_list.version + 1) W-2-3. call_rcu(block, reclaim_ramblock, rcu) W-1-2 represents the write observed by R-1-1. ram_list.version is read non-atomically on the update side because the update side is serialized with ram_list.mutex. The other ram_list accesses in these steps are reasoned about in two cases. When the grace period of W-2-3 contains R-2: qatomic_load_acquire() at R-1-1 and qatomic_store_release() at W-1-2 enforce the following ordering: W-1-1 -> W-1-2 -> R-1-1 -> R-1-2 The value of ram_list.blocks stored by W-1-1 or a newer value that was loaded by R-1-2 is still valid because of the grace period. When the grace period of W-2-3 ends before R-2: call_rcu() at W-2-3 and the read-side critical section at R-2 ensure the following ordering: W-2-2 -> W-2-3 -> the grace period -> R-2 -> R-2-1 The value of ram_list.version stored by W-2-2 or a newer value that was loaded by R-2-1 differs from rs->last_version and the reader invalidates rs->last_seen_block. Together, these steps ensure that rs->last_seen_block is invalidated whenever necessary. With added atomic operations, pre-existing memory barriers are no longer necessary and are removed. Any other ram_list accesses are already properly synchronized. [1] https://lore.kernel.org/qemu-devel/878q9fbmap.fsf@draig.linaro.org/ Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Link: https://lore.kernel.org/r/20260523-tsan-v1-1-07d5eb9dcaa2@rsg.ci.i.u-tokyo.ac.jp Signed-off-by: Peter Xu <peterx@redhat.com>

Akihiko Odaki committed May 23, 2026 at 21:38 UTC dadd06e727e7d49ca725d01f6516d7304da059db

2 files changed +12 -14

migration/ram.c

+5 -5

@@ -2495,7 +2495,10 @@ static void ram_state_reset(RAMState *rs)

2495

2496 rs->last_seen_block = NULL;

2497 rs->last_page = 0;

2498 - rs->last_version = ram_list.version;

2498 +

2499 + /* Read version before ram_list.blocks */

2500 + rs->last_version = qatomic_load_acquire(&ram_list.version);

2501 +

2502 rs->xbzrle_started = false;

2503

2504 ram_page_hint_reset(&rs->page_hint);

@@ -3270,13 +3273,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)

3273 */

3274 WITH_QEMU_LOCK_GUARD(&rs->bitmap_mutex) {

3275 WITH_RCU_READ_LOCK_GUARD() {

3273 - if (ram_list.version != rs->last_version) {

3276 + if (qatomic_read(&ram_list.version) != rs->last_version) {

3277 ram_state_reset(rs);

3278 }

3279

3277 - /* Read version before ram_list.blocks */

3278 - smp_rmb();

3279 -

3280 ret = rdma_registration_start(f, RAM_CONTROL_ROUND);

3281 if (ret < 0) {

3282 qemu_file_set_error(f, ret);

system/physmem.c

+7 -9

@@ -839,12 +839,12 @@ found:

839 /* It is safe to write mru_block outside the BQL. This

840 * is what happens:

841 *

842 - * mru_block = xxx

842 + * qatomic_set(&mru_block, xxx)

843 * rcu_read_unlock()

844 * xxx removed from list

845 * rcu_read_lock()

846 * read mru_block

847 - * mru_block = NULL;

847 + * qatomic_set(&mru_block, NULL);

848 * call_rcu(reclaim_ramblock, xxx);

849 * rcu_read_unlock()

850 *

@@ -852,7 +852,7 @@ found:

852 * when it was placed into the list. Here we're just making an extra

853 * copy of the pointer.

854 */

855 - ram_list.mru_block = block;

855 + qatomic_set(&ram_list.mru_block, block);

856 return block;

857 }

858

@@ -2260,11 +2260,10 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)

2260 } else { /* list is empty */

2261 QLIST_INSERT_HEAD_RCU(&ram_list.blocks, new_block, next);

2262 }

2263 - ram_list.mru_block = NULL;

2263 + qatomic_set(&ram_list.mru_block, NULL);

2264

2265 /* Write list before version */

2266 - smp_wmb();

2267 - ram_list.version++;

2266 + qatomic_store_release(&ram_list.version, ram_list.version + 1);

2267 qemu_mutex_unlock_ramlist();

2268

2269 physical_memory_set_dirty_range(new_block->offset,

@@ -2608,10 +2607,9 @@ void qemu_ram_free(RAMBlock *block)

2607 name = cpr_name(block->mr);

2608 cpr_delete_fd(name, 0);

2609 QLIST_REMOVE_RCU(block, next);

2611 - ram_list.mru_block = NULL;

2610 + qatomic_set(&ram_list.mru_block, NULL);

2611 /* Write list before version */

2613 - smp_wmb();

2614 - ram_list.version++;

2612 + qatomic_store_release(&ram_list.version, ram_list.version + 1);

2613 call_rcu(block, reclaim_ramblock, rcu);

2614 qemu_mutex_unlock_ramlist();

2615 }