| Message ID | 20211216091332.25681-4-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: misc cleanups | expand |
On 12/16/21 10:13, Juan Quintela wrote: > Remove the pages argument. And s/pages/page/ > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/ram.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > -static void ram_release_pages(const char *rbname, uint64_t offset, int pages) > +static void ram_release_page(const char *rbname, uint64_t offset) > { > if (!migrate_release_ram() || !migration_in_postcopy()) { > return; > } > > - ram_discard_range(rbname, offset, ((ram_addr_t)pages) << TARGET_PAGE_BITS); > + ram_discard_range(rbname, offset, ((ram_addr_t)1) << TARGET_PAGE_BITS); 1ULL? Otherwise, Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 12/16/21 10:13, Juan Quintela wrote: >> Remove the pages argument. And s/pages/page/ >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> migration/ram.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> -static void ram_release_pages(const char *rbname, uint64_t offset, int pages) >> +static void ram_release_page(const char *rbname, uint64_t offset) >> { >> if (!migrate_release_ram() || !migration_in_postcopy()) { >> return; >> } >> >> - ram_discard_range(rbname, offset, ((ram_addr_t)pages) << TARGET_PAGE_BITS); >> + ram_discard_range(rbname, offset, ((ram_addr_t)1) << TARGET_PAGE_BITS); > > 1ULL? I am changing it, but the argument to ram_discard_range is a size_t, and that is different in 32 bits arch. Once told that, it is not worse that what we have here, as ram_addr_t type depends on the phase of the moon. /* address in the RAM (different from a physical address) */ #if defined(CONFIG_XEN_BACKEND) typedef uint64_t ram_addr_t; # define RAM_ADDR_MAX UINT64_MAX # define RAM_ADDR_FMT "%" PRIx64 #else typedef uintptr_t ram_addr_t; # define RAM_ADDR_MAX UINTPTR_MAX # define RAM_ADDR_FMT "%" PRIxPTR #endif Later, Juan. PD. No, I don't know either why it is not casted to size_t. PD2. And yes, I still think that pure int operations should be ok. The value TARGET_PAGE_BITS more typical is 10, and here pages is only used with value 1. C promotion rules should make everything ok (famous last words).
diff --git a/migration/ram.c b/migration/ram.c index 7223b0d8ca..8d29d64b35 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1204,13 +1204,13 @@ static int save_zero_page(RAMState *rs, RAMBlock *block, ram_addr_t offset) return -1; } -static void ram_release_pages(const char *rbname, uint64_t offset, int pages) +static void ram_release_page(const char *rbname, uint64_t offset) { if (!migrate_release_ram() || !migration_in_postcopy()) { return; } - ram_discard_range(rbname, offset, ((ram_addr_t)pages) << TARGET_PAGE_BITS); + ram_discard_range(rbname, offset, ((ram_addr_t)1) << TARGET_PAGE_BITS); } /* @@ -1365,7 +1365,7 @@ static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block, } exit: - ram_release_pages(block->idstr, offset & TARGET_PAGE_MASK, 1); + ram_release_page(block->idstr, offset & TARGET_PAGE_MASK); return zero_page; } @@ -2153,7 +2153,7 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss) xbzrle_cache_zero_page(rs, block->offset + offset); XBZRLE_cache_unlock(); } - ram_release_pages(block->idstr, offset, res); + ram_release_page(block->idstr, offset); return res; }
Remove the pages argument. And s/pages/page/ Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)