[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] qcow2: mark the memory as no longer needed
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] qcow2: mark the memory as no longer needed after qcow2_cache_empty() |
Date: |
Tue, 02 Jun 2015 12:50:19 +0200 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
On Tue 02 Jun 2015 12:05:59 PM CEST, Kevin Wolf <address@hidden> wrote:
>> +#include <sys/mman.h>
>> #include "block/block_int.h"
>> #include "qemu-common.h"
>> +#include "qemu/osdep.h"
>> #include "qcow2.h"
>> #include "trace.h"
>
> This breaks the mingw build:
>
> /mnt/qemu/block/qcow2-cache.c:25:22: fatal error: sys/mman.h: No such file or
> directory
> #include <sys/mman.h>
Ok, I can use #if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE)
as it's done in util/osdep.c for the same header.
>> +#if QEMU_MADV_DONTNEED != QEMU_MADV_INVALID
>> + BDRVQcowState *s = bs->opaque;
>> + void *t = qcow2_cache_get_table_addr(bs, c, i);
>> + long align = sysconf(_SC_PAGESIZE);
>
> It seems that getpagesize() is usually used in qemu.
The getpagesize() manual page actually recommends using sysconf() for
portability reasons. But other than that I don't have a problem with
getpagesize() if it's the preferred choice in QEMU.
>> + size_t mem_size = (size_t) s->cluster_size * num_tables;
>> + size_t offset = QEMU_ALIGN_UP((uintptr_t) t, align) - (uintptr_t) t;
>> + size_t length = QEMU_ALIGN_DOWN(mem_size - offset, align);
>
> Instead of all the aligning here, shouldn't we just make sure that the
> tables are created with the right alignment?
The tables should have the right alignment and their size should be a
multiple of the page size. The latter condition is not necessarily true
(a cluster can be smaller than one page) so I'd keep that code in any
case.
Berto