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/