Re: A couple of 2.4.23-pre4 VM nits

[Marcelo Tosatti]

On Sun, 21 Sep 2003, Andrea Arcangeli wrote:

> Hi Shantanu,
> 
> On Sat, Sep 20, 2003 at 06:39:30AM -0700, Shantanu Goel wrote:
> > Hi Andrea,
> > 
> > The VM fixes perform rather well in my testing
> > (thanks!), but I noticed a couple of glitches that the
> > attached patch addresses.
> > 
> > 1. max_scan is never decremented in shrink_cache().  I
> > am assuming this is a typo.
> 
> this is an huge half-merging error, no surprise Andi run into vm
> troubles with pre4. My tree looks like this for years:
> 
> 		if (unlikely(!page_count(page)))
> 			continue;
> 
> 		only_metadata = 0;
> 		if (!memclass(page_zone(page), classzone)) {
> 			/*
> 			 * Hack to address an issue found by Rik. The
> 			 * problem is that
> 			 * highmem pages can hold buffer headers
> 			 * allocated
> 			 * from the slab on lowmem, and so if we are
> 			 * working
> 			 * on the NORMAL classzone here, it is correct
> 			 * not to
> 			 * try to free the highmem pages themself (that
> 			 * would be useless)
> 			 * but we must make sure to drop any lowmem
> 			 * metadata related to those
> 			 * highmem pages.
> 			 */
> 			if (page->buffers && page->mapping) { /* fast
> path racy check */
> 				if (unlikely(TryLockPage(page)))
> 					continue;
> 				if (page->buffers && page->mapping &&
> memclass_related_bhs(page, classzone)) { /* non racy check */
> 					only_metadata = 1;
> 					goto free_bhs;
> 				}
> 				UnlockPage(page);
> 			}
> 			continue;
> 		}
> 
> 		max_scan--;
> 
> max_scan-- should happen only after the memclass check to ensure not
> failing too early on GFP_KERNEL/DMA allocations (i.e. no highmem)
> 
> This is the right fix:
> 
> --- 2.4.23pre4/mm/vmscan.c.~1~	2003-09-13 00:08:04.000000000 +0200
> +++ 2.4.23pre4/mm/vmscan.c	2003-09-21 04:40:12.000000000 +0200
> @@ -401,6 +401,8 @@ static int shrink_cache(int nr_pages, zo
>  		if (!memclass(page_zone(page), classzone))
>  			continue;
>  
> +		max_scan--;
> +
>  		/* Racy check to avoid trylocking when not worthwhile */
>  		if (!page->buffers && (page_count(page) != 1 || !page->mapping))
>  			goto page_mapped;

Right! Thanks Andrea. 

Sorry for the merge mistake people. Shame on me.

Im going to release pre5 now with this. 

-
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/