From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58354) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsTBV-0004EJ-3n for qemu-devel@nongnu.org; Fri, 28 Jun 2013 03:33:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UsTBU-0007x0-3z for qemu-devel@nongnu.org; Fri, 28 Jun 2013 03:33:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52628) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsTBT-0007wv-SF for qemu-devel@nongnu.org; Fri, 28 Jun 2013 03:33:32 -0400 Message-ID: <51CD3C3F.6080207@redhat.com> Date: Fri, 28 Jun 2013 09:33:19 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1372373098-5877-1-git-send-email-mrhines@linux.vnet.ibm.com> <1372373098-5877-4-git-send-email-mrhines@linux.vnet.ibm.com> In-Reply-To: <1372373098-5877-4-git-send-email-mrhines@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/6] rdma: core logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mrhines@linux.vnet.ibm.com Cc: aliguori@us.ibm.com, quintela@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com, chegu_vinod@hp.com, knoel@redhat.com Il 28/06/2013 00:44, mrhines@linux.vnet.ibm.com ha scritto: > + * > + * This will have a terrible impact on migration performance, so until future > + * workload information or LRU information is available, do not attempt to use > + * this feature except for basic testing. > + */ > +//#define RDMA_ALLOW_REGISTRATION_WINDOW Out of curiosity, how terrible is the terrible impact? > + * Perform a non-optimized memory registration after every transfer > + * for demonsration purposes, only if pin-all is not requested. > + */ > +static int qemu_rdma_unregister_waiting(RDMAContext *rdma) { > +#ifdef RDMA_ALLOW_REGISTRATION_WINDOW IIRC function can be always enabled, it will just do nothing if no one registers anything. It is just this bit that needs the #ifdef: + + if (!rdma->pin_all) { + /* + * FYI: If one wanted to signal a specific chunk to be unregistered + * using LRU or workload-specific information, this is the function + * you would call to do so. That chunk would then get asynchronously + * unregistered later. + */ >> #ifdef RDMA_ALLOW_REGISTRATION_WINDOW + qemu_rdma_signal_unregister(rdma, index, chunk, wc.wr_id); >> #endif + } Then qemu_rdma_signal_unregister would be unused for !RDMA_ALLOW_REGISTRATION_WINDOW, so you would need to put that function within an #ifdef (the whole function, not just the body). At this point RDMA_ALLOW_REGISTRATION_WINDOW becomes a slightly wrong name. What about RDMA_UNREGISTRATION_EXAMPLE? > > + if(test_bit(chunk, block->transit_bitmap)) { > + DDPRINTF("Cannot unregister inflight chunk: %" PRIu64 "\n", chunk); > + continue; > + } Nobody will process this unregistration after this. Should you move this + // perhaps test_and_clear_bit here? and exit if the bit is zero + clear_bit(chunk, block->unregister_bitmap); + + ret = ibv_dereg_mr(block->pmr[chunk]); + block->pmr[chunk] = NULL; + block->remote_keys[chunk] = 0; + + if(ret != 0) { + perror("unregistration chunk failed"); + return -ret; + } + rdma->total_registrations--; + + reg.key.chunk = chunk; + ret = qemu_rdma_exchange_send(rdma, &head, (uint8_t *) ®, + &resp, &resp_idx, NULL); + if (ret < 0) { + return ret; + } + + DDPRINTF("Unregister for chunk: %" PRIu64 " complete.\n", chunk); to a separate function and check the unregister_bitmap when you get a transfer completed message from IB? Also, you do not need to receive the unregistration completed response now, do you? You can have another bitmap for "unregistration in progress" and only wait for completion before re-pinning. This way, the impact of unregistration is low if the page is not going to be changed soon. _However_, please make this new functionality a separate patch so that we can discuss it better. Paolo