[PATCH] mm: get_nid_for_pfn() returns int

[PATCH] mm: get_nid_for_pfn() returns int

[Roel Kluin]

get_nid_for_pfn() returns int

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
vi drivers/base/node.c +256
static int get_nid_for_pfn(unsigned long pfn)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 43fa90b..f8f578a 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -303,7 +303,7 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
 	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
 	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
 	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
-		unsigned int nid;
+		int nid;
 
 		nid = get_nid_for_pfn(pfn);
 		if (nid < 0)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Gary Hade]

On Sun, Jan 18, 2009 at 11:36:28PM +0100, Roel Kluin wrote:
> get_nid_for_pfn() returns int
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> vi drivers/base/node.c +256
> static int get_nid_for_pfn(unsigned long pfn)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 43fa90b..f8f578a 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -303,7 +303,7 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
>  	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
>  	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
>  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> -		unsigned int nid;
> +		int nid;
> 
>  		nid = get_nid_for_pfn(pfn);
>  		if (nid < 0)

My mistake.  Good catch.

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Andrew Morton]

On Mon, 19 Jan 2009 09:59:19 -0800 Gary Hade <garyhade@us.ibm.com> wrote:

> On Sun, Jan 18, 2009 at 11:36:28PM +0100, Roel Kluin wrote:
> > get_nid_for_pfn() returns int
> > 
> > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > ---
> > vi drivers/base/node.c +256
> > static int get_nid_for_pfn(unsigned long pfn)
> > 
> > diff --git a/drivers/base/node.c b/drivers/base/node.c
> > index 43fa90b..f8f578a 100644
> > --- a/drivers/base/node.c
> > +++ b/drivers/base/node.c
> > @@ -303,7 +303,7 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
> >  	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
> >  	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
> >  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> > -		unsigned int nid;
> > +		int nid;
> > 
> >  		nid = get_nid_for_pfn(pfn);
> >  		if (nid < 0)
> 
> My mistake.  Good catch.
> 

Presumably the (nid < 0) case has never happened.

Should we retain the test?

Is silently skipping the node in that case desirable behaviour?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Gary Hade]

On Mon, Jan 26, 2009 at 10:33:50PM -0800, Andrew Morton wrote:
> On Mon, 19 Jan 2009 09:59:19 -0800 Gary Hade <garyhade@us.ibm.com> wrote:
> 
> > On Sun, Jan 18, 2009 at 11:36:28PM +0100, Roel Kluin wrote:
> > > get_nid_for_pfn() returns int
> > > 
> > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > > ---
> > > vi drivers/base/node.c +256
> > > static int get_nid_for_pfn(unsigned long pfn)
> > > 
> > > diff --git a/drivers/base/node.c b/drivers/base/node.c
> > > index 43fa90b..f8f578a 100644
> > > --- a/drivers/base/node.c
> > > +++ b/drivers/base/node.c
> > > @@ -303,7 +303,7 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
> > >  	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
> > >  	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
> > >  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> > > -		unsigned int nid;
> > > +		int nid;
> > > 
> > >  		nid = get_nid_for_pfn(pfn);
> > >  		if (nid < 0)
> > 
> > My mistake.  Good catch.
> > 
> 
> Presumably the (nid < 0) case has never happened.

We do know that it is happening on one system while creating
a symlink for a memory section so it should also happen on
the same system if unregister_mem_sect_under_nodes() were
called to remove the same symlink.

The test was actually added in response to a problem with an
earlier version reported by Yasunori Goto where one or more
of the leading pages of a memory section on the 2nd node of
one of his systems was uninitialized because I believe they
coincided with a memory hole.  The earlier version did not
ignore uninitialized pages and determined the nid by considering
only the 1st page of each memory section.  This caused the
symlink to the 1st memory section on the 2nd node to be 
incorrectly created in /sys/devices/system/node/node0 instead
of /sys/devices/system/node/node1.  The problem was fixed by
adding the test to skip over uninitialized pages.

I suspect we have not seen any reports of the non-removal
of a symlink due to the incorrect declaration of the nid
variable in unregister_mem_sect_under_nodes() because
  - systems where a memory section could have an uninitialized
    range of leading pages are probably rare.
  - memory remove is probably not done very frequently on the
    systems that are capable of demonstrating the problem.
  - lingering symlink(s) that should have been removed may
    have simply gone unnoticed.
> 
> Should we retain the test?

Yes.

