[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 25/42] Postcopy: Maintain sentmap and calcula
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v7 25/42] Postcopy: Maintain sentmap and calculate discard |
Date: |
Fri, 31 Jul 2015 17:51:44 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Amit Shah (address@hidden) wrote:
> On (Tue) 16 Jun 2015 [11:26:38], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Where postcopy is preceeded by a period of precopy, the destination will
> > have received pages that may have been dirtied on the source after the
> > page was sent. The destination must throw these pages away before
> > starting it's CPUs.
> >
> > Maintain a 'sentmap' of pages that have already been sent.
> > Calculate list of sent & dirty pages
> > Provide helpers on the destination side to discard these.
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
>
> Reviewed-by: Amit Shah <address@hidden>
>
> Some whitespace issues, and some sentences in comments don't have a
> full-stop:
>
> > +/*
> > + * Called by the bitmap code for each chunk to discard
> > + * May send a discard message, may just leave it queued to
> > + * be sent later
> > + * 'start' and 'end' describe an inclusive range of pages in the
> > + * migration bitmap in the RAM block passed to postcopy_discard_send_init
> > + */
> > +void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState
> > *pds,
> > + unsigned long start, unsigned long end);
>
> unaligned line; no full-stop in comment above (similar elsewhere, not
> repeating that).
Fixed.
> > +/*
> > + * Discard the contents of memory start..end inclusive.
> > + * We can assume that if we've been called postcopy_ram_hosttest returned
> > true
> > + */
> > +int postcopy_ram_discard_range(MigrationIncomingState *mis, uint8_t *start,
> > + uint8_t *end)
> > +{
> > + trace_postcopy_ram_discard_range(start, end);
> > + if (madvise(start, (end-start)+1, MADV_DONTNEED)) {
>
> whitespace around operators
Fixed.
> > +/*
> > + * Called by the bitmap code for each chunk to discard
> > + * May send a discard message, may just leave it queued to
> > + * be sent later
> > + * 'start' and 'end' describe an inclusive range of pages in the
> > + * migration bitmap in the RAM block passed to postcopy_discard_send_init
>
> missing punctuation
>
> (also, you had started doing doxygen-style comments, want to keep on
> following that style?)
Fixed (I'm sure there are others, still getting into the hang of that).
> > +static RAMBlock *ram_find_block(const char *id)
>
> just a suggestion, not very particular about this: rename to
>
> ram_find_block_by_id()
>
> instead, so that it's clear what method of finding we're using; also
> no name conflicts when there might be other ways of doing a find.
Done.
Dave
>
> Amit
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK