All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/slab: Improve performance of gathering slabinfo stats
Date: Tue, 2 Aug 2016 11:43:43 +0900	[thread overview]
Message-ID: <20160802024342.GA15062@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <4a3fe3bc-eb1d-ea18-bd70-98b8b9c6a7d7@oracle.com>

On Mon, Aug 01, 2016 at 06:43:00PM -0700, Aruna Ramakrishna wrote:
> Hi Joonsoo,
> 
> On 08/01/2016 05:55 PM, Joonsoo Kim wrote:
> >Your patch updates these counters not only when a slabs are created and
> >destroyed but also when object is allocated/freed from the slab. This
> >would hurt runtime performance.
> >
> 
> The counters are not updated for each object allocation/free - only
> if that allocation/free results in that slab moving from one list
> (free/partial/full) to another.
> 
> >>> slab lists for gathering slabinfo stats, resulting in a dramatic
> >>> performance improvement. We tested this after growing the dentry cache to
> >>> 70GB, and the performance improved from 2s to 2ms.
> >Nice improvement. I can think of an altenative.
> >
> >I guess that improvement of your change comes from skipping to iterate
> >n->slabs_full list. We can achieve it just with introducing only num_slabs.
> >num_slabs can be updated when a slabs are created and destroyed.
> >
> 
> Yes, slabs_full is typically the largest list.
> 
> >We can calculate num_slabs_full by following equation.
> >
> >num_slabs_full = num_slabs - num_slabs_partial - num_slabs_free
> >
> >Calculating both num_slabs_partial and num_slabs_free by iterating
> >n->slabs_XXX list would not take too much time.
> 
> Yes, this would work too. We cannot avoid traversal of
> slabs_partial, and slabs_free is usually a small list, so this
> should give us similar performance benefits. But having separate
> counters could also be useful for debugging, like the ones defined
> under CONFIG_DEBUG_SLAB/STATS. Won't that help?

We can calculate these counters by traversing all list so it would not
be helpful except for performance reason. Cost of maintaining these
counters isn't free so it's better not to add more than we need.

Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/slab: Improve performance of gathering slabinfo stats
Date: Tue, 2 Aug 2016 11:43:43 +0900	[thread overview]
Message-ID: <20160802024342.GA15062@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <4a3fe3bc-eb1d-ea18-bd70-98b8b9c6a7d7@oracle.com>

On Mon, Aug 01, 2016 at 06:43:00PM -0700, Aruna Ramakrishna wrote:
> Hi Joonsoo,
> 
> On 08/01/2016 05:55 PM, Joonsoo Kim wrote:
> >Your patch updates these counters not only when a slabs are created and
> >destroyed but also when object is allocated/freed from the slab. This
> >would hurt runtime performance.
> >
> 
> The counters are not updated for each object allocation/free - only
> if that allocation/free results in that slab moving from one list
> (free/partial/full) to another.
> 
> >>> slab lists for gathering slabinfo stats, resulting in a dramatic
> >>> performance improvement. We tested this after growing the dentry cache to
> >>> 70GB, and the performance improved from 2s to 2ms.
> >Nice improvement. I can think of an altenative.
> >
> >I guess that improvement of your change comes from skipping to iterate
> >n->slabs_full list. We can achieve it just with introducing only num_slabs.
> >num_slabs can be updated when a slabs are created and destroyed.
> >
> 
> Yes, slabs_full is typically the largest list.
> 
> >We can calculate num_slabs_full by following equation.
> >
> >num_slabs_full = num_slabs - num_slabs_partial - num_slabs_free
> >
> >Calculating both num_slabs_partial and num_slabs_free by iterating
> >n->slabs_XXX list would not take too much time.
> 
> Yes, this would work too. We cannot avoid traversal of
> slabs_partial, and slabs_free is usually a small list, so this
> should give us similar performance benefits. But having separate
> counters could also be useful for debugging, like the ones defined
> under CONFIG_DEBUG_SLAB/STATS. Won't that help?

We can calculate these counters by traversing all list so it would not
be helpful except for performance reason. Cost of maintaining these
counters isn't free so it's better not to add more than we need.

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-08-02  2:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-02  0:09 [PATCH] mm/slab: Improve performance of gathering slabinfo stats Aruna Ramakrishna
2016-08-02  0:09 ` Aruna Ramakrishna
2016-08-02  0:55 ` Joonsoo Kim
2016-08-02  0:55   ` Joonsoo Kim
2016-08-02  1:43   ` Aruna Ramakrishna
2016-08-02  1:43     ` Aruna Ramakrishna
2016-08-02  2:43     ` Joonsoo Kim [this message]
2016-08-02  2:43       ` Joonsoo Kim
2016-08-02 14:59       ` Christoph Lameter
2016-08-02 14:59         ` Christoph Lameter
2016-08-02 17:39         ` Aruna Ramakrishna
2016-08-02 17:39           ` Aruna Ramakrishna

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=20160802024342.GA15062@js1304-P5Q-DELUXE \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=aruna.ramakrishna@oracle.com \
    --cc=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=penberg@kernel.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.