> 
> Is silently skipping the node in that case desirable behaviour?

It actually silently skips pages (not nodes) in it's quest
for valid nids for all the nodes that the memory section scans.
This is definitely desirable.

I hope this answers your questions.

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Yasunori Goto]

> On Mon, Jan 26, 2009 at 10:33:50PM -0800, Andrew Morton wrote:
> > On Mon, 19 Jan 2009 09:59:19 -0800 Gary Hade <garyhade@us.ibm.com> wrote:
> > 
> > > On Sun, Jan 18, 2009 at 11:36:28PM +0100, Roel Kluin wrote:
> > > > get_nid_for_pfn() returns int
> > > > 
> > > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > > > ---
> > > > vi drivers/base/node.c +256
> > > > static int get_nid_for_pfn(unsigned long pfn)
> > > > 
> > > > diff --git a/drivers/base/node.c b/drivers/base/node.c
> > > > index 43fa90b..f8f578a 100644
> > > > --- a/drivers/base/node.c
> > > > +++ b/drivers/base/node.c
> > > > @@ -303,7 +303,7 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
> > > >  	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
> > > >  	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
> > > >  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> > > > -		unsigned int nid;
> > > > +		int nid;
> > > > 
> > > >  		nid = get_nid_for_pfn(pfn);
> > > >  		if (nid < 0)
> > > 
> > > My mistake.  Good catch.
> > > 
> > 
> > Presumably the (nid < 0) case has never happened.
> 
> We do know that it is happening on one system while creating
> a symlink for a memory section so it should also happen on
> the same system if unregister_mem_sect_under_nodes() were
> called to remove the same symlink.
> 
> The test was actually added in response to a problem with an
> earlier version reported by Yasunori Goto where one or more
> of the leading pages of a memory section on the 2nd node of
> one of his systems was uninitialized because I believe they
> coincided with a memory hole. 

Yes. There are some memory hole pages which are occupied by firmware in
our box.

-- 
Yasunori Goto 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[roel kluin]

>> > > get_nid_for_pfn() returns int

>> > My mistake.  Good catch.

>> Presumably the (nid < 0) case has never happened.
>
> We do know that it is happening on one system while creating
> a symlink for a memory section so it should also happen on
> the same system if unregister_mem_sect_under_nodes() were
> called to remove the same symlink.
>
> The test was actually added in response to a problem with an
> earlier version reported by Yasunori Goto where one or more
> of the leading pages of a memory section on the 2nd node of
> one of his systems was uninitialized because I believe they
> coincided with a memory hole.  The earlier version did not
> ignore uninitialized pages and determined the nid by considering
> only the 1st page of each memory section.  This caused the
> symlink to the 1st memory section on the 2nd node to be
> incorrectly created in /sys/devices/system/node/node0 instead
> of /sys/devices/system/node/node1.  The problem was fixed by
> adding the test to skip over uninitialized pages.
>
> I suspect we have not seen any reports of the non-removal
> of a symlink due to the incorrect declaration of the nid
> variable in unregister_mem_sect_under_nodes() because
>  - systems where a memory section could have an uninitialized
>    range of leading pages are probably rare.
>  - memory remove is probably not done very frequently on the
>    systems that are capable of demonstrating the problem.
>  - lingering symlink(s) that should have been removed may
>    have simply gone unnoticed.
>>
>> Should we retain the test?
>
> Yes.
>
>>
>> Is silently skipping the node in that case desirable behaviour?
>
> It actually silently skips pages (not nodes) in it's quest
> for valid nids for all the nodes that the memory section scans.
> This is definitely desirable.
>
> I hope this answers your questions.

This still isn't applied, was it lost?

Roel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Gary Hade]

On Fri, Feb 27, 2009 at 03:56:40PM +0100, roel kluin wrote:
> >> > > get_nid_for_pfn() returns int
> 
> >> > My mistake.  Good catch.
> 
> >> Presumably the (nid < 0) case has never happened.
> >
> > We do know that it is happening on one system while creating
> > a symlink for a memory section so it should also happen on
> > the same system if unregister_mem_sect_under_nodes() were
> > called to remove the same symlink.
> >
> > The test was actually added in response to a problem with an
> > earlier version reported by Yasunori Goto where one or more
> > of the leading pages of a memory section on the 2nd node of
> > one of his systems was uninitialized because I believe they
> > coincided with a memory hole.  The earlier version did not
> > ignore uninitialized pages and determined the nid by considering
> > only the 1st page of each memory section.  This caused the
> > symlink to the 1st memory section on the 2nd node to be
> > incorrectly created in /sys/devices/system/node/node0 instead
> > of /sys/devices/system/node/node1.  The problem was fixed by
> > adding the test to skip over uninitialized pages.
> >
> > I suspect we have not seen any reports of the non-removal
> > of a symlink due to the incorrect declaration of the nid
> > variable in unregister_mem_sect_under_nodes() because
> >  - systems where a memory section could have an uninitialized
> >    range of leading pages are probably rare.
> >  - memory remove is probably not done very frequently on the
> >    systems that are capable of demonstrating the problem.
> >  - lingering symlink(s) that should have been removed may
> >    have simply gone unnoticed.
> >>
> >> Should we retain the test?
> >
> > Yes.
> >
> >>
> >> Is silently skipping the node in that case desirable behaviour?
> >
> > It actually silently skips pages (not nodes) in it's quest
> > for valid nids for all the nodes that the memory section scans.
> > This is definitely desirable.
> >
> > I hope this answers your questions.
> 
> This still isn't applied, was it lost?

It is still lingering in -mm:
http://userweb.kernel.org/~akpm/mmotm/broken-out/mm-get_nid_for_pfn-returns-int.patch

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Andrew Morton]

On Fri, 27 Feb 2009 13:33:40 -0800
Gary Hade <garyhade@us.ibm.com> wrote:

> On Fri, Feb 27, 2009 at 03:56:40PM +0100, roel kluin wrote:
> > >> > > get_nid_for_pfn() returns int
> > 
> > >> > My mistake. __Good catch.
> > 
> > >> Presumably the (nid < 0) case has never happened.
> > >
> > > We do know that it is happening on one system while creating
> > > a symlink for a memory section so it should also happen on
> > > the same system if unregister_mem_sect_under_nodes() were
> > > called to remove the same symlink.
> > >
> > > The test was actually added in response to a problem with an
> > > earlier version reported by Yasunori Goto where one or more
> > > of the leading pages of a memory section on the 2nd node of
> > > one of his systems was uninitialized because I believe they
> > > coincided with a memory hole. __The earlier version did not
> > > ignore uninitialized pages and determined the nid by considering
> > > only the 1st page of each memory section. __This caused the
> > > symlink to the 1st memory section on the 2nd node to be
> > > incorrectly created in /sys/devices/system/node/node0 instead
> > > of /sys/devices/system/node/node1. __The problem was fixed by
> > > adding the test to skip over uninitialized pages.
> > >
> > > I suspect we have not seen any reports of the non-removal
> > > of a symlink due to the incorrect declaration of the nid
> > > variable in unregister_mem_sect_under_nodes() because
> > > __- systems where a memory section could have an uninitialized
> > > __ __range of leading pages are probably rare.
> > > __- memory remove is probably not done very frequently on the
> > > __ __systems that are capable of demonstrating the problem.
> > > __- lingering symlink(s) that should have been removed may
> > > __ __have simply gone unnoticed.
> > >>
> > >> Should we retain the test?
> > >
> > > Yes.
> > >
> > >>
> > >> Is silently skipping the node in that case desirable behaviour?
> > >
> > > It actually silently skips pages (not nodes) in it's quest
> > > for valid nids for all the nodes that the memory section scans.
> > > This is definitely desirable.
> > >
> > > I hope this answers your questions.
> > 
> > This still isn't applied, was it lost?
> 
> It is still lingering in -mm:
> http://userweb.kernel.org/~akpm/mmotm/broken-out/mm-get_nid_for_pfn-returns-int.patch
> 

Should it unlinger?  I have it in the 2.6.30 pile.  Does it actually
fix a demonstrable bug?  

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Gary Hade]

