linux-kernel - Re: [RFC][PATCHv3 7/7] zsmalloc: register a shrinker to trigger auto-compaction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150629085744.GA549@swordfish>
Date:	Mon, 29 Jun 2015 17:57:44 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Minchan Kim <minchan@...nel.org>
Cc:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [RFC][PATCHv3 7/7] zsmalloc: register a shrinker to trigger
 auto-compaction

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/