qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Stefan Weil <sw@weilnetz.de>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	Amit Shah <amit.shah@redhat.com>,
	QEMU Developer <qemu-devel@nongnu.org>,
	Juan Quintela <quintela@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] migration: Fix remaining 32 bit compiler errors
Date: Wed, 4 Mar 2015 13:31:20 +0000	[thread overview]
Message-ID: <20150304133120.GG2530@work-vm> (raw)
In-Reply-To: <1425146983-16015-4-git-send-email-sw@weilnetz.de>

* Stefan Weil (sw@weilnetz.de) wrote:
> Fix type casts between pointers and 64 bit integers.
> Now 32 bit builds are possible again.
> 
> Signed-off-by: Stefan Weil <sw@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.

>      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.

>          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.

(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 / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2015-03-04 13:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-28 18:09 [Qemu-devel] migration: Fix 32 bit compiler errors Stefan Weil
2015-02-28 18:09 ` [Qemu-devel] [PATCH 1/3] migration: Fix coding style (whitespace issues) Stefan Weil
2015-03-02 12:15   ` Michael Tokarev
2015-03-04 12:20   ` Dr. David Alan Gilbert
2015-03-17 12:42   ` Juan Quintela
2015-02-28 18:09 ` [Qemu-devel] [PATCH 2/3] migration: Fix some 32 bit compiler errors Stefan Weil
2015-03-04 12:44   ` Dr. David Alan Gilbert
2015-03-04 21:00     ` Stefan Weil
2015-03-05 10:28       ` Dr. David Alan Gilbert
2015-03-17 13:10   ` Juan Quintela
2015-02-28 18:09 ` [Qemu-devel] [PATCH 3/3] migration: Fix remaining " Stefan Weil
2015-03-04 13:31   ` Dr. David Alan Gilbert [this message]
2015-03-04 20:45     ` Stefan Weil
2015-03-05 10:10       ` Dr. David Alan Gilbert
2015-03-17 13:11   ` Juan Quintela
2015-03-02  5:58 ` [Qemu-devel] migration: Fix " Amit Shah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150304133120.GG2530@work-vm \
    --to=dgilbert@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).