On Fri, Feb 27, 2009 at 01:46:16PM -0800, Andrew Morton wrote:
> On Fri, 27 Feb 2009 13:33:40 -0800
> Gary Hade <garyhade@us.ibm.com> wrote:
> 
> > On Fri, Feb 27, 2009 at 03:56:40PM +0100, roel kluin wrote:
> > > >> > > get_nid_for_pfn() returns int
> > > 
> > > >> > My mistake. __Good catch.
> > > 
> > > >> Presumably the (nid < 0) case has never happened.
> > > >
> > > > We do know that it is happening on one system while creating
> > > > a symlink for a memory section so it should also happen on
> > > > the same system if unregister_mem_sect_under_nodes() were
> > > > called to remove the same symlink.
> > > >
> > > > The test was actually added in response to a problem with an
> > > > earlier version reported by Yasunori Goto where one or more
> > > > of the leading pages of a memory section on the 2nd node of
> > > > one of his systems was uninitialized because I believe they
> > > > coincided with a memory hole. __The earlier version did not
> > > > ignore uninitialized pages and determined the nid by considering
> > > > only the 1st page of each memory section. __This caused the
> > > > symlink to the 1st memory section on the 2nd node to be
> > > > incorrectly created in /sys/devices/system/node/node0 instead
> > > > of /sys/devices/system/node/node1. __The problem was fixed by
> > > > adding the test to skip over uninitialized pages.
> > > >
> > > > I suspect we have not seen any reports of the non-removal
> > > > of a symlink due to the incorrect declaration of the nid
> > > > variable in unregister_mem_sect_under_nodes() because
> > > > __- systems where a memory section could have an uninitialized
> > > > __ __range of leading pages are probably rare.
> > > > __- memory remove is probably not done very frequently on the
> > > > __ __systems that are capable of demonstrating the problem.
> > > > __- lingering symlink(s) that should have been removed may
> > > > __ __have simply gone unnoticed.
> > > >>
> > > >> Should we retain the test?
> > > >
> > > > Yes.
> > > >
> > > >>
> > > >> Is silently skipping the node in that case desirable behaviour?
> > > >
> > > > It actually silently skips pages (not nodes) in it's quest
> > > > for valid nids for all the nodes that the memory section scans.
> > > > This is definitely desirable.
> > > >
> > > > I hope this answers your questions.
> > > 
> > > This still isn't applied, was it lost?
> > 
> > It is still lingering in -mm:
> > http://userweb.kernel.org/~akpm/mmotm/broken-out/mm-get_nid_for_pfn-returns-int.patch
> > 
> 
> Should it unlinger?  I have it in the 2.6.30 pile.

Yes, that would be good. :)

> Does it actually fix a demonstrable bug?  

I am not aware of anyone that has actually reproduced the
problem.  I do not believe that we have any systems where 
it can be reproduced since it would require both
  (1) a memory section with an uninitialized range of
      pages and
  (2) a memory remove event for that memory section.
As far as I know, none of our systems have (1).  Yasunori Goto
has a system with (1) but I am not sure if he can do (2).

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Andrew Morton]

On Fri, 27 Feb 2009 16:14:00 -0800
Gary Hade <garyhade@us.ibm.com> wrote:

> On Fri, Feb 27, 2009 at 01:46:16PM -0800, Andrew Morton wrote:
>
> > > It is still lingering in -mm:
> > > http://userweb.kernel.org/~akpm/mmotm/broken-out/mm-get_nid_for_pfn-returns-int.patch
> > > 
> > 
> > Should it unlinger?  I have it in the 2.6.30 pile.
> 
> Yes, that would be good. :)

What would be good?  Your answer is ambiguous.

> > Does it actually fix a demonstrable bug?  
> 
> I am not aware of anyone that has actually reproduced the
> problem.

What problem?

All I gave at present is

  From: Roel Kluin <roel.kluin@gmail.com>

  get_nid_for_pfn() returns int

  Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
  Cc: Gary Hade <garyhade@us.ibm.com>

>  I do not believe that we have any systems where 
> it can be reproduced since it would require both
>   (1) a memory section with an uninitialized range of
>       pages and
>   (2) a memory remove event for that memory section.
> As far as I know, none of our systems have (1).  Yasunori Goto
> has a system with (1) but I am not sure if he can do (2).

Please send a new changelog for this patch.

If you believe this patch should be merged into 2.6.29 then please
explain why.  Please also consider whether it should be backported into
2.6.28.x and eariler.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Gary Hade]

On Fri, Feb 27, 2009 at 04:22:49PM -0800, Andrew Morton wrote:
> On Fri, 27 Feb 2009 16:14:00 -0800
> Gary Hade <garyhade@us.ibm.com> wrote:
> 
> > On Fri, Feb 27, 2009 at 01:46:16PM -0800, Andrew Morton wrote:
> >
> > > > It is still lingering in -mm:
> > > > http://userweb.kernel.org/~akpm/mmotm/broken-out/mm-get_nid_for_pfn-returns-int.patch
> > > > 
> > > 
> > > Should it unlinger?  I have it in the 2.6.30 pile.
> > 
> > Yes, that would be good. :)
> 
> What would be good?  Your answer is ambiguous.

