On Tue, Jul 07, 2015 at 08:56:59PM +0900, Sergey Senozhatsky wrote: > `zs_compact_control' accounts the number of migrated objects but > it has a limited lifespan -- we lose it as soon as zs_compaction() > returns back to zram. It worked fine, because (a) zram had it's own > counter of migrated objects and (b) only zram could trigger > compaction. However, this does not work for automatic pool > compaction (not issued by zram). To account objects migrated > during auto-compaction (issued by the shrinker) we need to store > this number in zs_pool. > > Define a new `struct zs_pool_stats' structure to keep zs_pool's > stats there. It provides only `num_migrated', as of this writing, > but it surely can be extended. > > A new zsmalloc zs_pool_stats() symbol exports zs_pool's stats > back to caller. > > Use zs_pool_stats() in zram and remove `num_migrated' from > zram_stats. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx> > Suggested-by: Minchan Kim <minchan@xxxxxxxxxx> > --- > drivers/block/zram/zram_drv.c | 11 ++++++----- > drivers/block/zram/zram_drv.h | 1 - > include/linux/zsmalloc.h | 6 ++++++ > mm/zsmalloc.c | 42 ++++++++++++++++++++++-------------------- > 4 files changed, 34 insertions(+), 26 deletions(-) > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index fb655e8..aa22fe07 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -388,7 +388,6 @@ static ssize_t comp_algorithm_store(struct device *dev, > static ssize_t compact_store(struct device *dev, > struct device_attribute *attr, const char *buf, size_t len) > { > - unsigned long nr_migrated; > struct zram *zram = dev_to_zram(dev); > struct zram_meta *meta; > > @@ -399,8 +398,7 @@ static ssize_t compact_store(struct device *dev, > } > > meta = zram->meta; > - nr_migrated = zs_compact(meta->mem_pool); > - atomic64_add(nr_migrated, &zram->stats.num_migrated); > + zs_compact(meta->mem_pool); > up_read(&zram->init_lock); > > return len; > @@ -428,13 +426,16 @@ static ssize_t mm_stat_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > struct zram *zram = dev_to_zram(dev); > + struct zs_pool_stats pool_stats = {0}; Does it work even if first member of the structure is non-scalar? Personally I prefer memset for initliazation. I believe modern compiler would optimize that quite well. > u64 orig_size, mem_used = 0; > long max_used; > ssize_t ret; > > down_read(&zram->init_lock); > - if (init_done(zram)) > + if (init_done(zram)) { > mem_used = zs_get_total_pages(zram->meta->mem_pool); > + zs_pool_stats(zram->meta->mem_pool, &pool_stats); > + } > > orig_size = atomic64_read(&zram->stats.pages_stored); > max_used = atomic_long_read(&zram->stats.max_used_pages); > @@ -447,7 +448,7 @@ static ssize_t mm_stat_show(struct device *dev, > zram->limit_pages << PAGE_SHIFT, > max_used << PAGE_SHIFT, > (u64)atomic64_read(&zram->stats.zero_pages), > - (u64)atomic64_read(&zram->stats.num_migrated)); > + pool_stats.num_migrated); > up_read(&zram->init_lock); > > return ret; > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h > index 6dbe2df..8e92339 100644 > --- a/drivers/block/zram/zram_drv.h > +++ b/drivers/block/zram/zram_drv.h > @@ -78,7 +78,6 @@ struct zram_stats { > atomic64_t compr_data_size; /* compressed size of pages stored */ > atomic64_t num_reads; /* failed + successful */ > atomic64_t num_writes; /* --do-- */ > - atomic64_t num_migrated; /* no. of migrated object */ > atomic64_t failed_reads; /* can happen when memory is too low */ > atomic64_t failed_writes; /* can happen when memory is too low */ > atomic64_t invalid_io; /* non-page-aligned I/O requests */ > diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h > index 1338190..9340fce 100644 > --- a/include/linux/zsmalloc.h > +++ b/include/linux/zsmalloc.h > @@ -34,6 +34,11 @@ enum zs_mapmode { > */ > }; > > +struct zs_pool_stats { > + /* How many objects were migrated */ > + u64 num_migrated; > +}; > + > struct zs_pool; > > struct zs_pool *zs_create_pool(char *name, gfp_t flags); > @@ -49,4 +54,5 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle); > unsigned long zs_get_total_pages(struct zs_pool *pool); > unsigned long zs_compact(struct zs_pool *pool); > > +void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats); > #endif > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index ce1484e..db3cb2d 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -237,16 +237,18 @@ struct link_free { > }; > > struct zs_pool { > - char *name; > + char *name; huge tab? > > - struct size_class **size_class; > - struct kmem_cache *handle_cachep; > + struct size_class **size_class; > + struct kmem_cache *handle_cachep; tab? tab? > > - gfp_t flags; /* allocation flags used when growing pool */ > - atomic_long_t pages_allocated; Why changes comment position? > + /* Allocation flags used when growing pool */ > + gfp_t flags; > + atomic_long_t pages_allocated; > Why blank line? > + struct zs_pool_stats stats; > #ifdef CONFIG_ZSMALLOC_STAT > - struct dentry *stat_dentry; > + struct dentry *stat_dentry; Tab. > #endif > }; > > @@ -1587,7 +1589,7 @@ struct zs_compact_control { > /* Starting object index within @s_page which used for live object > * in the subpage. */ > int index; > - /* how many of objects are migrated */ > + /* How many of objects were migrated */ > int nr_migrated; > }; > > @@ -1599,7 +1601,6 @@ static int migrate_zspage(struct zs_pool *pool, struct size_class *class, > struct page *s_page = cc->s_page; > struct page *d_page = cc->d_page; > unsigned long index = cc->index; > - int nr_migrated = 0; > int ret = 0; > > while (1) { > @@ -1626,13 +1627,12 @@ static int migrate_zspage(struct zs_pool *pool, struct size_class *class, > record_obj(handle, free_obj); > unpin_tag(handle); > obj_free(pool, class, used_obj); > - nr_migrated++; > + cc->nr_migrated++; > } > > /* Remember last position in this iteration */ > cc->s_page = s_page; > cc->index = index; > - cc->nr_migrated = nr_migrated; > > return ret; > } > @@ -1707,14 +1707,13 @@ static unsigned long zs_can_compact(struct size_class *class) > return obj_wasted * get_pages_per_zspage(class->size); > } > > -static unsigned long __zs_compact(struct zs_pool *pool, > - struct size_class *class) > +static void __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_total_migrated = 0; > > + cc.nr_migrated = 0; > spin_lock(&class->lock); > while ((src_page = isolate_source_page(class))) { > > @@ -1736,7 +1735,6 @@ static unsigned long __zs_compact(struct zs_pool *pool, > break; > > putback_zspage(pool, class, dst_page); > - nr_total_migrated += cc.nr_migrated; > } > > /* Stop if we couldn't find slot */ > @@ -1746,7 +1744,6 @@ static unsigned long __zs_compact(struct zs_pool *pool, > putback_zspage(pool, class, dst_page); > putback_zspage(pool, class, src_page); > spin_unlock(&class->lock); > - nr_total_migrated += cc.nr_migrated; > cond_resched(); > spin_lock(&class->lock); > } > @@ -1754,15 +1751,14 @@ static unsigned long __zs_compact(struct zs_pool *pool, > if (src_page) > putback_zspage(pool, class, src_page); > > - spin_unlock(&class->lock); > + pool->stats.num_migrated += cc.nr_migrated; > > - return nr_total_migrated; > + spin_unlock(&class->lock); > } > > unsigned long zs_compact(struct zs_pool *pool) > { > int i; > - unsigned long nr_migrated = 0; > struct size_class *class; > > for (i = zs_size_classes - 1; i >= 0; i--) { > @@ -1771,13 +1767,19 @@ unsigned long zs_compact(struct zs_pool *pool) > continue; > if (class->index != i) > continue; > - nr_migrated += __zs_compact(pool, class); > + __zs_compact(pool, class); > } > > - return nr_migrated; > + return pool->stats.num_migrated; > } > EXPORT_SYMBOL_GPL(zs_compact); > > +void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats) > +{ > + memcpy(stats, &pool->stats, sizeof(struct zs_pool_stats)); > +} > +EXPORT_SYMBOL_GPL(zs_pool_stats); > + > /** > * zs_create_pool - Creates an allocation pool to work from. > * @flags: allocation flags used to allocate pool metadata > -- > 2.4.5 > -- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>