Re: [Qemu-devel] [PATCH 02/14] nvdimm acpi: drop the lock of fit buffer

[Igor Mammedov <imammedo@redhat.com>]

On Mon,  7 Nov 2016 19:13:37 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:

> as there is a global lock to protect vm-exit handlers and
> QMP/monitor, this lock can be dropped
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/nvdimm.c        | 11 +----------
>  include/hw/mem/nvdimm.h | 17 +++++------------
>  2 files changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 623bb36..0fe3547 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -371,17 +371,14 @@ static GArray *nvdimm_build_device_structure(void)
>  
>  static void nvdimm_init_fit_buffer(NvdimmFitBuffer *fit_buf)
>  {
> -    qemu_mutex_init(&fit_buf->lock);
>      fit_buf->fit = g_array_new(false, true /* clear */, 1);
>  }
>  
>  static void nvdimm_build_fit_buffer(NvdimmFitBuffer *fit_buf)
>  {
> -    qemu_mutex_lock(&fit_buf->lock);
>      g_array_free(fit_buf->fit, true);
>      fit_buf->fit = nvdimm_build_device_structure();
>      fit_buf->dirty = true;
> -    qemu_mutex_unlock(&fit_buf->lock);
>  }
>  
>  void nvdimm_acpi_hotplug(AcpiNVDIMMState *state)
> @@ -395,11 +392,10 @@ static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets,
>      NvdimmFitBuffer *fit_buf = &state->fit_buf;
>      unsigned int header;
>  
> -    qemu_mutex_lock(&fit_buf->lock);
>  
>      /* NVDIMM device is not plugged? */
>      if (!fit_buf->fit->len) {
> -        goto exit;
> +        return;
>      }
>  
>      acpi_add_table(table_offsets, table_data);
> @@ -413,9 +409,6 @@ static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets,
>      build_header(linker, table_data,
>                   (void *)(table_data->data + header), "NFIT",
>                   sizeof(NvdimmNfitHeader) + fit_buf->fit->len, 1, NULL, NULL);
> -
> -exit:
> -    qemu_mutex_unlock(&fit_buf->lock);
>  }
>  
>  struct NvdimmDsmIn {
> @@ -544,7 +537,6 @@ static void nvdimm_dsm_func_read_fit(AcpiNVDIMMState *state, NvdimmDsmIn *in,
>      read_fit = (NvdimmFuncReadFITIn *)in->arg3;
>      le32_to_cpus(&read_fit->offset);
>  
> -    qemu_mutex_lock(&fit_buf->lock);
>      fit = fit_buf->fit;
>  
>      nvdimm_debug("Read FIT: offset %#x FIT size %#x Dirty %s.\n",
> @@ -578,7 +570,6 @@ exit:
>      cpu_physical_memory_write(dsm_mem_addr, read_fit_out, size);
>  
>      g_free(read_fit_out);
> -    qemu_mutex_unlock(&fit_buf->lock);
>  }
>  
>  static void nvdimm_dsm_reserved_root(AcpiNVDIMMState *state, NvdimmDsmIn *in,
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 33cd421..d3ffb25 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -99,20 +99,13 @@ typedef struct NVDIMMClass NVDIMMClass;
>  #define NVDIMM_ACPI_IO_LEN      4
>  
>  /*
> - * The buffer, @fit, saves the FIT info for all the presented NVDIMM
> - * devices which is updated after the NVDIMM device is plugged or
> - * unplugged.
> - *
> - * Rules to use the buffer:
> - *    1) the user should hold the @lock to access the buffer.
> - *    2) mark @dirty whenever the buffer is updated.
> - *
> - * These rules preserve NVDIMM ACPI _FIT method to read incomplete
> - * or obsolete fit info if fit update happens during multiple RFIT
> - * calls.
> + * NvdimmFitBuffer:
> + * @fit: FIT structures for present NVDIMMs. It is updated when
> + *   the NVDIMM device is plugged or unplugged.
> + * @dirty: It allows OSPM to detect change and restart read in
> + *   progress if there is any.
>   */
>  struct NvdimmFitBuffer {
> -    QemuMutex lock;
>      GArray *fit;
>      bool dirty;
>  };