Sorry, I was just trying to agree that your plan to wait
until 2.6.30 works for me.  Unless someone else objects
leave it in your 2.6.30 pile.

> 
> > > Does it actually fix a demonstrable bug?  
> > 
> > I am not aware of anyone that has actually reproduced the
> > problem.
> 
> What problem?

During a memory remove operation there is a chance on 
yet to be discovered system(s) that a mem section symlink
for a removed memory section could incorrectly persist.
Earlier in this thread I described the possible problem
as follows.
===
On Mon, Jan 26, 2009 at 10:33:50PM -0800, Andrew Morton wrote:
            ...
> Presumably the (nid < 0) case has never happened.

We do know that it is happening on one system while creating
a symlink for a memory section so it should also happen on
the same system if unregister_mem_sect_under_nodes() were
called to remove the same symlink.

The test was actually added in response to a problem with an
earlier version reported by Yasunori Goto where one or more
of the leading pages of a memory section on the 2nd node of
one of his systems was uninitialized because I believe they
coincided with a memory hole.  The earlier version did not
ignore uninitialized pages and determined the nid by considering
only the 1st page of each memory section.  This caused the
symlink to the 1st memory section on the 2nd node to be
incorrectly created in /sys/devices/system/node/node0 instead
of /sys/devices/system/node/node1.  The problem was fixed by
adding the test to skip over uninitialized pages.

I suspect we have not seen any reports of the non-removal
of a symlink due to the incorrect declaration of the nid
variable in unregister_mem_sect_under_nodes() because
  - systems where a memory section could have an uninitialized
    range of leading pages are probably rare.
  - memory remove is probably not done very frequently on the
    systems that are capable of demonstrating the problem.
  - lingering symlink(s) that should have been removed may
    have simply gone unnoticed.
===

> 
> All I gave at present is
> 
>   From: Roel Kluin <roel.kluin@gmail.com>
> 
>   get_nid_for_pfn() returns int
> 
>   Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>   Cc: Gary Hade <garyhade@us.ibm.com>
> 
> >  I do not believe that we have any systems where 
> > it can be reproduced since it would require both
> >   (1) a memory section with an uninitialized range of
> >       pages and
> >   (2) a memory remove event for that memory section.
> > As far as I know, none of our systems have (1).  Yasunori Goto
> > has a system with (1) but I am not sure if he can do (2).
> 
> Please send a new changelog for this patch.

Can you include the above words? 

> 
> If you believe this patch should be merged into 2.6.29 then please
> explain why.

2.6.30 is fine with me.

> Please also consider whether it should be backported into
> 2.6.28.x and eariler.

The "mm: show node to memory section relationship with symlinks
in sysfs" code that it improves was not introduced until 2.6.29-rc1.

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Andrew Morton]

On Fri, 27 Feb 2009 19:02:00 -0800 Gary Hade <garyhade@us.ibm.com> wrote:

> > > > Should it unlinger?  I have it in the 2.6.30 pile.
> > > 
> > > Yes, that would be good. :)
> > 
> > What would be good?  Your answer is ambiguous.
> 
> Sorry, I was just trying to agree that your plan to wait
> until 2.6.30 works for me.  Unless someone else objects
> leave it in your 2.6.30 pile.

I object ;)

The change is obviously correct, let's merge it now.

This could cause presently-working systems to stop working due to
hitherto-undiscovered bugs.  If so, sue me.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: get_nid_for_pfn() returns int

[Gary Hade]

On Fri, Feb 27, 2009 at 08:08:05PM -0800, Andrew Morton wrote:
> On Fri, 27 Feb 2009 19:02:00 -0800 Gary Hade <garyhade@us.ibm.com> wrote:
> 
> > > > > Should it unlinger?  I have it in the 2.6.30 pile.
> > > > 
> > > > Yes, that would be good. :)
> > > 
> > > What would be good?  Your answer is ambiguous.
> > 
> > Sorry, I was just trying to agree that your plan to wait
> > until 2.6.30 works for me.  Unless someone else objects
> > leave it in your 2.6.30 pile.
> 
> I object ;)
> 
> The change is obviously correct, let's merge it now.

Well, I wouldn't quibble with that. :)  Thanks!

> 
> This could cause presently-working systems to stop working due to
> hitherto-undiscovered bugs.  If so, sue me.

Extremely unlikely IMO.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>