HI,
On Mon, 19 Jun 2006, Christoph Lameter wrote:
> Add a new function drop_freelist that removes slabs with objects
> that are already free and use that in various places.
I think you mean drain_freelist(). Anyway, looks good. Some
minor comments below.
> -static int __node_shrink(struct kmem_cache *cachep, int node)
> +/*
> + * Remove slabs from the list of free slabs.
> + * Specify the number of slabs to drain in tofree.
> + *
> + * Returns the actual number of slabs released.
> + */
> +static int long drain_freelist(struct kmem_cache *cachep,
> + struct kmem_list3 *l3, int tofree)
I have been trying to slowly kill the 'p' prefix so I'd appreciate if you
could just call the parameter 'cache'. Also, l3 could be 'lists'.
> + nr_freed = 0;
> + while (nr_freed < tofree && !list_empty(&l3->slabs_free)) {
>
> - p = l3->slabs_free.prev;
> - if (p == &l3->slabs_free)
> - break;
> + spin_lock_irq(&l3->list_lock);
> + p = l3->slabs_free.next;
> + if (p == &(l3->slabs_free)) {
Please drop the redundant parenthesis.
> + spin_unlock_irq(&l3->list_lock);
> + return nr_freed;
> + }
Goto to the bottom would be nicer than return here, maybe.
>
> - slabp = list_entry(l3->slabs_free.prev, struct slab, list);
> + slabp = list_entry(p, struct slab, list);
> #if DEBUG
> BUG_ON(slabp->inuse);
> #endif
> list_del(&slabp->list);
> -
> + /*
> + * Safe to drop the lock. The slab is no longer linked
> + * to the cache.
> + */
> l3->free_objects -= cachep->num;
> spin_unlock_irq(&l3->list_lock);
> slab_destroy(cachep, slabp);
> - spin_lock_irq(&l3->list_lock);
> - }
> - ret = !list_empty(&l3->slabs_full) || !list_empty(&l3->slabs_partial);
> - return ret;
> + nr_freed ++;
Redundant whitespace.
> + };
Redundant semicolon.
> + else {
> + int x;
nr_freed, would be better.
>
> - slabp = list_entry(p, struct slab, list);
> - BUG_ON(slabp->inuse);
> - list_del(&slabp->list);
> - STATS_INC_REAPED(searchp);
> -
> - /*
> - * Safe to drop the lock. The slab is no longer linked
> - * to the cache. searchp cannot disappear, we hold
> - * cache_chain_lock
> - */
> - l3->free_objects -= searchp->num;
> - spin_unlock_irq(&l3->list_lock);
> - slab_destroy(searchp, slabp);
> - } while (--tofree > 0);
> + x = drain_freelist(searchp, l3, (l3->free_limit +
> + 5 * searchp->num - 1) / (5 * searchp->num));
> + STATS_ADD_REAPED(searchp, x);
Maybe extract a local variable 'to_free' for readability.
Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
