[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 02/42] exec: Fix memory allocation when memor
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v2 02/42] exec: Fix memory allocation when memory path isn't on hugetlbfs |
Date: |
Tue, 8 Mar 2016 15:17:29 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 07/03/2016 20:25, Markus Armbruster wrote:
> gethugepagesize() works reliably only when its argument is on
> hugetlbfs. When it's not, it returns the filesystem's "optimal
> transfer block size", which may or may not be the actual page size
> you'll get when you mmap().
>
> If the value is too small or not a power of two, we fail
> qemu_ram_mmap()'s assertions. These were added in commit 794e8f3
> (v2.5.0). The bug's impact before that is currently unknown. Seems
> fairly unlikely at least when the normal page size is 4KiB.
>
> Else, if the value is too large, we align more strictly than
> necessary.
>
> gethugepagesize() goes back to commit c902760 (v0.13). That commit
> clearly intended gethugepagesize() to be used on hugetlbfs only. Not
> only was it named accordingly, it also printed a warning when used on
> anything else. However, the commit neglected to spell out the
> restriction in user documentation of -mem-path.
>
> Commit bfc2a1a (v2.5.0) dropped the warning as bogus "because QEMU
> functions perfectly well with the path on a regular tmpfs filesystem".
> It sure does when you're sufficiently lucky. In my testing, I was
> lucky, too.
>
> Fix by switching to qemu_fd_getpagesize(). Rename the variable
> holding its result from hpagesize to page_size.
>
> Cc: Paolo Bonzini <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> exec.c | 40 +++++++---------------------------------
> 1 file changed, 7 insertions(+), 33 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 5275ff4..d41194e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1207,27 +1207,6 @@ void qemu_mutex_unlock_ramlist(void)
> }
>
> #ifdef __linux__
> -
> -#include <sys/vfs.h>
> -
> -#define HUGETLBFS_MAGIC 0x958458f6
> -
> -static long gethugepagesize(int fd)
> -{
> - struct statfs fs;
> - int ret;
> -
> - do {
> - ret = fstatfs(fd, &fs);
> - } while (ret != 0 && errno == EINTR);
> -
> - if (ret != 0) {
> - return -1;
> - }
> -
> - return fs.f_bsize;
> -}
> -
> static void *file_ram_alloc(RAMBlock *block,
> ram_addr_t memory,
> const char *path,
> @@ -1239,7 +1218,7 @@ static void *file_ram_alloc(RAMBlock *block,
> char *c;
> void *area;
> int fd;
> - int64_t hpagesize;
> + int64_t page_size;
>
> if (kvm_enabled() && !kvm_has_sync_mmu()) {
> error_setg(errp,
> @@ -1294,22 +1273,17 @@ static void *file_ram_alloc(RAMBlock *block,
> */
> }
>
> - hpagesize = gethugepagesize(fd);
> - if (hpagesize < 0) {
> - error_setg_errno(errp, errno, "can't get page size for %s",
> - path);
> - goto error;
> - }
> - block->mr->align = hpagesize;
> + page_size = qemu_fd_getpagesize(fd);
> + block->mr->align = page_size;
>
> - if (memory < hpagesize) {
> + if (memory < page_size) {
> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
> "or larger than page size 0x%" PRIx64,
> - memory, hpagesize);
> + memory, page_size);
> goto error;
> }
>
> - memory = ROUND_UP(memory, hpagesize);
> + memory = ROUND_UP(memory, page_size);
>
> /*
> * ftruncate is not supported by hugetlbfs in older
> @@ -1321,7 +1295,7 @@ static void *file_ram_alloc(RAMBlock *block,
> perror("ftruncate");
> }
>
> - area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED);
> + area = qemu_ram_mmap(fd, memory, page_size, block->flags & RAM_SHARED);
> if (area == MAP_FAILED) {
> error_setg_errno(errp, errno,
> "unable to map backing store for guest RAM");
>
Queued, thanks.
Paolo
- [Qemu-devel] [PATCH v2 11/42] ivshmem-test: Improve test cases /ivshmem/server-*, (continued)
- [Qemu-devel] [PATCH v2 11/42] ivshmem-test: Improve test cases /ivshmem/server-*, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 04/42] ivshmem-server: Fix and clean up command line help, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 13/42] ivshmem: Add missing newlines to debug printfs, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 01/42] exec: Fix memory allocation when memory path names new file, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 09/42] ivshmem-test: Improve test case /ivshmem/single, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 14/42] ivshmem: Compile debug prints unconditionally to prevent bit-rot, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 08/42] tests/libqos/pci-pc: Fix qpci_pc_iomap() to map BARs aligned, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 19/42] ivshmem: Failed realize() can leave migration blocker behind, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 02/42] exec: Fix memory allocation when memory path isn't on hugetlbfs, Markus Armbruster, 2016/03/07
- Re: [Qemu-devel] [PATCH v2 02/42] exec: Fix memory allocation when memory path isn't on hugetlbfs,
Paolo Bonzini <=
- [Qemu-devel] [PATCH v2 21/42] ivshmem: Clean up MSI-X conditions, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 03/42] target-ppc: Document TOCTTOU in hugepage support, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 06/42] qemu-doc: Fix ivshmem huge page example, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 35/42] ivshmem: Inline check_shm_size() into its only caller, Markus Armbruster, 2016/03/07