* Stefan Weil (s...@weilnetz.de) wrote:
> Please see my comments below.
>
> Am 04.03.2015 um 14:31 schrieb Dr. David Alan Gilbert:
> >* Stefan Weil (s...@weilnetz.de) wrote:
> >>Fix type casts between pointers and 64 bit integers.
> >>Now 32 bit builds are possible again.
> >>
> >>Signed-off-by: Stefan Weil <s...@weilnetz.de>
> >>---
> >> migration/rdma.c | 57
> >> +++++++++++++++++++++++++++++-------------------------
> >> 1 file changed, 31 insertions(+), 26 deletions(-)
> >>
> >>diff --git a/migration/rdma.c b/migration/rdma.c
> >>index 1512460..a68f1f1 100644
> >>--- a/migration/rdma.c
> >>+++ b/migration/rdma.c
> >>@@ -493,8 +493,8 @@ static inline uint64_t ram_chunk_index(const uint8_t
> >>*start,
> >> static inline uint8_t *ram_chunk_start(const RDMALocalBlock
> >> *rdma_ram_block,
> >> uint64_t i)
> >> {
> >>- return (uint8_t *) (((uintptr_t) rdma_ram_block->local_host_addr)
> >>- + (i << RDMA_REG_CHUNK_SHIFT));
> >>+ return (uint8_t *)(uintptr_t)(rdma_ram_block->local_host_addr +
> >>+ (i << RDMA_REG_CHUNK_SHIFT));
> >> }
> >> static inline uint8_t *ram_chunk_end(const RDMALocalBlock *rdma_ram_block,
> >>@@ -515,7 +515,7 @@ static int __qemu_rdma_add_block(RDMAContext *rdma,
> >>void *host_addr,
> >> {
> >This will clash with my 'unbreak dtrace tracing due to double _ in rdma
> >names' that's
> >going through Stefan's tree.
>
> Then this part will have to wait until Stefan's tree was applied.
>
> >
> >> RDMALocalBlocks *local = &rdma->local_ram_blocks;
> >> RDMALocalBlock *block = g_hash_table_lookup(rdma->blockmap,
> >>- (void *) block_offset);
> >>+ (void *)(uintptr_t)block_offset);
> >> RDMALocalBlock *old = local->block;
> >
> >> assert(block == NULL);
> >>@@ -526,9 +526,11 @@ static int __qemu_rdma_add_block(RDMAContext *rdma,
> >>void *host_addr,
> >> int x;
> >> for (x = 0; x < local->nb_blocks; x++) {
> >>- g_hash_table_remove(rdma->blockmap, (void *)old[x].offset);
> >>- g_hash_table_insert(rdma->blockmap, (void *)old[x].offset,
> >>- &local->block[x]);
> >>+ g_hash_table_remove(rdma->blockmap,
> >>+ (void *)(uintptr_t)old[x].offset);
> >>+ g_hash_table_insert(rdma->blockmap,
> >>+ (void *)(uintptr_t)old[x].offset,
> >>+ &local->block[x]);
> >> }
> >> memcpy(local->block, old, sizeof(RDMALocalBlock) *
> >> local->nb_blocks);
> >> g_free(old);
> >>@@ -549,12 +551,12 @@ static int __qemu_rdma_add_block(RDMAContext *rdma,
> >>void *host_addr,
> >> block->is_ram_block = local->init ? false : true;
> >>- g_hash_table_insert(rdma->blockmap, (void *) block_offset, block);
> >>+ g_hash_table_insert(rdma->blockmap, (void *)(uintptr_t)block_offset,
> >>block);
> >> trace___qemu_rdma_add_block(local->nb_blocks,
> >>- (uint64_t) block->local_host_addr,
> >>block->offset,
> >>+ (uintptr_t)block->local_host_addr,
> >>block->offset,
> >> block->length,
> >>- (uint64_t) (block->local_host_addr +
> >>block->length),
> >>+ (uintptr_t)(block->local_host_addr +
> >>block->length),
> >> BITS_TO_LONGS(block->nb_chunks) *
> >> sizeof(unsigned long) * 8,
> >> block->nb_chunks);
> >>@@ -595,7 +597,7 @@ static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
> >> return 0;
> >> }
> >>-static int __qemu_rdma_delete_block(RDMAContext *rdma, ram_addr_t
> >>block_offset)
> >>+static int __qemu_rdma_delete_block(RDMAContext *rdma, uintptr_t
> >>block_offset)
> >> {
> >> RDMALocalBlocks *local = &rdma->local_ram_blocks;
> >> RDMALocalBlock *block = g_hash_table_lookup(rdma->blockmap,
> >>@@ -635,7 +637,7 @@ static int __qemu_rdma_delete_block(RDMAContext *rdma,
> >>ram_addr_t block_offset)
> >> block->remote_keys = NULL;
> >> for (x = 0; x < local->nb_blocks; x++) {
> >>- g_hash_table_remove(rdma->blockmap, (void *)old[x].offset);
> >>+ g_hash_table_remove(rdma->blockmap, (void
> >>*)(uintptr_t)old[x].offset);
> >> }
> >> if (local->nb_blocks > 1) {
> >>@@ -658,9 +660,9 @@ static int __qemu_rdma_delete_block(RDMAContext *rdma,
> >>ram_addr_t block_offset)
> >> }
> >> trace___qemu_rdma_delete_block(local->nb_blocks,
> >>- (uint64_t)block->local_host_addr,
> >>+ (uintptr_t)block->local_host_addr,
> >> block->offset, block->length,
> >>- (uint64_t)(block->local_host_addr +
> >>block->length),
> >>+ (uintptr_t)(block->local_host_addr +
> >>block->length),
> >> BITS_TO_LONGS(block->nb_chunks) *
> >> sizeof(unsigned long) * 8,
> >> block->nb_chunks);
> >>@@ -670,8 +672,9 @@ static int __qemu_rdma_delete_block(RDMAContext *rdma,
> >>ram_addr_t block_offset)
> >> if (local->nb_blocks) {
> >> for (x = 0; x < local->nb_blocks; x++) {
> >>- g_hash_table_insert(rdma->blockmap, (void
> >>*)local->block[x].offset,
> >>- &local->block[x]);
> >>+ g_hash_table_insert(rdma->blockmap,
> >>+ (void *)(uintptr_t)local->block[x].offset,
> >>+ &local->block[x]);
> >> }
> >> }
> >>@@ -1076,7 +1079,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext
> >>*rdma)
> >> * This search cannot fail or the migration will fail.
> >> */
> >> static int qemu_rdma_search_ram_block(RDMAContext *rdma,
> >>- uint64_t block_offset,
> >>+ uintptr_t block_offset,
> >> uint64_t offset,
> >> uint64_t length,
> >> uint64_t *block_index,
> >>@@ -1380,8 +1383,8 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma,
> >>uint64_t *wr_id_out,
> >> RDMALocalBlock *block = &(rdma->local_ram_blocks.block[index]);
> >> trace_qemu_rdma_poll_write(print_wrid(wr_id), wr_id,
> >> rdma->nb_sent,
> >>- index, chunk,
> >>- block->local_host_addr, (void *)block->remote_host_addr);
> >>+ index, chunk, block->local_host_addr,
> >>+ (void
> >>*)(uintptr_t)block->remote_host_addr);
> >I think that's the wrong fix there; remote_host_addr is actually 64bit
> >because
> >even if this host is 32bit the other end might be 64bit; I think the right
> >fix is to change the trace to use a uint64_t for the remote_host_addr.
>
> That depends on the kind of output which is wanted.
>
> We can either use %p for pointers, then the output will depend on the host's
> pointer size.
> Or we can use %16 PRIu64, then the output will always show 16 digits (with
> at least 8
> leading zeros on 32 bit hosts).
Since it's trying to print the remote_host_addr and that really can be 64bit
it doesn't seem right to use a %p since that's dependent on the local pointer
size. I'd use a % PRIx64.
> >> clear_bit(chunk, block->transit_bitmap);
> >>@@ -1524,7 +1527,7 @@ static int qemu_rdma_post_send_control(RDMAContext
> >>*rdma, uint8_t *buf,
> >> RDMAWorkRequestData *wr = &rdma->wr_data[RDMA_WRID_CONTROL];
> >> struct ibv_send_wr *bad_wr;
> >> struct ibv_sge sge = {
> >>- .addr = (uint64_t)(wr->control),
> >>+ .addr = (uintptr_t)(wr->control),
> >No, as before, my belief is that the .addr is a uint64_t.
>
> Yes, because wr->control is a pointer which you want to convert to an
> integer value.
> C allows assignments from 32 bit integers to 64 bit integers - the compiler
> will add the
> necessary 0 bits.
Yes, fair enough.
Dave
> >(Although curiously I did a test and found that gcc lets me pass a uint64_t
> >to a function that has a uintptr_t parameter on a 32bit machine, but I don't
> >understand why.)
> >
> >Dave
> >
> >> .length = head->len +
> >> sizeof(RDMAControlHeader),
> >> .lkey = wr->control_mr->lkey,
> >> };
> >>@@ -1578,7 +1581,7 @@ static int qemu_rdma_post_recv_control(RDMAContext
> >>*rdma, int idx)
> >> {
> >> struct ibv_recv_wr *bad_wr;
> >> struct ibv_sge sge = {
> >>- .addr = (uint64_t)(rdma->wr_data[idx].control),
> >>+ .addr =
> >>(uintptr_t)(rdma->wr_data[idx].control),
> >> .length = RDMA_CONTROL_MAX_BUFFER,
> >> .lkey = rdma->wr_data[idx].control_mr->lkey,
> >> };
> >>@@ -1825,11 +1828,12 @@ static int qemu_rdma_write_one(QEMUFile *f,
> >>RDMAContext *rdma,
> >> };
> >> retry:
> >>- sge.addr = (uint64_t)(block->local_host_addr +
> >>+ sge.addr = (uintptr_t)(block->local_host_addr +
> >> (current_addr - block->offset));
> >> sge.length = length;
> >>- chunk = ram_chunk_index(block->local_host_addr, (uint8_t *) sge.addr);
> >>+ chunk = ram_chunk_index(block->local_host_addr,
> >>+ (uint8_t *)(uintptr_t)sge.addr);
> >> chunk_start = ram_chunk_start(block, chunk);
> >> if (block->is_ram_block) {
> >>@@ -1882,8 +1886,9 @@ retry:
> >> * memset() + madvise() the entire chunk without RDMA.
> >> */
> >>- if (can_use_buffer_find_nonzero_offset((void *)sge.addr,
> >>length)
> >>- && buffer_find_nonzero_offset((void *)sge.addr,
> >>+ if (can_use_buffer_find_nonzero_offset((void
> >>*)(uintptr_t)sge.addr,
> >>+ length)
> >>+ && buffer_find_nonzero_offset((void
> >>*)(uintptr_t)sge.addr,
> >> length) == length) {
> >> RDMACompress comp = {
> >> .offset = current_addr,
> >>@@ -2978,7 +2983,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f,
> >>void *opaque,
> >> */
> >> for (i = 0; i < local->nb_blocks; i++) {
> >> rdma->block[i].remote_host_addr =
> >>- (uint64_t)(local->block[i].local_host_addr);
> >>+ (uintptr_t)(local->block[i].local_host_addr);
> >> if (rdma->pin_all) {
> >> rdma->block[i].remote_rkey = local->block[i].mr->rkey;
> >>@@ -3042,7 +3047,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f,
> >>void *opaque,
> >> goto out;
> >> }
> >>- reg_result->host_addr = (uint64_t) block->local_host_addr;
> >>+ reg_result->host_addr = (uintptr_t)block->local_host_addr;
> >> trace_qemu_rdma_registration_handle_register_rkey(
> >>
> >> reg_result->rkey);
> >>--
> >>1.7.10.4
> >>
> >>
> >--
> >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
