From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43369) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPuL4-0004sl-Pr for qemu-devel@nongnu.org; Wed, 10 Apr 2013 08:41:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPuKz-00075Z-U8 for qemu-devel@nongnu.org; Wed, 10 Apr 2013 08:41:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17338) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPuKz-00075R-LZ for qemu-devel@nongnu.org; Wed, 10 Apr 2013 08:41:17 -0400 Message-ID: <51655DB4.3070600@redhat.com> Date: Wed, 10 Apr 2013 14:40:20 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1365568180-19593-1-git-send-email-mrhines@linux.vnet.ibm.com> <1365568180-19593-5-git-send-email-mrhines@linux.vnet.ibm.com> <516519CA.4090809@redhat.com> <51655C62.2030508@linux.vnet.ibm.com> In-Reply-To: <51655C62.2030508@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH RDMA support v6: 4/7] Introduce two new capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael R. Hines" Cc: aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com Il 10/04/2013 14:34, Michael R. Hines ha scritto: > On 04/10/2013 03:50 AM, Paolo Bonzini wrote: >> Il 10/04/2013 06:29, mrhines@linux.vnet.ibm.com ha scritto: >>> From: "Michael R. Hines" >>> >>> RDMA performs very slowly with zero-page checking. >>> Without the ability to disable it, RDMA throughput and >>> latency promises and high performance links cannot be >>> fully realized. >>> >>> On the other hand, dynamic page registration support is also >>> included in the RDMA protocol. This second capability also >>> cannot be fully realized without the ability to enable zero >>> page scanning. >>> >>> So, we have two new capabilities which work together: >>> >>> 1. migrate_set_capability check_for_zero on|off (default on) >>> 2. migrate_set_capability chunk_register_destination on|off (default >>> off) >>> >>> Signed-off-by: Michael R. Hines >> Michael, please listen to _all_ review comments. >> >> 1) I asked you to place check_for_zero in a separate patch. >> >> 2) Again, patch 3 cannot compile without this one. The code should >> compile after each patch, with and without --enable-rdma. > > My apologies - I misunderstood the request. I am indeed addressing every > comment. > > When you said separate, I thought you meant a different patch series > altpgether. > > This is my first time, so the meaning of "separate" was not clear =) > > And yes, patch 3 does in fact compile both with and without --enable-rdma. How does this: +#ifdef CONFIG_RDMA +const QEMURamControlOps qemu_rdma_write_control = { + .before_ram_iterate = qemu_ram_registration_start, + .after_ram_iterate = qemu_rdma_registration_stop, + .register_ram_iterate = qemu_rdma_registration_handle, + .save_page = qemu_rdma_save_page, +}; compile (and link) successfully without a definition of qemu_ram_registration_start, which is in patch 5? (Honest question). Similarly, patch 5 cannot link without qemu_ram_foreach_block. Anyway, patch 1 does not compile with --enable-rdma. :) > You flip-flopped on me =) > You said conditionalize it, then make a separate patch, then delete it > altogether =) Make qemu_set_nonblock conditional, dropping the assert. Do not touch the implementation of qemu_set_nonblock. (Yes, this was a mess). Paolo