From: Alok Kataria <alok.kataria@calsoftinc.com>
To: Christoph Lameter <clameter@engr.sgi.com>
Cc: akpm@osdl.org, manfred@colorfullife.com,
Pekka Enberg <penberg@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: slab: Remove SLAB_NO_REAP option
Date: Thu, 23 Feb 2006 13:24:17 +0530 [thread overview]
Message-ID: <1140681257.6810.24.camel@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.64.0602221428510.30219@schroedinger.engr.sgi.com>
On Thu, 2006-02-23 at 04:04, Christoph Lameter wrote:
> SLAB_NO_REAP is documented as an option that will cause this slab
> not to be reaped under memory pressure. However, that is not what
> happens. The only thing that SLAB_NO_REAP controls at the moment
> is the reclaim of the unused slab elements that were allocated in
> batch in cache_reap(). Cache_reap() is run every few seconds
> independently of memory pressure.
>
> Could we remove the whole thing? Its only used by three slabs
> anyways and I cannot find a reason for having this option.
>
There can be some caches which are not used quite often, kmem_cache for
instance. Now from performance perspective having SLAB_NO_REAP for such
caches is good.
But again i am not sure if there are any other caches which show such a
behavior. What i don't understand is why were ocfs2_lock, and the
iscsi_taskcache made to use this flag.
If there was some specific reason then it would be better we don't
change it.
> There is an additional problem with SLAB_NO_REAP. If set then
> the recovery of objects from alien caches is switched off.
> Objects not freed on the same node where they were initially
> allocated will only be reused if a certain amount of objects
> accumulates from one alien node (not very likely) or if the cache is
> explicitly shrunk. (Strangely __cache_shrink does not check for
> SLAB_NO_REAP)
>
> Getting rid of SLAB_NO_REAP fixes the problems with alien cache
> freeing.
As far as this problem is concerned we could move the draining code
above the check for SLAB_NO_REAP flag. That ways the alien caches will
be freed irrespective of the flag.
Thanks & Regards,
Alok
>
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
>
> Index: linux-2.6.16-rc4/mm/slab.c
> ===================================================================
> --- linux-2.6.16-rc4.orig/mm/slab.c 2006-02-17 14:23:45.000000000 -0800
> +++ linux-2.6.16-rc4/mm/slab.c 2006-02-22 14:06:23.000000000 -0800
> @@ -170,12 +170,12 @@
> #if DEBUG
> # define CREATE_MASK (SLAB_DEBUG_INITIAL | SLAB_RED_ZONE | \
> SLAB_POISON | SLAB_HWCACHE_ALIGN | \
> - SLAB_NO_REAP | SLAB_CACHE_DMA | \
> + SLAB_CACHE_DMA | \
> SLAB_MUST_HWCACHE_ALIGN | SLAB_STORE_USER | \
> SLAB_RECLAIM_ACCOUNT | SLAB_PANIC | \
> SLAB_DESTROY_BY_RCU)
> #else
> -# define CREATE_MASK (SLAB_HWCACHE_ALIGN | SLAB_NO_REAP | \
> +# define CREATE_MASK (SLAB_HWCACHE_ALIGN | \
> SLAB_CACHE_DMA | SLAB_MUST_HWCACHE_ALIGN | \
> SLAB_RECLAIM_ACCOUNT | SLAB_PANIC | \
> SLAB_DESTROY_BY_RCU)
> @@ -642,7 +642,7 @@ static struct kmem_cache cache_cache = {
> .limit = BOOT_CPUCACHE_ENTRIES,
> .shared = 1,
> .buffer_size = sizeof(struct kmem_cache),
> - .flags = SLAB_NO_REAP,
> + .flags = 0,
> .spinlock = SPIN_LOCK_UNLOCKED,
> .name = "kmem_cache",
> #if DEBUG
> @@ -1689,9 +1689,6 @@ static inline size_t calculate_slab_orde
> * %SLAB_RED_ZONE - Insert `Red' zones around the allocated memory to check
> * for buffer overruns.
> *
> - * %SLAB_NO_REAP - Don't automatically reap this cache when we're under
> - * memory pressure.
> - *
> * %SLAB_HWCACHE_ALIGN - Align the objects in this cache to a hardware
> * cacheline. This can be beneficial if you're counting cycles as closely
> * as davem.
> @@ -3487,10 +3484,6 @@ static void cache_reap(void *unused)
> struct slab *slabp;
>
> searchp = list_entry(walk, struct kmem_cache, next);
> -
> - if (searchp->flags & SLAB_NO_REAP)
> - goto next;
> -
> check_irq_on();
>
> l3 = searchp->nodelists[numa_node_id()];
> Index: linux-2.6.16-rc4/include/linux/slab.h
> ===================================================================
> --- linux-2.6.16-rc4.orig/include/linux/slab.h 2006-02-17 14:23:45.000000000 -0800
> +++ linux-2.6.16-rc4/include/linux/slab.h 2006-02-22 14:05:25.000000000 -0800
> @@ -38,7 +38,6 @@ typedef struct kmem_cache kmem_cache_t;
> #define SLAB_DEBUG_INITIAL 0x00000200UL /* Call constructor (as verifier) */
> #define SLAB_RED_ZONE 0x00000400UL /* Red zone objs in a cache */
> #define SLAB_POISON 0x00000800UL /* Poison objects */
> -#define SLAB_NO_REAP 0x00001000UL /* never reap from the cache */
> #define SLAB_HWCACHE_ALIGN 0x00002000UL /* align objs on a h/w cache lines */
> #define SLAB_CACHE_DMA 0x00004000UL /* use GFP_DMA memory */
> #define SLAB_MUST_HWCACHE_ALIGN 0x00008000UL /* force alignment */
> Index: linux-2.6.16-rc4/drivers/scsi/iscsi_tcp.c
> ===================================================================
> --- linux-2.6.16-rc4.orig/drivers/scsi/iscsi_tcp.c 2006-02-17 14:23:45.000000000 -0800
> +++ linux-2.6.16-rc4/drivers/scsi/iscsi_tcp.c 2006-02-22 14:08:46.000000000 -0800
> @@ -3639,7 +3639,7 @@ iscsi_tcp_init(void)
>
> taskcache = kmem_cache_create("iscsi_taskcache",
> sizeof(struct iscsi_data_task), 0,
> - SLAB_HWCACHE_ALIGN | SLAB_NO_REAP, NULL, NULL);
> + SLAB_HWCACHE_ALIGN, NULL, NULL);
> if (!taskcache)
> return -ENOMEM;
>
> Index: linux-2.6.16-rc4/fs/ocfs2/super.c
> ===================================================================
> --- linux-2.6.16-rc4.orig/fs/ocfs2/super.c 2006-02-17 14:23:45.000000000 -0800
> +++ linux-2.6.16-rc4/fs/ocfs2/super.c 2006-02-22 14:08:17.000000000 -0800
> @@ -959,7 +959,7 @@ static int ocfs2_initialize_mem_caches(v
> ocfs2_lock_cache = kmem_cache_create("ocfs2_lock",
> sizeof(struct ocfs2_journal_lock),
> 0,
> - SLAB_NO_REAP|SLAB_HWCACHE_ALIGN,
> + SLAB_HWCACHE_ALIGN,
> NULL, NULL);
> if (!ocfs2_lock_cache)
> return -ENOMEM;
next prev parent reply other threads:[~2006-02-23 7:59 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-02-22 22:34 slab: Remove SLAB_NO_REAP option Christoph Lameter
2006-02-23 7:46 ` Pekka Enberg
2006-02-23 7:54 ` Alok Kataria [this message]
2006-02-23 8:48 ` Pekka Enberg
2006-02-23 9:35 Alok Kataria
2006-02-23 10:09 ` Andrew Morton
2006-02-23 11:34 ` Pekka J Enberg
2006-02-23 17:20 ` Christoph Lameter
2006-02-23 18:36 ` Pekka Enberg
2006-02-23 18:47 ` Christoph Lameter
2006-02-23 19:17 ` Ravikiran G Thirumalai
2006-02-23 19:24 ` Christoph Lameter
2006-02-24 7:36 ` Pekka Enberg
2006-02-24 16:19 ` Christoph Lameter
2006-02-25 22:15 ` Pekka Enberg
2006-02-23 17:17 ` Christoph Lameter
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=1140681257.6810.24.camel@localhost.localdomain \
--to=alok.kataria@calsoftinc.com \
--cc=akpm@osdl.org \
--cc=clameter@engr.sgi.com \
--cc=linux-kernel@vger.kernel.org \
--cc=manfred@colorfullife.com \
--cc=penberg@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).