nr_free_buffer_pages 2.4.23pre4

nr_free_buffer_pages 2.4.23pre4

[Andrea Arcangeli]

Hi Marcelo,

this patch corrects two bugs in nr_free_buffer_pages (they weren't fatal
but it's better to get them right of course ;)

	http://www.us.kernel.org/pub/linux/kernel/people/andrea/patches/v2.4/2.4.23pre4/nr_free_buffer_pages-fixes-1

the first bug is the zonep++ before dereferencing it, the second bug is
the free variable that has to be unsigned to handle empty zones
correctly (thanks to Bjorn Helgaas for noticing the latter sign issue).

diff -urNp --exclude CVS --exclude BitKeeper 2.4.23pre4/mm/page_alloc.c x/mm/page_alloc.c
--- 2.4.23pre4/mm/page_alloc.c	2003-09-13 00:08:04.000000000 +0200
+++ x/mm/page_alloc.c	2003-09-18 06:42:15.000000000 +0200
@@ -512,14 +512,12 @@ unsigned int nr_free_buffer_pages (void)
 		class_idx = zone_idx(zone);
 
 		sum += zone->nr_cache_pages;
-		do {
-			unsigned int free = zone->free_pages - zone->watermarks[class_idx].high;
-			zonep++;
-			zone = *zonep;
+		for (zone = pgdat->node_zones; zone < pgdat->node_zones + MAX_NR_ZONES; zone++) {
+			int free = zone->free_pages - zone->watermarks[class_idx].high;
 			if (free <= 0)
 				continue;
 			sum += free;
-		} while (zone);
+		}
 	}
 
 	return sum;


(this had more prio than the waterkmark fixes, they will came next)

According to the kernel CVS you didn't merge this yet, so please merge
the below too, it will remove a not necessary branch that also generates
a gcc false positive (all harmless of course but it's more correct to
remove it):

--- 2.4.23pre4/mm/page_alloc.c.~1~	2003-09-13 00:08:04.000000000 +0200
+++ 2.4.23pre4/mm/page_alloc.c	2003-09-14 01:05:24.000000000 +0200
@@ -258,8 +258,6 @@ static struct page * balance_classzone(z
 	struct page * page = NULL;
 	int __freed;
 
-	if (!(gfp_mask & __GFP_WAIT))
-		goto out;
 	if (in_interrupt())
 		BUG();
 
thanks,

Andrea

/*
 * If you refuse to depend on closed software for a critical
 * part of your business, these links may be useful:
 *
 * rsync.kernel.org::pub/scm/linux/kernel/bkcvs/linux-2.5/
 * rsync.kernel.org::pub/scm/linux/kernel/bkcvs/linux-2.4/
 * http://www.cobite.com/cvsps/
 *
 * svn://svn.kernel.org/linux-2.6/trunk
 * svn://svn.kernel.org/linux-2.4/trunk
 */

Re: nr_free_buffer_pages 2.4.23pre4

[Mikael Pettersson]

On Thu, 18 Sep 2003 07:06:12 +0200, Andrea Arcangeli <andrea@suse.de> wrote:
> According to the kernel CVS you didn't merge this yet, so please merge
> the below too, it will remove a not necessary branch that also generates
> a gcc false positive (all harmless of course but it's more correct to
> remove it):
> 
> --- 2.4.23pre4/mm/page_alloc.c.~1~	2003-09-13 00:08:04.000000000 +0200
> +++ 2.4.23pre4/mm/page_alloc.c	2003-09-14 01:05:24.000000000 +0200
> @@ -258,8 +258,6 @@ static struct page * balance_classzone(z
>  	struct page * page = NULL;
>  	int __freed;
>  
> -	if (!(gfp_mask & __GFP_WAIT))
> -		goto out;
>  	if (in_interrupt())
>  		BUG();

Andrea,

This cleanup leaves the 'out' label unused, triggering
yet another gcc warning (though less scary than the previous).
Please apply this cleanup patch on top of the one above.

/Mikael

--- linux-2.4.23-pre4/mm/page_alloc.c.~1~	2003-09-18 13:43:41.753607800 +0200
+++ linux-2.4.23-pre4/mm/page_alloc.c	2003-09-18 13:55:46.785155159 +0200
@@ -317,7 +317,6 @@
 		}
 		current->nr_local_pages = 0;
 	}
- out:
 	*freed = __freed;
 	return page;
 }

Re: nr_free_buffer_pages 2.4.23pre4

[Andrea Arcangeli]

On Thu, Sep 18, 2003 at 02:01:43PM +0200, Mikael Pettersson wrote:
> On Thu, 18 Sep 2003 07:06:12 +0200, Andrea Arcangeli <andrea@suse.de> wrote:
> > According to the kernel CVS you didn't merge this yet, so please merge
> > the below too, it will remove a not necessary branch that also generates
> > a gcc false positive (all harmless of course but it's more correct to
> > remove it):
> > 
> > --- 2.4.23pre4/mm/page_alloc.c.~1~	2003-09-13 00:08:04.000000000 +0200
> > +++ 2.4.23pre4/mm/page_alloc.c	2003-09-14 01:05:24.000000000 +0200
> > @@ -258,8 +258,6 @@ static struct page * balance_classzone(z
> >  	struct page * page = NULL;
> >  	int __freed;
> >  
> > -	if (!(gfp_mask & __GFP_WAIT))
> > -		goto out;
> >  	if (in_interrupt())
> >  		BUG();
> 
> Andrea,
> 
> This cleanup leaves the 'out' label unused, triggering
> yet another gcc warning (though less scary than the previous).
> Please apply this cleanup patch on top of the one above.

yes, I recall I also cleaned up that bit in a later patch. And yes, it
was harmless too ;)

> 
> /Mikael
> 
> --- linux-2.4.23-pre4/mm/page_alloc.c.~1~	2003-09-18 13:43:41.753607800 +0200
> +++ linux-2.4.23-pre4/mm/page_alloc.c	2003-09-18 13:55:46.785155159 +0200
> @@ -317,7 +317,6 @@
>  		}
>  		current->nr_local_pages = 0;
>  	}
> - out:
>  	*freed = __freed;
>  	return page;
>  }


Andrea

/*
 * If you refuse to depend on closed software for a critical
 * part of your business, these links may be useful:
 *
 * rsync.kernel.org::pub/scm/linux/kernel/bkcvs/linux-2.5/
 * rsync.kernel.org::pub/scm/linux/kernel/bkcvs/linux-2.4/
 * http://www.cobite.com/cvsps/
 *
 * svn://svn.kernel.org/linux-2.6/trunk
 * svn://svn.kernel.org/linux-2.4/trunk
 */