From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38098) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VMJG2-0001FE-9N for qemu-devel@nongnu.org; Wed, 18 Sep 2013 11:01:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VMJFu-0003K6-Ae for qemu-devel@nongnu.org; Wed, 18 Sep 2013 11:01:34 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:50400) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VMJFu-0003Jz-57 for qemu-devel@nongnu.org; Wed, 18 Sep 2013 11:01:26 -0400 Received: from /spool/local by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 18 Sep 2013 09:01:25 -0600 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id AB1ED3E4004E for ; Wed, 18 Sep 2013 09:01:21 -0600 (MDT) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r8IF1JmW330720 for ; Wed, 18 Sep 2013 09:01:19 -0600 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r8IF1I1R016517 for ; Wed, 18 Sep 2013 09:01:18 -0600 Message-ID: <5239C03C.3050209@linux.vnet.ibm.com> Date: Wed, 18 Sep 2013 11:01:16 -0400 From: "Michael R. Hines" MIME-Version: 1.0 References: <5d53d839653bf4fd24d5cb56f6d24d3dc3f8f8a5.1378261891.git.yamahata@private.email.ne.jp> <9683795de456be26be098f36807d1d4da3027cd0.1378261891.git.yamahata@private.email.ne.jp> In-Reply-To: <9683795de456be26be098f36807d1d4da3027cd0.1378261891.git.yamahata@private.email.ne.jp> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] rdma: simplify qemu_rdma_register_and_get_keys() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: owasserm@redhat.com, quintela@redhat.com, mrhines@us.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com On 09/03/2013 10:32 PM, Isaku Yamahata wrote: > Signed-off-by: Isaku Yamahata > --- > migration-rdma.c | 23 +++++++---------------- > 1 file changed, 7 insertions(+), 16 deletions(-) > > diff --git a/migration-rdma.c b/migration-rdma.c > index db5a908..941c07e 100644 > --- a/migration-rdma.c > +++ b/migration-rdma.c > @@ -1128,8 +1128,7 @@ static int qemu_rdma_search_ram_block(RDMAContext *rdma, > */ > static int qemu_rdma_register_and_get_keys(RDMAContext *rdma, > RDMALocalBlock *block, uint8_t *host_addr, > - uint32_t *lkey, uint32_t *rkey, int chunk, > - uint8_t *chunk_start, uint8_t *chunk_end) > + uint32_t *lkey, uint32_t *rkey, int chunk) > { > if (block->mr) { > if (lkey) { > @@ -1155,6 +1154,8 @@ static int qemu_rdma_register_and_get_keys(RDMAContext *rdma, > * If 'lkey', then we're the source VM, so grant access only to ourselves. > */ > if (!block->pmr[chunk]) { > + uint8_t *chunk_start = ram_chunk_start(block, chunk); > + uint8_t *chunk_end = ram_chunk_end(block, chunk); > uint64_t len = chunk_end - chunk_start; NACK. chunk_end cannot be calculated like this for all calls to this function. See below....... > > DDPRINTF("Registering %" PRIu64 " bytes @ %p\n", > @@ -1849,7 +1850,6 @@ static int qemu_rdma_write_one(QEMUFile *f, RDMAContext *rdma, > struct ibv_send_wr *bad_wr; > int reg_result_idx, ret, count = 0; > uint64_t chunk, chunks; > - uint8_t *chunk_start, *chunk_end; > RDMALocalBlock *block = &(rdma->local_ram_blocks.block[current_index]); > RDMARegister reg; > RDMARegisterResult *reg_result; > @@ -1865,7 +1865,6 @@ retry: > sge.length = length; > > chunk = ram_chunk_index(block->local_host_addr, (uint8_t *) sge.addr); > - chunk_start = ram_chunk_start(block, chunk); > > if (block->is_ram_block) { > chunks = length / (1UL << RDMA_REG_CHUNK_SHIFT); > @@ -1884,8 +1883,6 @@ retry: > DDPRINTF("Writing %" PRIu64 " chunks, (%" PRIu64 " MB)\n", > chunks + 1, (chunks + 1) * (1UL << RDMA_REG_CHUNK_SHIFT) / 1024 / 1024); > > - chunk_end = ram_chunk_end(block, chunk + chunks); > - NACK. The value of chunk_end changes based on whether or not the value of block->is_ram_block is true of false. > if (!rdma->pin_all) { > #ifdef RDMA_UNREGISTRATION_EXAMPLE > qemu_rdma_unregister_waiting(rdma); > @@ -1974,8 +1971,7 @@ retry: > /* try to overlap this single registration with the one we sent. */ > if (qemu_rdma_register_and_get_keys(rdma, block, > (uint8_t *) sge.addr, > - &sge.lkey, NULL, chunk, > - chunk_start, chunk_end)) { > + &sge.lkey, NULL, chunk)) { > fprintf(stderr, "cannot get lkey!\n"); > return -EINVAL; > } > @@ -1995,8 +1991,7 @@ retry: > /* already registered before */ > if (qemu_rdma_register_and_get_keys(rdma, block, > (uint8_t *)sge.addr, > - &sge.lkey, NULL, chunk, > - chunk_start, chunk_end)) { > + &sge.lkey, NULL, chunk)) { > fprintf(stderr, "cannot get lkey!\n"); > return -EINVAL; > } > @@ -2007,8 +2002,7 @@ retry: > send_wr.wr.rdma.rkey = block->remote_rkey; > > if (qemu_rdma_register_and_get_keys(rdma, block, (uint8_t *)sge.addr, > - &sge.lkey, NULL, chunk, > - chunk_start, chunk_end)) { > + &sge.lkey, NULL, chunk)) { > fprintf(stderr, "cannot get lkey!\n"); > return -EINVAL; > } > @@ -3054,7 +3048,6 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque, > > for (count = 0; count < head.repeat; count++) { > uint64_t chunk; > - uint8_t *chunk_start, *chunk_end; > > reg = ®isters[count]; > network_to_register(reg); > @@ -3076,11 +3069,9 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque, > host_addr = block->local_host_addr + > (reg->key.chunk * (1UL << RDMA_REG_CHUNK_SHIFT)); > } > - chunk_start = ram_chunk_start(block, chunk); > - chunk_end = ram_chunk_end(block, chunk + reg->chunks); > if (qemu_rdma_register_and_get_keys(rdma, block, > (uint8_t *)host_addr, NULL, ®_result->rkey, > - chunk, chunk_start, chunk_end)) { > + chunk)) { > fprintf(stderr, "cannot get rkey!\n"); > ret = -EINVAL; > goto out;