All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Greg Thelen <gthelen@google.com>,
	Aruna Ramakrishna <aruna.ramakrishna@oracle.com>,
	Christoph Lameter <cl@linux.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] mm, slab: faster active and free stats
Date: Mon, 28 Nov 2016 16:40:02 +0900	[thread overview]
Message-ID: <20161128074001.GA32105@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <alpine.DEB.2.10.1611110222440.16406@chino.kir.corp.google.com>

On Fri, Nov 11, 2016 at 02:30:39AM -0800, David Rientjes wrote:
> On Fri, 11 Nov 2016, Joonsoo Kim wrote:
> 
> > Hello, David.
> > 
> > Maintaining acitve/free_slab counters looks so complex. And, I think
> > that we don't need to maintain these counters for faster slabinfo.
> > Key point is to remove iterating n->slabs_partial list.
> > 
> > We can calculate active slab/object by following equation as you did in
> > this patch.
> > 
> > active_slab(n) = n->num_slab - the number of free_slab
> > active_object(n) = n->num_slab * cachep->num - n->free_objects
> > 
> > To get the number of free_slab, we need to iterate n->slabs_free list
> > but I guess it would be small enough.
> > 
> > If you don't like to iterate n->slabs_free list in slabinfo, just
> > maintaining the number of slabs_free would be enough.
> > 
> 
> Hi Joonsoo,
> 
> It's a good point, although I don't think the patch has overly complex 
> logic to keep track of slab state.
> 
> We don't prefer to do any iteration in get_slabinfo() since users can 
> read /proc/slabinfo constantly; it's better to just settle the stats when 
> slab state changes instead of repeating an expensive operation over and 
> over if someone is running slabtop(1) or /proc/slabinfo is scraped 
> regularly for stats.
> 
> That said, I imagine there are more clever ways to arrive at the same 
> answer, and you bring up a good point about maintaining a n->num_slabs and 
> n->free_slabs rather than n->active_slabs and n->free_slabs.
> 
> I don't feel strongly about either approach, but I think some improvement, 
> such as what this patch provides, is needed to prevent how expensive 
> simply reading /proc/slabinfo can be.

Hello,

Sorry for long delay.
I agree that this improvement is needed. Could you try the approach
that maintains n->num_slabs and n->free_slabs? I guess that it would be
simpler than this patch so more maintainable.

Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Greg Thelen <gthelen@google.com>,
	Aruna Ramakrishna <aruna.ramakrishna@oracle.com>,
	Christoph Lameter <cl@linux.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] mm, slab: faster active and free stats
Date: Mon, 28 Nov 2016 16:40:02 +0900	[thread overview]
Message-ID: <20161128074001.GA32105@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <alpine.DEB.2.10.1611110222440.16406@chino.kir.corp.google.com>

On Fri, Nov 11, 2016 at 02:30:39AM -0800, David Rientjes wrote:
> On Fri, 11 Nov 2016, Joonsoo Kim wrote:
> 
> > Hello, David.
> > 
> > Maintaining acitve/free_slab counters looks so complex. And, I think
> > that we don't need to maintain these counters for faster slabinfo.
> > Key point is to remove iterating n->slabs_partial list.
> > 
> > We can calculate active slab/object by following equation as you did in
> > this patch.
> > 
> > active_slab(n) = n->num_slab - the number of free_slab
> > active_object(n) = n->num_slab * cachep->num - n->free_objects
> > 
> > To get the number of free_slab, we need to iterate n->slabs_free list
> > but I guess it would be small enough.
> > 
> > If you don't like to iterate n->slabs_free list in slabinfo, just
> > maintaining the number of slabs_free would be enough.
> > 
> 
> Hi Joonsoo,
> 
> It's a good point, although I don't think the patch has overly complex 
> logic to keep track of slab state.
> 
> We don't prefer to do any iteration in get_slabinfo() since users can 
> read /proc/slabinfo constantly; it's better to just settle the stats when 
> slab state changes instead of repeating an expensive operation over and 
> over if someone is running slabtop(1) or /proc/slabinfo is scraped 
> regularly for stats.
> 
> That said, I imagine there are more clever ways to arrive at the same 
> answer, and you bring up a good point about maintaining a n->num_slabs and 
> n->free_slabs rather than n->active_slabs and n->free_slabs.
> 
> I don't feel strongly about either approach, but I think some improvement, 
> such as what this patch provides, is needed to prevent how expensive 
> simply reading /proc/slabinfo can be.

Hello,

Sorry for long delay.
I agree that this improvement is needed. Could you try the approach
that maintains n->num_slabs and n->free_slabs? I guess that it would be
simpler than this patch so more maintainable.

Thanks.

--
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>

  reply	other threads:[~2016-11-28  7:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-08 23:06 [patch] mm, slab: faster active and free stats David Rientjes
2016-11-08 23:06 ` David Rientjes
2016-11-08 23:17 ` Andrew Morton
2016-11-08 23:17   ` Andrew Morton
2016-11-10  0:38   ` David Rientjes
2016-11-10  0:38     ` David Rientjes
2016-11-11  5:53     ` Joonsoo Kim
2016-11-11  5:53       ` Joonsoo Kim
2016-11-11 10:30       ` David Rientjes
2016-11-11 10:30         ` David Rientjes
2016-11-28  7:40         ` Joonsoo Kim [this message]
2016-11-28  7:40           ` Joonsoo Kim
2016-11-30  0:56           ` David Rientjes
2016-11-30  0:56             ` David Rientjes
2016-12-02  7:58             ` 김준수/선임연구원/SW Platform(연)AOT팀(iamjoonsoo.kim@lge.com)
2016-12-02  7:58               ` 김준수/선임연구원/SW Platform(연)AOT팀(iamjoonsoo.kim@lge.com)
2016-12-05  4:23               ` [patch -mm] mm, slab: maintain total slab count instead of active count David Rientjes
2016-12-05  4:23                 ` David Rientjes

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=20161128074001.GA32105@js1304-P5Q-DELUXE \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=aruna.ramakrishna@oracle.com \
    --cc=cl@linux.com \
    --cc=gthelen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    /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.