[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/11] Add cache handling functions
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 04/11] Add cache handling functions |
Date: |
Thu, 26 Jul 2012 15:51:36 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 |
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> Add LRU page cache mechanism.
> The page are accessed by their address.
>
> Signed-off-by: Benoit Hudzia <address@hidden>
> Signed-off-by: Petter Svard <address@hidden>
> Signed-off-by: Aidan Shribman <address@hidden>
> Signed-off-by: Orit Wasserman <address@hidden>
> +
> +PageCache *cache_init(int64_t num_pages, unsigned int page_size)
> +{
> + int64_t i;
> +
> + PageCache *cache = g_malloc(sizeof(*cache));
> +
> + if (num_pages <= 0) {
> + DPRINTF("invalid number of pages\n");
> + return NULL;
Unless memory returned by g_malloc() is automatically garbage collected,
then this is a memory leak.
> +static unsigned long cache_get_cache_pos(const PageCache *cache,
> + uint64_t address)
> +{
> + unsigned long pos;
On a 32-bit platform, this could be 32 bits...
> +
> + g_assert(cache->max_num_items);
> + pos = (address / cache->page_size) & (cache->max_num_items - 1);
while cache->max_num_items is int64_t and could thus overflow. Then
again, a 32-bit platform can't access more than 4G memory, so I think
this limitation is theoretical; still, I can't help but wonder if you
should be consistently using size_t instead of a mix of 'unsigned int',
'int32_t', and 'unsigned long' in referring to sizing within your cache
table.
> +int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
> +{
> +
> + /* move all data from old cache */
> + for (i = 0; i < cache->max_num_items; i++) {
> + old_it = &cache->page_cache[i];
> + if (old_it->it_addr != -1) {
> + /* check for collision , if there is, keep the first value */
No space before ',' in English sentences.
The comment about 'keep the first value' is wrong, you are keeping the
'MRU page'...
> + new_it = cache_get_by_addr(new_cache, old_it->it_addr);
> + if (new_it->it_data) {
> + /* keep the oldest page */
...also wrong, you are keeping the MRU page, not the oldest page...
> + if (new_it->it_age >= old_it->it_age) {
> + g_free(old_it->it_data);
since a larger it_age implies more recently used.
> +++ b/qemu-common.h
> @@ -1,3 +1,4 @@
> +
> /* Common header file that is included by all of qemu. */
> #ifndef QEMU_COMMON_H
> #define QEMU_COMMON_H
> @@ -411,6 +412,18 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b,
> uint32_t c)
> /* Round number up to multiple */
> #define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m))
>
> +static inline bool is_power_of_2(int64_t value)
> +{
> + if (!value) {
> + return 0;
> + }
> +
> + return !(value & (value - 1));
Technically undefined by C99 if value is INT64_MIN, since 'value - 1'
then overflows. Do you want this function to take uint64_t instead, to
guarantee defined results even for 0x8000000000000000?
--
Eric Blake address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 00/11] Migration next v6, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 01/11] Add migration capabilities, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 03/11] Add XBZRLE documentation, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 05/11] Add uleb encoding/decoding functions, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 04/11] Add cache handling functions, Orit Wasserman, 2012/07/25
- Re: [Qemu-devel] [PATCH 04/11] Add cache handling functions,
Eric Blake <=
- [Qemu-devel] [PATCH 00/11] Migration next v6, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live, Orit Wasserman, 2012/07/25
- [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command, Orit Wasserman, 2012/07/25