Message ID | 20230117220914.2062125-9-peterx@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | migration: Support hugetlb doublemaps | expand |
* Peter Xu (peterx@redhat.com) wrote: > We do proper page size alignment for file backed mmap()s for ramblocks. > Even if it's as simple as that, cache the value because it'll be used in > multiple places. > > Since at it, drop size for file_ram_alloc() and just use max_length because > that's always true for file-backed ramblocks. Having a length previously called 'memory' was a bit odd! > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > include/exec/ramblock.h | 2 ++ > softmmu/physmem.c | 14 +++++++------- > 2 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h > index 76cd0812c8..3f31ce1591 100644 > --- a/include/exec/ramblock.h > +++ b/include/exec/ramblock.h > @@ -32,6 +32,8 @@ struct RAMBlock { > ram_addr_t offset; > ram_addr_t used_length; > ram_addr_t max_length; > + /* Only used for file-backed ramblocks */ > + ram_addr_t mmap_length; > void (*resized)(const char*, uint64_t length, void *host); > uint32_t flags; > /* Protected by iothread lock. */ > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > index aa1a7466e5..b5be02f1cb 100644 > --- a/softmmu/physmem.c > +++ b/softmmu/physmem.c > @@ -1533,7 +1533,6 @@ static int file_ram_open(const char *path, > } > > static void *file_ram_alloc(RAMBlock *block, > - ram_addr_t memory, > int fd, > bool readonly, > bool truncate, > @@ -1563,14 +1562,14 @@ static void *file_ram_alloc(RAMBlock *block, > } > #endif > > - if (memory < block->page_size) { > + if (block->max_length < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > "or larger than page size 0x%zx", > - memory, block->page_size); > + block->max_length, block->page_size); > return NULL; > } > > - memory = ROUND_UP(memory, block->page_size); > + block->mmap_length = ROUND_UP(block->max_length, block->page_size); > > /* > * ftruncate is not supported by hugetlbfs in older > @@ -1586,7 +1585,7 @@ static void *file_ram_alloc(RAMBlock *block, > * those labels. Therefore, extending the non-empty backend file > * is disabled as well. > */ > - if (truncate && ftruncate(fd, memory)) { > + if (truncate && ftruncate(fd, block->mmap_length)) { > perror("ftruncate"); > } > > @@ -1594,7 +1593,8 @@ static void *file_ram_alloc(RAMBlock *block, > qemu_map_flags |= (block->flags & RAM_SHARED) ? QEMU_MAP_SHARED : 0; > qemu_map_flags |= (block->flags & RAM_PMEM) ? QEMU_MAP_SYNC : 0; > qemu_map_flags |= (block->flags & RAM_NORESERVE) ? QEMU_MAP_NORESERVE : 0; > - area = qemu_ram_mmap(fd, memory, block->mr->align, qemu_map_flags, offset); > + area = qemu_ram_mmap(fd, block->mmap_length, block->mr->align, > + qemu_map_flags, offset); > if (area == MAP_FAILED) { > error_setg_errno(errp, errno, > "unable to map backing store for guest RAM"); > @@ -2100,7 +2100,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > new_block->used_length = size; > new_block->max_length = size; > new_block->flags = ram_flags; > - new_block->host = file_ram_alloc(new_block, size, fd, readonly, > + new_block->host = file_ram_alloc(new_block, fd, readonly, > !file_size, offset, errp); > if (!new_block->host) { > g_free(new_block); > -- > 2.37.3 >
On Mon, Jan 23, 2023 at 06:51:51PM +0000, Dr. David Alan Gilbert wrote: > * Peter Xu (peterx@redhat.com) wrote: > > We do proper page size alignment for file backed mmap()s for ramblocks. > > Even if it's as simple as that, cache the value because it'll be used in > > multiple places. > > > > Since at it, drop size for file_ram_alloc() and just use max_length because > > that's always true for file-backed ramblocks. > > Having a length previously called 'memory' was a bit odd! :-D > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thanks,
Peter Xu <peterx@redhat.com> wrote: > We do proper page size alignment for file backed mmap()s for ramblocks. > Even if it's as simple as that, cache the value because it'll be used in > multiple places. > > Since at it, drop size for file_ram_alloc() and just use max_length because > that's always true for file-backed ramblocks. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> > @@ -2100,7 +2100,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > new_block->used_length = size; > new_block->max_length = size; > new_block->flags = ram_flags; > - new_block->host = file_ram_alloc(new_block, size, fd, readonly, > + new_block->host = file_ram_alloc(new_block, fd, readonly, > !file_size, offset, errp); > if (!new_block->host) { > g_free(new_block); Passing "size" in three places, not bad at all O:-)
On Mon, Jan 30, 2023 at 06:05:47AM +0100, Juan Quintela wrote: > Peter Xu <peterx@redhat.com> wrote: > > We do proper page size alignment for file backed mmap()s for ramblocks. > > Even if it's as simple as that, cache the value because it'll be used in > > multiple places. > > > > Since at it, drop size for file_ram_alloc() and just use max_length because > > that's always true for file-backed ramblocks. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > Reviewed-by: Juan Quintela <quintela@redhat.com> Thanks for reviewing the set! > > > @@ -2100,7 +2100,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > > new_block->used_length = size; > > new_block->max_length = size; > > new_block->flags = ram_flags; > > - new_block->host = file_ram_alloc(new_block, size, fd, readonly, > > + new_block->host = file_ram_alloc(new_block, fd, readonly, > > !file_size, offset, errp); > > if (!new_block->host) { > > g_free(new_block); > > Passing "size" in three places, not bad at all O:-) Yes it's a bit unfortunate. :(
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h index 76cd0812c8..3f31ce1591 100644 --- a/include/exec/ramblock.h +++ b/include/exec/ramblock.h @@ -32,6 +32,8 @@ struct RAMBlock { ram_addr_t offset; ram_addr_t used_length; ram_addr_t max_length; + /* Only used for file-backed ramblocks */ + ram_addr_t mmap_length; void (*resized)(const char*, uint64_t length, void *host); uint32_t flags; /* Protected by iothread lock. */ diff --git a/softmmu/physmem.c b/softmmu/physmem.c index aa1a7466e5..b5be02f1cb 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1533,7 +1533,6 @@ static int file_ram_open(const char *path, } static void *file_ram_alloc(RAMBlock *block, - ram_addr_t memory, int fd, bool readonly, bool truncate, @@ -1563,14 +1562,14 @@ static void *file_ram_alloc(RAMBlock *block, } #endif - if (memory < block->page_size) { + if (block->max_length < block->page_size) { error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " "or larger than page size 0x%zx", - memory, block->page_size); + block->max_length, block->page_size); return NULL; } - memory = ROUND_UP(memory, block->page_size); + block->mmap_length = ROUND_UP(block->max_length, block->page_size); /* * ftruncate is not supported by hugetlbfs in older @@ -1586,7 +1585,7 @@ static void *file_ram_alloc(RAMBlock *block, * those labels. Therefore, extending the non-empty backend file * is disabled as well. */ - if (truncate && ftruncate(fd, memory)) { + if (truncate && ftruncate(fd, block->mmap_length)) { perror("ftruncate"); } @@ -1594,7 +1593,8 @@ static void *file_ram_alloc(RAMBlock *block, qemu_map_flags |= (block->flags & RAM_SHARED) ? QEMU_MAP_SHARED : 0; qemu_map_flags |= (block->flags & RAM_PMEM) ? QEMU_MAP_SYNC : 0; qemu_map_flags |= (block->flags & RAM_NORESERVE) ? QEMU_MAP_NORESERVE : 0; - area = qemu_ram_mmap(fd, memory, block->mr->align, qemu_map_flags, offset); + area = qemu_ram_mmap(fd, block->mmap_length, block->mr->align, + qemu_map_flags, offset); if (area == MAP_FAILED) { error_setg_errno(errp, errno, "unable to map backing store for guest RAM"); @@ -2100,7 +2100,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, new_block->used_length = size; new_block->max_length = size; new_block->flags = ram_flags; - new_block->host = file_ram_alloc(new_block, size, fd, readonly, + new_block->host = file_ram_alloc(new_block, fd, readonly, !file_size, offset, errp); if (!new_block->host) { g_free(new_block);
We do proper page size alignment for file backed mmap()s for ramblocks. Even if it's as simple as that, cache the value because it'll be used in multiple places. Since at it, drop size for file_ram_alloc() and just use max_length because that's always true for file-backed ramblocks. Signed-off-by: Peter Xu <peterx@redhat.com> --- include/exec/ramblock.h | 2 ++ softmmu/physmem.c | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-)