lkml.org 
[lkml]   [2007]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 02/10] SLUB: slab defragmentation and kmem_cache_vacate
On Mon, 21 May 2007, Mel Gorman wrote:

> I know I brought up this "less than a quarter" thing before and I
> haven't thought of a better alternative. However, it occurs to be that
> shrink_slab() is called when there is awareness of a reclaim priority.
> It may be worth passing that down so that the fraction of candidates
> pages is calculated based on priority.

Hmmmm.. Yes I am thinking about that one too. Right now I have a system
that triggers reclaim every 10 seconds or after more than 100 objects have
been reclaimed.

> That said..... where is kmem_cache_shrink() ever called? The freeing of
> slab pages seems to be indirect these days. Way back,
> kmem_cache_shrink() used to be called directly but I'm not sure where it
> happens now.

Well, this one only allows manual triggering. To my embarasasment I found
that the kmem_cache_shrink calls for icache and dentries are only in 2.4.
They have been removed in 2.6.X. I need to add them back to 2.6.

> > /*
> > + * Order the freelist so that addresses increase as object are allocated.
> > + * This is useful to trigger the cpu cacheline prefetching logic.
> > + */
>
> makes sense. However, it occurs to me that maybe this should be a
> separate patch so it can be measured to be sure. It makes sense though.

Ok.

> > +/*
> > + * Vacate all objects in the given slab.
> > + *
> > + * Slab must be locked and frozen. Interrupts are disabled (flags must
> > + * be passed).
> > + *
>
> It may not hurt to have a VM_BUG_ON() if interrupts are still enabled when
> this is called

Well flags need to be passed and those flags are obtained via disabling
interrupts.


> > + /*
> > + * Got references. Now we can drop the slab lock. The slab
> > + * is frozen so it cannot vanish from under us nor will
> > + * allocations be performed on the slab. However, unlocking the
> > + * slab will allow concurrent slab_frees to proceed.
> > + */
> > + slab_unlock(page);
> > + local_irq_restore(flags);
>
> I recognise that you want to restore interrupts as early as possible but
> it should be noted somewhere that kmem_cache_vacate() disables
> interrupts and __kmem_cache_vacate() enabled them again. I had to go
> searching to see where interrupts are enabled again.
>
> Maybe even a slab_lock_irq() and slab_unlock_irq() would clarify things
> a little.

Hmmmm... Okay but this is a rare situation in SLUB. Regular slab
operations always run with interrupts disabled.

> > +
> > + vector = kmalloc(s->objects * sizeof(void *), GFP_KERNEL);
> > + if (!vector)
> > + return 0;
>
> Is it worth logging this event, returning -ENOMEM or something so that
> callers are aware of why kmem_cache_vacate() failed in this instance?
>
> Also.. we have called get_page_unless_zero() but if we are out of memory
> here, where have we called put_page()? Maybe we should be "goto out"
> here with a

Ahh. Thanks. Will fix that.

> > + * We are holding a lock on a slab page and all operations on the
> > + * slab are blocking.
> > + */
> > + if (!s->ops->get || !s->ops->kick)
> > + goto out_locked;
> > + freeze_from_list(s, page);
> > + vacated = __kmem_cache_vacate(s, page, flags, vector) == 0;
>
> That is a little funky looking. This may be nicer;
>
> vacated = __kmem_cache_vacate(s, page, flags, vector);
> out:
> ...
> return vacated == 0;
>

Right. Done.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-05-21 19:03    [W:0.049 / U:0.536 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site