All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 6/7] zsmalloc: account the number of compacted pages
Date: Mon, 6 Jul 2015 22:48:50 +0900	[thread overview]
Message-ID: <20150706134850.GB663@swordfish> (raw)
In-Reply-To: <20150706132249.GA16529@blaptop>

On (07/06/15 22:22), Minchan Kim wrote:
> On Mon, Jul 06, 2015 at 09:17:49PM +0900, Sergey Senozhatsky wrote:
> > Compaction returns back to zram the number of migrated objects,
> > which is quite uninformative -- we have objects of different
> > sizes so user space cannot obtain any valuable data from that
> > number. Change compaction to operate in terms of pages and
> > return back to compaction issuer the number of pages that
> > were freed during compaction. So from now on `num_compacted'
> > column in zram<id>/mm_stat represents more meaningful value:
> > the number of freed (compacted) pages.
> 
> Fair enough.
>  
> The main reason I introduced num_migrated is to investigate
> the effieciency of compaction. ie, num_freed / num_migrated.
> However, I didn't put num_freed at that time so I can't get 
> my goal with only num_migrated.
>  
> We could put new knob num_compacted as well as num_migrated
> but I don't think we need it now. Zram's compaction would be
> much efficient compared to VM's compaction because we don't
> have any non-movable objects in zspages and we can account
> exact number of free slots.
> 
> So, I want to change name from num_migrated to num_compacted
> and maintain only it which is more useful for admin, you said.
> 
> It's not far since we introduced num_migrated so I don't think
> change name wouldn't be a big problem for userspace,(I hope).
> 

Hello,

Yes, num_migrated rename patch was on my table.
I was thinking about two variants:

struct zs_pool
  atomic_long_t		pages_allocated;
  unsigned long		num_compacted;

or

struct zs_pool
  atomic_long_t		pages_allocated;
  unsigned long		pages_compacted;

the latter looks even better I think. But I didn't come up with a
sane API name to get these stats-- zs_get_pages_compacted() is a bit
misleading. So I decided to keep num_compacted to make the patch set
smaller.

Hm, exporting a new `struct zs_pool_stat' to zram is probably a good
way to go.

	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 6/7] zsmalloc: account the number of compacted pages
Date: Mon, 6 Jul 2015 22:48:50 +0900	[thread overview]
Message-ID: <20150706134850.GB663@swordfish> (raw)
In-Reply-To: <20150706132249.GA16529@blaptop>

On (07/06/15 22:22), Minchan Kim wrote:
> On Mon, Jul 06, 2015 at 09:17:49PM +0900, Sergey Senozhatsky wrote:
> > Compaction returns back to zram the number of migrated objects,
> > which is quite uninformative -- we have objects of different
> > sizes so user space cannot obtain any valuable data from that
> > number. Change compaction to operate in terms of pages and
> > return back to compaction issuer the number of pages that
> > were freed during compaction. So from now on `num_compacted'
> > column in zram<id>/mm_stat represents more meaningful value:
> > the number of freed (compacted) pages.
> 
> Fair enough.
>  
> The main reason I introduced num_migrated is to investigate
> the effieciency of compaction. ie, num_freed / num_migrated.
> However, I didn't put num_freed at that time so I can't get 
> my goal with only num_migrated.
>  
> We could put new knob num_compacted as well as num_migrated
> but I don't think we need it now. Zram's compaction would be
> much efficient compared to VM's compaction because we don't
> have any non-movable objects in zspages and we can account
> exact number of free slots.
> 
> So, I want to change name from num_migrated to num_compacted
> and maintain only it which is more useful for admin, you said.
> 
> It's not far since we introduced num_migrated so I don't think
> change name wouldn't be a big problem for userspace,(I hope).
> 

Hello,

Yes, num_migrated rename patch was on my table.
I was thinking about two variants:

struct zs_pool
  atomic_long_t		pages_allocated;
  unsigned long		num_compacted;

or

struct zs_pool
  atomic_long_t		pages_allocated;
  unsigned long		pages_compacted;

the latter looks even better I think. But I didn't come up with a
sane API name to get these stats-- zs_get_pages_compacted() is a bit
misleading. So I decided to keep num_compacted to make the patch set
smaller.

Hm, exporting a new `struct zs_pool_stat' to zram is probably a good
way to go.

	-ss

  reply	other threads:[~2015-07-06 13:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-06 12:17 [PATCH v5 0/7] mm/zsmalloc: introduce automatic pool compaction Sergey Senozhatsky
2015-07-06 12:17 ` [PATCH v5 1/7] zsmalloc: drop unused variable `nr_to_migrate' Sergey Senozhatsky
2015-07-06 12:17 ` [PATCH v5 2/7] zsmalloc: always keep per-class stats Sergey Senozhatsky
2015-07-06 12:17 ` [PATCH v5 3/7] zsmalloc: introduce zs_can_compact() function Sergey Senozhatsky
2015-07-06 12:17 ` [PATCH v5 4/7] zsmalloc: cosmetic compaction code adjustments Sergey Senozhatsky
2015-07-06 12:17 ` [PATCH v5 5/7] zsmalloc/zram: store compaction stats in zspool Sergey Senozhatsky
     [not found]   ` <20150706132728.GB16529@blaptop>
2015-07-06 13:56     ` Sergey Senozhatsky
2015-07-06 13:56       ` Sergey Senozhatsky
2015-07-06 14:01       ` Minchan Kim
2015-07-06 14:01         ` Minchan Kim
2015-07-06 12:17 ` [PATCH v5 6/7] zsmalloc: account the number of compacted pages Sergey Senozhatsky
2015-07-06 13:22   ` Minchan Kim
2015-07-06 13:22     ` Minchan Kim
2015-07-06 13:48     ` Sergey Senozhatsky [this message]
2015-07-06 13:48       ` Sergey Senozhatsky
2015-07-06 14:00       ` Minchan Kim
2015-07-06 14:00         ` Minchan Kim
2015-07-06 12:17 ` [PATCH v5 7/7] zsmalloc: register a shrinker to trigger auto-compaction Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150706134850.GB663@swordfish \
    --to=sergey.senozhatsky@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.