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 <pbonz...@redhat.com>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> 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
