Re: [Qemu-devel] [RFC PATCH] convert ram_list to RCU DQ

[Paolo Bonzini <pbonzini@redhat.com>]

Il 28/08/2013 18:02, Mike Day ha scritto:
> @@ -457,8 +459,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>      MemoryRegion *mr;
>      ram_addr_t current_addr;
>  
> +    rcu_read_lock();
>      if (!block)
> -        block = QTAILQ_FIRST(&ram_list.blocks);
> +        block = QLIST_FIRST_RCU(&ram_list.blocks);
>  
>      while (true) {
>          mr = block->mr;
> @@ -469,9 +472,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>          }
>          if (offset >= block->length) {
>              offset = 0;
> -            block = QTAILQ_NEXT(block, next);
> +            block = QLIST_NEXT_RCU(block, next);
>              if (!block) {
> -                block = QTAILQ_FIRST(&ram_list.blocks);
> +                block = QLIST_FIRST_RCU(&ram_list.blocks);
>                  complete_round = true;
>                  ram_bulk_stage = false;
>              }
> @@ -526,6 +529,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>              }
>          }
>      }
> +    rcu_read_unlock();

block lives across calls to ram_save_block, which is why the mutex was
locked in the caller (ram_save_iterate) rather than here.  For a first
conversion, keeping the long RCU critical sections is fine.  We don't
use RCU enough yet to care about delaying other call_rcu callbacks.

We can later check push the check for ram_list.version inside
ram_save_block, which should let us make the critical section smaller.
But that would be a bit tricky, so it's better to do it in a separate patch.

> @@ -828,13 +829,18 @@ static inline void *host_from_stream_offset(QEMUFile *f,
>      qemu_get_buffer(f, (uint8_t *)id, len);
>      id[len] = 0;
>  
> -    QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> -        if (!strncmp(id, block->idstr, sizeof(id)))
> -            return memory_region_get_ram_ptr(block->mr) + offset;
> +    rcu_read_lock();
> +    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> +        if (!strncmp(id, block->idstr, sizeof(id))) {
> +            ptr = memory_region_get_ram_ptr(block->mr) + offset;
> +            goto unlock_out;
> +        }
>      }
>  
>      fprintf(stderr, "Can't find block %s!\n", id);
> -    return NULL;
> +unlock_out:
> +    rcu_read_unlock();
> +    return ptr;
>  }


Similarly, here the critical section includes the caller, and block is
living across calls to host.  Again, for now just put all of ram_load
under a huge RCU critical section.  Later we can use ram_list.version to
refresh the list and make the critical sections smaller.

Paolo