Re: [Qemu-devel] [PATCH v2 0/8] nvdimm: hotplug support

[Xiao Guangrong]

On 10/10/2016 08:59 PM, Igor Mammedov wrote:
> On Sat, 8 Oct 2016 16:34:06 +0800
> Xiao Guangrong <address@hidden> wrote:
>
> > On 10/03/2016 09:48 PM, Igor Mammedov wrote:
> > > On Fri, 12 Aug 2016 14:54:02 +0800
> > > Xiao Guangrong <address@hidden> wrote:
> > >
> > > General design issue in this series is regenerating
> > > _FIT data every time inside of _FIT read loop.
> > >
> > > The issue here is that if FIT data doesn't fit in one page
> > > RFIT would be called several times resulting in calling
> > > nvdimm_dsm_func_read_fit() N-times, and in between
> > > these N exits generated FIT data could change
> > > (for example if another NVDIMM has been hotpluged in between)
> > > resulting in corrupted concatenated FIT buffer returned by
> > > _FIT method.
> > > So one need either to block follow up hotplug or make sure
> > > that FIT data regenerated/updated only once per _FIT
> > > invocation and stay immutable until _FIT is completed
> > > (maybe RCU protected buffer).
>
>
> > > Regenerating/updating inside qemu could also be shared
> > > with NFIT table creation path so that both cold/hotplug
> > > would reuse the same data which are updated only when
> > > a NVDIMM is added/removed.
> try to take into account this point so that FIT data
> aren't regenerated on every RFIT invocation.

Okay.

The reason that the FIT was always regenerated is avoiding saving
FIT during live-migration. Well, it seems that we are hard to
avoid it now. :)

> > Explicitly sync up QEMU and OSPM is hard as OSPM is running
> > in a VM which is vulnerable and can not be expected always
> > triggering correct sequence.
> >
> > So how about introducing generation-number which is updated
> > for each hotplug event and a new DSM function reserved to
> > get this number by OSPM. Then the ACPI code is like this:
> >
> > restart:
> >     old-number = DSM(Read_NUM);
> >     FIT = DSM(FIT)
> >     new-number = DSM(Read_NUM);
> >
> >     if (old-number != new-number)
> >        goto restart;
> >
> >     return FIT
> It's a bit complicated.

Hmm, how about this algorithm:

struct fit_data {
        u64 generation_number;
        GArray *fit;
};

struct fit_data *fit;
u64 current_number;

The update-side (hotplug happens):
  new_fit = malloc();

  old_fit = rcu_dereference(fit);

  new_fit->generation_number = old_fit->generation_number + 1;
  new_fit->fit = ...generate-fit...;

  rcu_assign(fit, new_fit);
  sync_rcu()
  free(old_fit);


On the read-side (DSM issued by OSPM):
{
   rcu_read_lock()

   fit = rcu_dereference(fit);

   /* if it is the first time issuing RFIT. */
   if (fit-read-position == 0)
     current_number = fit->generation_number;
   else if (current_number != fit->generation_number)
     return FAIL_FIT_CHANGED;

   ... fill returned info with fit->fit...;

   rcu_read_unlock()
}

Of course, @fit and @current_number should be persistent during live migration.