From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtLVT-0007qI-JA for qemu-devel@nongnu.org; Mon, 02 Nov 2015 15:15:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtLVO-0007Tl-Fi for qemu-devel@nongnu.org; Mon, 02 Nov 2015 15:15:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53705) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtLVO-0007Tg-8Y for qemu-devel@nongnu.org; Mon, 02 Nov 2015 15:15:02 -0500 Date: Mon, 2 Nov 2015 20:14:56 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20151102201455.GC22239@work-vm> References: <1443515898-3594-1-git-send-email-dgilbert@redhat.com> <1443515898-3594-33-git-send-email-dgilbert@redhat.com> <87lhawa2sj.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87lhawa2sj.fsf@neno.neno> Subject: Re: [Qemu-devel] [PATCH v8 32/54] Postcopy: Maintain sentmap and calculate discard List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: aarcange@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, bharata@linux.vnet.ibm.com, amit.shah@redhat.com, pbonzini@redhat.com * Juan Quintela (quintela@redhat.com) wrote: > > +/* **** functions for postcopy ***** */ > > + > > +/* > > + * Callback from postcopy_each_ram_send_discard for each RAMBlock > > + * start,end: Indexes into the bitmap for the first and last bit > > + * representing the named block > > + */ > > +static int postcopy_send_discard_bm_ram(MigrationState *ms, > > + PostcopyDiscardState *pds, > > + unsigned long start, unsigned long end) > > +{ > > + unsigned long current; > > + > > + for (current = start; current <= end; ) { > > + unsigned long set = find_next_bit(ms->sentmap, end + 1, current); > > + > > + if (set <= end) { > > + unsigned long zero = find_next_zero_bit(ms->sentmap, > > + end + 1, set + 1); > > + > > + if (zero > end) { > > + zero = end + 1; > > + } > > + postcopy_discard_send_range(ms, pds, set, zero - 1); > > + current = zero + 1; > > + } else { > > + current = set; > > + } > > + } > > I think I undrestood the logic here at the end.... > > But if we change the meaning of postcopy_discard_send_range() from > (begin, end), to (begin, length), I think everything goes clearer, no? > > if (set <= end) { > unsigned long zero = find_next_zero_bit(ms->sentmap, > end + 1, set + 1); > unsigned long length; > > if (zero > end) { > length = end - set; > } else { > lenght = zero - 1 - set; > current = zero + 1; > } > postcopy_discard_send_range(ms, pds, set, len); > } else { > current = set; > } > } > > Y would clame that if we call one zero, the other would be called one. > Or change to set/unset, but that is just me. Yes, I haven't tested, and > it is possible that there is a off-by-one somewhere... > > Looking at postocpy_eand_ram_send_discard, I even think that it would be > a good idea to pass length to this function. OK, so I've ended up with (build tested only so far): /* * Callback from postcopy_each_ram_send_discard for each RAMBlock * Note: At this point the 'unsentmap' is the processed bitmap combined * with the dirtymap; so a '1' means it's either dirty or unsent. * start,length: Indexes into the bitmap for the first bit * representing the named block and length in target-pages */ static int postcopy_send_discard_bm_ram(MigrationState *ms, PostcopyDiscardState *pds, unsigned long start, unsigned long length) { unsigned long end = start + length; /* one after the end */ unsigned long current; for (current = start; current < end; ) { unsigned long one = find_next_bit(ms->unsentmap, end, current); if (one <= end) { unsigned long zero = find_next_zero_bit(ms->unsentmap, end, one + 1); unsigned long discard_length; if (zero >= end) { discard_length = end - one; } else { discard_length = zero - one; } postcopy_discard_send_range(ms, pds, one, discard_length); current = one + discard_length; } else { current = one; } } return 0; } Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK