From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49752) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z37Lo-0003nk-5I for qemu-devel@nongnu.org; Thu, 11 Jun 2015 14:37:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z37Lk-0002vO-VK for qemu-devel@nongnu.org; Thu, 11 Jun 2015 14:37:16 -0400 Received: from e18.ny.us.ibm.com ([129.33.205.208]:50583) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z37Lk-0002vJ-Qd for qemu-devel@nongnu.org; Thu, 11 Jun 2015 14:37:12 -0400 Received: from /spool/local by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Jun 2015 14:37:12 -0400 Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 2193938C8026 for ; Thu, 11 Jun 2015 14:37:07 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t5BIb6R556033408 for ; Thu, 11 Jun 2015 18:37:06 GMT Received: from d01av02.pok.ibm.com (localhost [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t5BIb6UY029138 for ; Thu, 11 Jun 2015 14:37:06 -0400 Message-ID: <5579D533.9070608@linux.vnet.ibm.com> Date: Thu, 11 Jun 2015 13:36:35 -0500 From: "Michael R. Hines" MIME-Version: 1.0 References: <1434043048-4444-1-git-send-email-dgilbert@redhat.com> <1434043048-4444-9-git-send-email-dgilbert@redhat.com> In-Reply-To: <1434043048-4444-9-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 08/12] Allow rdma_delete_block to work without the hash List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: amit.shah@redhat.com, arei.gonglei@huawei.com, mrhines@us.ibm.com, quintela@redhat.com On 06/11/2015 12:17 PM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > In the next patch we remove the hash on the destination, > rdma_delete_block does two things with the hash which can be avoided: > a) The caller passes the offset and rdma_delete_block looks it up > in the hash; fixed by getting the caller to pass the block > b) The hash gets recreated after deletion; fixed by making that > conditional on the hash being initialised. > > While this function is currently only used during cleanup, Michael > asked that we keep it general for future dynamic block registration > work. > > Signed-off-by: Dr. David Alan Gilbert > --- > migration/rdma.c | 27 ++++++++++++++++----------- > trace-events | 2 +- > 2 files changed, 17 insertions(+), 12 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index 396329c..8d99378 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -617,16 +617,19 @@ static int qemu_rdma_init_ram_blocks(RDMAContext *rdma) > return 0; > } > > -static int rdma_delete_block(RDMAContext *rdma, ram_addr_t block_offset) > +/* > + * Note: If used outside of cleanup, the caller must ensure that the destination > + * block structures are also updated > + */ > +static int rdma_delete_block(RDMAContext *rdma, RDMALocalBlock *block) > { > RDMALocalBlocks *local = &rdma->local_ram_blocks; > - RDMALocalBlock *block = g_hash_table_lookup(rdma->blockmap, > - (void *) block_offset); > RDMALocalBlock *old = local->block; > int x; > > - assert(block); > - > + if (rdma->blockmap) { > + g_hash_table_remove(rdma->blockmap, (void *)(uintptr_t)block->offset); > + } > if (block->pmr) { > int j; > > @@ -659,8 +662,11 @@ static int rdma_delete_block(RDMAContext *rdma, ram_addr_t block_offset) > g_free(block->block_name); > block->block_name = NULL; > > - for (x = 0; x < local->nb_blocks; x++) { > - g_hash_table_remove(rdma->blockmap, (void *)(uintptr_t)old[x].offset); > + if (rdma->blockmap) { > + for (x = 0; x < local->nb_blocks; x++) { > + g_hash_table_remove(rdma->blockmap, > + (void *)(uintptr_t)old[x].offset); > + } > } > > if (local->nb_blocks > 1) { > @@ -682,8 +688,7 @@ static int rdma_delete_block(RDMAContext *rdma, ram_addr_t block_offset) > local->block = NULL; > } > > - trace_rdma_delete_block(local->nb_blocks, > - (uintptr_t)block->local_host_addr, > + trace_rdma_delete_block(block, (uintptr_t)block->local_host_addr, > block->offset, block->length, > (uintptr_t)(block->local_host_addr + block->length), > BITS_TO_LONGS(block->nb_chunks) * > @@ -693,7 +698,7 @@ static int rdma_delete_block(RDMAContext *rdma, ram_addr_t block_offset) > > local->nb_blocks--; > > - if (local->nb_blocks) { > + if (local->nb_blocks && rdma->blockmap) { > for (x = 0; x < local->nb_blocks; x++) { > g_hash_table_insert(rdma->blockmap, > (void *)(uintptr_t)local->block[x].offset, > @@ -2214,7 +2219,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > > if (rdma->local_ram_blocks.block) { > while (rdma->local_ram_blocks.nb_blocks) { > - rdma_delete_block(rdma, rdma->local_ram_blocks.block->offset); > + rdma_delete_block(rdma, &rdma->local_ram_blocks.block[0]); > } > } Looks good overall. Maybe this is a silly question, but have you done a few migrations over actual RDMA hardware yet? Reviewed-by: Michael R. Hines > diff --git a/trace-events b/trace-events > index 0f37a4b..7dff362 100644 > --- a/trace-events > +++ b/trace-events > @@ -1452,7 +1452,7 @@ qemu_rdma_write_one_sendreg(uint64_t chunk, int len, int index, int64_t offset) > qemu_rdma_write_one_top(uint64_t chunks, uint64_t size) "Writing %" PRIu64 " chunks, (%" PRIu64 " MB)" > qemu_rdma_write_one_zero(uint64_t chunk, int len, int index, int64_t offset) "Entire chunk is zero, sending compress: %" PRIu64 " for %d bytes, index: %d, offset: %" PRId64 > rdma_add_block(const char *block_name, int block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Added Block: '%s':%d, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d" > -rdma_delete_block(int block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Deleted Block: %d, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d" > +rdma_delete_block(void *block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Deleted Block: %p, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d" > rdma_start_incoming_migration(void) "" > rdma_start_incoming_migration_after_dest_init(void) "" > rdma_start_incoming_migration_after_rdma_listen(void) "" These messages are also empty? What happened to them? =)