Hi Sergey,

On Wed, Jul 01, 2015 at 04:29:52PM +0900, Sergey Senozhatsky wrote:
> On (06/30/15 21:35), Sergey Senozhatsky wrote:
> [..]
> >     if (src_page)
> >             putback_zspage(pool, class, src_page);
> >  
> > -   pool->num_migrated += cc.nr_migrated;
> > +   cc.nr_migrated /= get_maxobj_per_zspage(class->size,
> > +                   class->pages_per_zspage);
> > +
> > +   pool->num_migrated += cc.nr_migrated *
> > +           get_pages_per_zspage(class->size);
> >  
> >     spin_unlock(&class->lock);
> 
> Oh, well. This is bloody wrong, sorry. We don't pick up src_page-s that we
> can completely drain. Thus, the fact that we can't compact (!zs_can_compact())
> anymore doesn't mean that we actually have released any zspages.
> 
> So...
> 
> (a) we can isolate_source_page() more accurately -- iterate list and
> look for pages that have ->inuse less or equal to the amount of unused
> objects. So we can guarantee that this particular zspage will be released
> at the end. It adds O(n) every time we isolate_source_page(), because
> the number of unused objects changes. But it's sort of worth it, I
> think. Otherwise we still can move M objects w/o releasing any pages
> after all. If we consider compaction as a slow path (and I think we do)
> then this option doesn't look so bad.
> 
> 
> 
> (b) if (a) is not an option, then we need to know that we have drained the
> src_page. And it seems that the easiest way to do it is to change
> 'void putback_zspage(...)' to 'bool putback_zspage(...)' and return `true'
> from putback_zspage() when putback resulted in free_zspage() (IOW, the page
> was ZS_EMPTY). And in __zs_compact() do something like

Just nit:

putback_zspage is not related to "free zspage" so I want to handle it in
caller. For it, putback_zspage could return fullness type page is added.
Something like that.

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 99133e8..32e3bb9 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1653,7 +1653,17 @@ static struct page *isolate_target_page(struct 
size_class *class)
        return page;
 }
 
-static void putback_zspage(struct zs_pool *pool, struct size_class *class,
+/*
+ * putback_zspage - add @first_page into right class's fullness list
+ * @pool: target pool
+ * @class: destination class
+ * @first_page: target page
+ *
+ * Return:
+ * The fullness_group @fist_page is added
+ */
+static enum fullness_group putback_zspage(struct zs_pool *pool,
+                               struct size_class *class,
                                struct page *first_page)
 {
        enum fullness_group fullness;
@@ -1672,6 +1682,8 @@ static void putback_zspage(struct zs_pool *pool, struct 
size_class *class,
 
                free_zspage(first_page);
        }
+
+       return fullness;
 }
 
 static struct page *isolate_source_page(struct size_class *class)
@@ -1707,11 +1719,13 @@ static unsigned long zs_can_compact(struct size_class 
*class)
        return obj_wasted * get_pages_per_zspage(class->size);
 }
 
-static void __zs_compact(struct zs_pool *pool, struct size_class *class)
+static unsigned long __zs_compact(struct zs_pool *pool,
+                               struct size_class *class)
 {
        struct zs_compact_control cc;
        struct page *src_page;
        struct page *dst_page = NULL;
+       unsigned long nr_freed = 0;
 
        cc.nr_migrated = 0;
        spin_lock(&class->lock);
@@ -1742,7 +1756,8 @@ static void __zs_compact(struct zs_pool *pool, struct 
size_class *class)
                        break;
 
                putback_zspage(pool, class, dst_page);
-               putback_zspage(pool, class, src_page);
+               if (ZS_EMPTY == putback_zspage(pool, class, src_page))
+                       nr_freed += get_pages_per_zspage(class->size);
                spin_unlock(&class->lock);
                cond_resched();
                spin_lock(&class->lock);
@@ -1758,22 +1773,36 @@ static void __zs_compact(struct zs_pool *pool, struct 
size_class *class)
                get_pages_per_zspage(class->size);
 
        spin_unlock(&class->lock);
+
+       return nr_freed;
 }
 
+/*
+ * zs_compact - migrate objects and free empty zspage in the @pool
+ * @pool: target pool for compaction
+ *
+ * Return:
+ * The number of freed pages by compaction
+ */
 unsigned long zs_compact(struct zs_pool *pool)
 {
        int i;
        struct size_class *class;
+       unsigned long nr_freed = 0;
 
        for (i = zs_size_classes - 1; i >= 0; i--) {
+
                class = pool->size_class[i];
                if (!class)
                        continue;
+
                if (class->index != i)
                        continue;
-               __zs_compact(pool, class);
+
+               nr_freed += __zs_compact(pool, class);
        }
-       return pool->num_migrated;
+
+       return nr_freed;
 }
 EXPORT_SYMBOL_GPL(zs_compact);
 
-- 
1.9.3



-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to