The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Dave Hansen <haveblue@us.ibm.com>
To: Yasunori Goto <y-goto@jp.fujitsu.com>
Cc: Andrew Morton <akpm@osdl.org>,
	Linux Kernel ML <linux-kernel@vger.kernel.org>
Subject: Re: [Patch]Fix spanned_pages is not updated at a case of memory hot-add.
Date: Tue, 23 May 2006 09:28:09 -0700	[thread overview]
Message-ID: <1148401689.8658.12.camel@localhost.localdomain> (raw)
In-Reply-To: <20060523170830.97E1.Y-GOTO@jp.fujitsu.com>

On Tue, 2006-05-23 at 17:29 +0900, Yasunori Goto wrote:
> Hello.
> 
> I found there is a bug in grow_zone_span() and grow_pgdat_span().
...
> @@ -95,16 +95,18 @@ EXPORT_SYMBOL_GPL(__add_pages);
>  static void grow_zone_span(struct zone *zone,
>  		unsigned long start_pfn, unsigned long end_pfn)
>  {
> -	unsigned long old_zone_end_pfn;
> +	unsigned long new_zone_end_pfn;
>  
>  	zone_span_writelock(zone);
>  
> -	old_zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +	new_zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;

I really don't like the idea of having this variable called "new_"
something.  That implies that this is what the new end_pfn is going to
be.  The *new* one.  In reality, it is what it _might_ have been.  How
about "tmp_zone_end_pfn"?

This practice of dealing with spanned_pages is a real pain. 

I generally try to avoid max/min in code, but this struck me as possibly
being useful.  Do you find this easier to read, or your patch?

diff -puN mm/memory_hotplug.c~fix-spanned-pages mm/memory_hotplug.c
--- work/mm/memory_hotplug.c~fix-spanned-pages	2006-05-23 09:04:31.000000000 -0700
+++ work-dave/mm/memory_hotplug.c	2006-05-23 09:22:18.000000000 -0700
@@ -91,8 +91,8 @@ static void grow_zone_span(struct zone *
 	if (start_pfn < zone->zone_start_pfn)
 		zone->zone_start_pfn = start_pfn;
 
-	if (end_pfn > old_zone_end_pfn)
-		zone->spanned_pages = end_pfn - zone->zone_start_pfn;
+	zone->spanned_pages = max(old_zone_end_pfn, end_pfn) -
+				zone->zone_start_pfn);
 
 	zone_span_writeunlock(zone);
 }
@@ -106,8 +106,8 @@ static void grow_pgdat_span(struct pglis
 	if (start_pfn < pgdat->node_start_pfn)
 		pgdat->node_start_pfn = start_pfn;
 
-	if (end_pfn > old_pgdat_end_pfn)
-		pgdat->node_spanned_pages = end_pfn - pgdat->node_start_pfn;
+	pgdat->node_spanned_pages = max(old_pgdat_end_pfn, end_pfn) -
+					pgdat->node_start_pfn;
 }
 
 int online_pages(unsigned long pfn, unsigned long nr_pages)
_


-- Dave


  reply	other threads:[~2006-05-23 16:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-23  8:29 [Patch]Fix spanned_pages is not updated at a case of memory hot-add Yasunori Goto
2006-05-23 16:28 ` Dave Hansen [this message]
2006-05-24  1:18   ` Yasunori Goto
2006-05-24  5:05     ` Dave Hansen
2006-05-24  5:26       ` Yasunori Goto
2006-05-26  4:46       ` [Patch]Fix spanned_pages is not updated at a case of memory hot-add take 2 Yasunori Goto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1148401689.8658.12.camel@localhost.localdomain \
    --to=haveblue@us.ibm.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=y-goto@jp.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox