From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XaThC-0007Iw-Uz for qemu-devel@nongnu.org; Sat, 04 Oct 2014 14:04:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XaTh2-0005ON-2e for qemu-devel@nongnu.org; Sat, 04 Oct 2014 14:04:42 -0400 Received: from mail-wi0-x232.google.com ([2a00:1450:400c:c05::232]:53417) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XaTh1-0005OH-RM for qemu-devel@nongnu.org; Sat, 04 Oct 2014 14:04:31 -0400 Received: by mail-wi0-f178.google.com with SMTP id cc10so1366750wib.17 for ; Sat, 04 Oct 2014 11:04:31 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <543036A9.3000703@redhat.com> Date: Sat, 04 Oct 2014 20:04:25 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-38-git-send-email-dgilbert@redhat.com> In-Reply-To: <1412358473-31398-38-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the post-copy queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, lilei@linux.vnet.ibm.com, quintela@redhat.com, cristian.klein@cs.umu.se, amit.shah@redhat.com, yanghy@cn.fujitsu.com Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: > + /* > + * Don't break host-page chunks up with queue items > + * so only unqueue if, > + * a) The last item came from the queue anyway > + * b) The last sent item was the last target-page in a host page > + */ > + if (last_was_from_queue || (!last_sent_block) || Extra parentheses. Is the last_was_from_queue check necessary? Or would one of the other checks be true anyway if last_was_from_queue is true? > + /* We're sending this page, and since it's postcopy nothing else > + * will dirty it, and we must make sure it doesn't get sent again. > + */ > + if (!migration_bitmap_clear_dirty(bitoffset << TARGET_PAGE_BITS)) { > + DPRINTF("%s: Not dirty for postcopy %s/%zx bito=%zx (sent=%d)", > + __func__, tmpblock->idstr, tmpoffset, bitoffset, > + test_bit(bitoffset, ms->sentmap)); If a DPRINTF occurs in a loop, please change it to a tracepoint. This function looks like a candidate for cleaning its logic up and/or splitting it. But it can be done later by the poor soul who will touch it next. :) Paolo