Patchwork [2/4] Fix cache_resize to keep old entry age

login
register
mail settings
Submitter Orit Wasserman
Date Feb. 25, 2013, 1:19 p.m.
Message ID <1361798382-6515-3-git-send-email-owasserm@redhat.com>
Download mbox | patch
Permalink /patch/222921/
State New
Headers show

Comments

Orit Wasserman - Feb. 25, 2013, 1:19 p.m.
Instead of using cache_insert do the update itself

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 page_cache.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
Peter Lieven - Feb. 25, 2013, 1:51 p.m.
On 25.02.2013 14:19, Orit Wasserman wrote:
> Instead of using cache_insert do the update itself
>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> ---
>   page_cache.c | 16 ++++++----------
>   1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/page_cache.c b/page_cache.c
> index 748957b..21a4cde 100644
> --- a/page_cache.c
> +++ b/page_cache.c
> @@ -192,18 +192,14 @@ int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
>           if (old_it->it_addr != -1) {
>               /* check for collision, if there is, keep MRU page */
>               new_it = cache_get_by_addr(new_cache, old_it->it_addr);
> -            if (new_it->it_data) {
> +            if (new_it->it_data && new_it->it_age >= old_it->it_age) {
>                   /* keep the MRU page */
> -                if (new_it->it_age >= old_it->it_age) {
> -                    g_free(old_it->it_data);
> -                } else {
> -                    g_free(new_it->it_data);
> -                    new_it->it_data = old_it->it_data;
> -                    new_it->it_age = old_it->it_age;
> -                    new_it->it_addr = old_it->it_addr;
> -                }
> +                g_free(old_it->it_data);
>               } else {
> -                cache_insert(new_cache, old_it->it_addr, old_it->it_data);
> +                g_free(new_it->it_data);
> +                new_it->it_data = old_it->it_data;
> +                new_it->it_age = old_it->it_age;
> +                new_it->it_addr = old_it->it_addr;
>               }
>           }
>       }
Reviewed-by: Peter Lieven <pl@kamp.de>
Juan Quintela - Feb. 25, 2013, 4:34 p.m.
Orit Wasserman <owasserm@redhat.com> wrote:
> Instead of using cache_insert do the update itself
>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> ---
>  page_cache.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/page_cache.c b/page_cache.c
> index 748957b..21a4cde 100644
> --- a/page_cache.c
> +++ b/page_cache.c
> @@ -192,18 +192,14 @@ int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
>          if (old_it->it_addr != -1) {
>              /* check for collision, if there is, keep MRU page */
>              new_it = cache_get_by_addr(new_cache, old_it->it_addr);
> -            if (new_it->it_data) {
> +            if (new_it->it_data && new_it->it_age >= old_it->it_age) {
>                  /* keep the MRU page */
> -                if (new_it->it_age >= old_it->it_age) {
> -                    g_free(old_it->it_data);
> -                } else {
> -                    g_free(new_it->it_data);
> -                    new_it->it_data = old_it->it_data;
> -                    new_it->it_age = old_it->it_age;
> -                    new_it->it_addr = old_it->it_addr;
> -                }
> +                g_free(old_it->it_data);
>              } else {
> -                cache_insert(new_cache, old_it->it_addr, old_it->it_data);
> +                g_free(new_it->it_data);
> +                new_it->it_data = old_it->it_data;
> +                new_it->it_age = old_it->it_age;
> +                new_it->it_addr = old_it->it_addr;
>              }
>          }
>      }

We are missing
    if (!it->it_data) {
        cache->num_items++;
    }

in the !new_it->it_date case.

Later, Juan.

Patch

diff --git a/page_cache.c b/page_cache.c
index 748957b..21a4cde 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -192,18 +192,14 @@  int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
         if (old_it->it_addr != -1) {
             /* check for collision, if there is, keep MRU page */
             new_it = cache_get_by_addr(new_cache, old_it->it_addr);
-            if (new_it->it_data) {
+            if (new_it->it_data && new_it->it_age >= old_it->it_age) {
                 /* keep the MRU page */
-                if (new_it->it_age >= old_it->it_age) {
-                    g_free(old_it->it_data);
-                } else {
-                    g_free(new_it->it_data);
-                    new_it->it_data = old_it->it_data;
-                    new_it->it_age = old_it->it_age;
-                    new_it->it_addr = old_it->it_addr;
-                }
+                g_free(old_it->it_data);
             } else {
-                cache_insert(new_cache, old_it->it_addr, old_it->it_data);
+                g_free(new_it->it_data);
+                new_it->it_data = old_it->it_data;
+                new_it->it_age = old_it->it_age;
+                new_it->it_addr = old_it->it_addr;
             }
         }
     }