Hello,
thanks for review.
On (06/29/15 16:07), Minchan Kim wrote:
[..]
> > if (!migrate_zspage(pool, class, &cc))
> > - break;
> > + goto out;
>
> It should retry with another target_page instead of going out.
yep.
[..]
> > +static unsigned long zs_shrinker_scan(struct shrinker *shrinker,
> > + struct shrink_control *sc)
> > +{
[..]
>
> Returns migrated object count.
>
[..]
> > +static unsigned long zs_shrinker_count(struct shrinker *shrinker,
> > + struct shrink_control *sc)
> > +{
[..]
>
> But it returns wasted_obj / max_obj_per_zspage?
>
Good catch.
So, __zs_compact() and zs_shrinker_count() are ok. Returning
"wasted_obj / max_obj_per_zspage" from zs_can_compact() makes
sense there. The only place is zs_shrinker_scan()->zs_compact().
Hm, I can think of:
(a) We can change zs_compact() to return the total number of
freed zspages. That will not really change a user visible
interface. We export (fwiw) the number of compacted objects
in mm_stat. Basically, this is internal zsmalloc() counter and
no user space program can ever do anything with that data. From
that prospective we will just replace one senseless number with
another (equally meaningless) one.
(b) replace zs_compact() call in zs_shrinker_scan() with a class loop
1764 int i;
1765 unsigned long nr_migrated = 0;
1766 struct size_class *class;
1767
1768 for (i = zs_size_classes - 1; i >= 0; i--) {
1769 class = pool->size_class[i];
1770 if (!class)
1771 continue;
1772 if (class->index != i)
1773 continue;
1774 nr_migrated += __zs_compact(pool, class);
1775 }
1776
1777 return nr_migrated;
But on every iteration increment nr_migrated with
"nr_migrated += just_migrated / max_obj_per_zspage"
(which will be unnecessary if zs_compact() will return the number of freed
zspages).
So, (b) is mostly fine, except that we already have several pool->size_class
loops, with same `if (!class)' and `if (class->index...)' checks; and it
asks for some sort of refactoring or... a tricky for_each_class() define.
In both cases, however, we don't tell anything valuable to user space.
Thus,
(c) Return from zs_compact() the number of pages (PAGE_SIZE) freed.
And change compaction to operate in terms of pages (PAGE_SIZE). At
least mm_stat::compacted will turn into something useful for user
space.
-ss
--
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>