From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40501) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHVqZ-0001ZL-IE for qemu-devel@nongnu.org; Tue, 21 Jul 2015 07:36:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZHVqV-0000bw-Gy for qemu-devel@nongnu.org; Tue, 21 Jul 2015 07:36:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55336) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHVqV-0000bm-BT for qemu-devel@nongnu.org; Tue, 21 Jul 2015 07:36:27 -0400 Date: Tue, 21 Jul 2015 17:06:00 +0530 From: Amit Shah Message-ID: <20150721113600.GJ10280@grmbl.mre> References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-26-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1434450415-11339-26-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 25/42] Postcopy: Maintain sentmap and calculate discard List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, pbonzini@redhat.com, david@gibson.dropbear.id.au On (Tue) 16 Jun 2015 [11:26:38], Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > 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 Reviewed-by: Amit Shah 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). > +/* > + * 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 > +/* > + * 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?) > +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. Amit