Re: Large slab cache in 2.6.1

[Nick Piggin]

Chris Wedgwood wrote:

> On Sun, Feb 22, 2004 at 04:37:46PM +1100, Nick Piggin wrote:
>
>
> > Can you upgrade to 2.6.3-mm2? It would be ideal if you could test
> > this patch against that kernel due to the other VM changes.
>
> Sure.
>
>
> > Chris, could you test this too please? Thanks.
>
> I tested this change to a stock 2.6.3 kernel and saw a marginally
> better situation... 650MB in slab instead of 850MB:

In your case, this is probably ideal if the system is
not doing much. You now have a reasonable amount of low
memory available.


> ===== page_alloc.c 1.186 vs edited =====
> --- 1.186/mm/page_alloc.c	Wed Feb 18 19:43:04 2004
> +++ edited/page_alloc.c	Sat Feb 21 21:05:32 2004
> @@ -764,13 +764,18 @@
>
> EXPORT_SYMBOL(nr_free_pages);
>
> +/*
> + * return the number of non-highmem pages (we should probably rename
> + * this function? --cw)
> + */
> unsigned int nr_used_zone_pages(void)
> {
> 	unsigned int pages = 0;
> 	struct zone *zone;
>
> 	for_each_zone(zone)
> -		pages += zone->nr_active + zone->nr_inactive;
> +		if (!is_highmem(zone))
> +		    pages += zone->nr_active + zone->nr_inactive;
>
> 	return pages;
> }
>
>
> I'll test -mm2 with your patch shortly.

My patch will be functionally the same as yours so you'll be
mainly testing the other VM changes (which isn't a bad thing).
Thanks.

> > @@ -145,7 +145,7 @@ static int shrink_slab(unsigned long sca
> > 	if (down_trylock(&shrinker_sem))
> > 		return 0;
> >
> > -	pages = nr_used_zone_pages();
> > +	pages = nr_lowmem_lru_pages();
>
> Cool. I think renaming this i a good idea.

Yep.

> > -unsigned int nr_used_zone_pages(void)
> > +unsigned int nr_lowmem_lru_pages(void)
> > {
> > +	pg_data_t *pgdat;
> > 	unsigned int pages = 0;
> > -	struct zone *zone;
> >
> > -	for_each_zone(zone)
> > -		pages += zone->nr_active + zone->nr_inactive;
> > +	for_each_pgdat(pgdat) {
> > +		int i;
> > +		for (i = 0; i < ZONE_HIGHMEM; i++) {
> > +			struct zone *zone = pgdat->node_zones + i;
> > +			pages += zone->nr_active + zone->nr_inactive;
> > +		}
> > +	}
>
> Why not just check is_highmem(zone) here?

Why indeed? Easier to read vs a tiny bit faster. It isn't
really a fast path, so your version is probably better.

> > -extern unsigned int nr_used_zone_pages(void);
> > +extern unsigned int nr_lowmem_lru_pages(void);
>
> Since shrink_slab() is the only consumer of this why not move the
> function to vmscan.c just above shrink_slab()?

Might as well I suppose. I'll incorporate your suggestions if
it tests well and I end up sending it off to Andrew.

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