mm: Do not iterate over NR_CPUS in __zone_pcp_update()

mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Thomas Gleixner]

__zone_pcp_update() iterates over NR_CPUS instead of limiting the
access to the possible cpus. This might result in access to
uninitialized areas as the per cpu allocator only populates the per
cpu memory for possible cpus.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 mm/page_alloc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c
+++ linux-2.6/mm/page_alloc.c
@@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
 	int cpu;
 	unsigned long batch = zone_batchsize(zone), flags;
 
-	for (cpu = 0; cpu < NR_CPUS; cpu++) {
+	for_each_possible_cpu(cpu) {
 		struct per_cpu_pageset *pset;
 		struct per_cpu_pages *pcp;
 

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Pekka Enberg]

On Mon, Mar 8, 2010 at 11:21 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> __zone_pcp_update() iterates over NR_CPUS instead of limiting the
> access to the possible cpus. This might result in access to
> uninitialized areas as the per cpu allocator only populates the per
> cpu memory for possible cpus.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Looks OK to me.

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

> ---
>  mm/page_alloc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c
> +++ linux-2.6/mm/page_alloc.c
> @@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
>        int cpu;
>        unsigned long batch = zone_batchsize(zone), flags;
>
> -       for (cpu = 0; cpu < NR_CPUS; cpu++) {
> +       for_each_possible_cpu(cpu) {
>                struct per_cpu_pageset *pset;
>                struct per_cpu_pages *pcp;
>
>
> --
> 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>
>

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Christoph Lameter]

Acked-by: Christoph Lameter <cl@linux-foundation.org>


--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Mel Gorman]

On Mon, Mar 08, 2010 at 10:21:04AM +0100, Thomas Gleixner wrote:
> __zone_pcp_update() iterates over NR_CPUS instead of limiting the
> access to the possible cpus. This might result in access to
> uninitialized areas as the per cpu allocator only populates the per
> cpu memory for possible cpus.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Looks good.

Acked-by: Mel Gorman <mel@csn.ul.ie>

> ---
>  mm/page_alloc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c
> +++ linux-2.6/mm/page_alloc.c
> @@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
>  	int cpu;
>  	unsigned long batch = zone_batchsize(zone), flags;
>  
> -	for (cpu = 0; cpu < NR_CPUS; cpu++) {
> +	for_each_possible_cpu(cpu) {
>  		struct per_cpu_pageset *pset;
>  		struct per_cpu_pages *pcp;
>  
> 
> --
> 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>
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Tejun Heo]

On 03/08/2010 10:29 PM, Pekka Enberg wrote:
> On Mon, Mar 8, 2010 at 11:21 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> __zone_pcp_update() iterates over NR_CPUS instead of limiting the
>> access to the possible cpus. This might result in access to
>> uninitialized areas as the per cpu allocator only populates the per
>> cpu memory for possible cpus.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> Looks OK to me.
> 
> Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

Yeap, that's buggy.

Acked-by: Tejun Heo <tj@kernel.org>

I suppose this would go through the mm tree?

Thanks.

-- 
tejun

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[KOSAKI Motohiro]

> __zone_pcp_update() iterates over NR_CPUS instead of limiting the
> access to the possible cpus. This might result in access to
> uninitialized areas as the per cpu allocator only populates the per
> cpu memory for possible cpus.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  mm/page_alloc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks good.
	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

> 
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c
> +++ linux-2.6/mm/page_alloc.c
> @@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
>  	int cpu;
>  	unsigned long batch = zone_batchsize(zone), flags;
>  
> -	for (cpu = 0; cpu < NR_CPUS; cpu++) {
> +	for_each_possible_cpu(cpu) {
>  		struct per_cpu_pageset *pset;
>  		struct per_cpu_pages *pcp;
>  
> 
> --
> 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>



--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Christoph Lameter]

On Tue, 9 Mar 2010, Tejun Heo wrote:

> Yeap, that's buggy.
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
> I suppose this would go through the mm tree?

As you said: Its a bug so it needs to be applied to upstream.

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Pekka Enberg]

Christoph Lameter wrote:
> On Tue, 9 Mar 2010, Tejun Heo wrote:
> 
>> Yeap, that's buggy.
>>
>> Acked-by: Tejun Heo <tj@kernel.org>
>>
>> I suppose this would go through the mm tree?
> 
> As you said: Its a bug so it needs to be applied to upstream.

Sure but someone needs to pick up the patch and send it to Linus. Andrew?

			Pekka

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Andrew Morton]

On Tue, 09 Mar 2010 22:04:02 +0200
Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> Christoph Lameter wrote:
> > On Tue, 9 Mar 2010, Tejun Heo wrote:
> > 
> >> Yeap, that's buggy.
> >>
> >> Acked-by: Tejun Heo <tj@kernel.org>
> >>
> >> I suppose this would go through the mm tree?
> > 
> > As you said: Its a bug so it needs to be applied to upstream.
> 
> Sure but someone needs to pick up the patch and send it to Linus. Andrew?
> 

yup, I found the patch.

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Andrew Morton]

On Mon, 8 Mar 2010 10:21:04 +0100 (CET)
Thomas Gleixner <tglx@linutronix.de> wrote:

> __zone_pcp_update() iterates over NR_CPUS instead of limiting the
> access to the possible cpus. This might result in access to
> uninitialized areas as the per cpu allocator only populates the per
> cpu memory for possible cpus.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  mm/page_alloc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c
> +++ linux-2.6/mm/page_alloc.c
> @@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
>  	int cpu;
>  	unsigned long batch = zone_batchsize(zone), flags;
>  
> -	for (cpu = 0; cpu < NR_CPUS; cpu++) {
> +	for_each_possible_cpu(cpu) {
>  		struct per_cpu_pageset *pset;
>  		struct per_cpu_pages *pcp;
>  

I'm having trouble working out whether we want to backport this into
2.6.33.x or earlier.  Help?

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Christoph Lameter]

On Tue, 9 Mar 2010, Andrew Morton wrote:

> > __zone_pcp_update() iterates over NR_CPUS instead of limiting the
> > access to the possible cpus. This might result in access to
> > uninitialized areas as the per cpu allocator only populates the per
> > cpu memory for possible cpus.
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  mm/page_alloc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Index: linux-2.6/mm/page_alloc.c
> > ===================================================================
> > --- linux-2.6.orig/mm/page_alloc.c
> > +++ linux-2.6/mm/page_alloc.c
> > @@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
> >  	int cpu;
> >  	unsigned long batch = zone_batchsize(zone), flags;
> >
> > -	for (cpu = 0; cpu < NR_CPUS; cpu++) {
> > +	for_each_possible_cpu(cpu) {
> >  		struct per_cpu_pageset *pset;
> >  		struct per_cpu_pages *pcp;
> >
>
> I'm having trouble working out whether we want to backport this into
> 2.6.33.x or earlier.  Help?

Nope. This problem was created as a result of the dynamic allocation of
pagesets from percpu memory that went in during the merge window.
(99dcc3e5a94ed491fbef402831d8c0bbb267f995)

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Robert P. J. Day]

On Tue, 9 Mar 2010, Andrew Morton wrote:

> On Mon, 8 Mar 2010 10:21:04 +0100 (CET)
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > __zone_pcp_update() iterates over NR_CPUS instead of limiting the
> > access to the possible cpus. This might result in access to
> > uninitialized areas as the per cpu allocator only populates the per
> > cpu memory for possible cpus.
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  mm/page_alloc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Index: linux-2.6/mm/page_alloc.c
> > ===================================================================
> > --- linux-2.6.orig/mm/page_alloc.c
> > +++ linux-2.6/mm/page_alloc.c
> > @@ -3224,7 +3224,7 @@ static int __zone_pcp_update(void *data)
> >  	int cpu;
> >  	unsigned long batch = zone_batchsize(zone), flags;
> >
> > -	for (cpu = 0; cpu < NR_CPUS; cpu++) {
> > +	for_each_possible_cpu(cpu) {
> >  		struct per_cpu_pageset *pset;
> >  		struct per_cpu_pages *pcp;
>
> I'm having trouble working out whether we want to backport this into
> 2.6.33.x or earlier.  Help?

  given the above aesthetic mod, shouldn't that same change be applied
to *all* explicit loops of that form?  after all, checkpatch.pl warns
against it:

=====
# use of NR_CPUS is usually wrong
# ignore definitions of NR_CPUS and usage to define arrays as likely right
                if ($line =~ /\bNR_CPUS\b/ &&
                    $line !~ /^.\s*\s*#\s*if\b.*\bNR_CPUS\b/ &&
                    $line !~ /^.\s*\s*#\s*define\b.*\bNR_CPUS\b/ &&
                    $line !~ /^.\s*$Declare\s.*\[[^\]]*NR_CPUS[^\]]*\]/ &&
                    $line !~ /\[[^\]]*\.\.\.[^\]]*NR_CPUS[^\]]*\]/ &&
                    $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/)
                {
                        WARN("usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr);
                }
=====

rday
--

========================================================================
Robert P. J. Day                               Waterloo, Ontario, CANADA

            Linux Consulting, Training and Kernel Pedantry.

Web page:                                          http://crashcourse.ca
Twitter:                                       http://twitter.com/rpjday
========================================================================

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Christoph Lameter]

On Tue, 9 Mar 2010, Robert P. J. Day wrote:

> > I'm having trouble working out whether we want to backport this into
> > 2.6.33.x or earlier.  Help?
>
>   given the above aesthetic mod, shouldn't that same change be applied
> to *all* explicit loops of that form?  after all, checkpatch.pl warns
> against it:

The number of NR_CPUS should be significantly less after the percpu
rework. Would you audit the kernel for NR_CPUS use?

--
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: mm: Do not iterate over NR_CPUS in __zone_pcp_update()

[Robert P. J. Day]

On Tue, 9 Mar 2010, Christoph Lameter wrote:

> On Tue, 9 Mar 2010, Robert P. J. Day wrote:
>
> > > I'm having trouble working out whether we want to backport this into
> > > 2.6.33.x or earlier.  Help?
> >
> >   given the above aesthetic mod, shouldn't that same change be
> > applied to *all* explicit loops of that form?  after all,
> > checkpatch.pl warns against it:
>
> The number of NR_CPUS should be significantly less after the percpu
> rework. Would you audit the kernel for NR_CPUS use?

  i just did a simple grep for the obvious pattern:

$ grep -r "for.*NR_CPUS" *
arch/sparc/mm/init_64.c:	for (i = 0; i < NR_CPUS; i++)
arch/sparc/kernel/sun4d_smp.c:	for (i = 0; i < NR_CPUS; i++) {
arch/sparc/kernel/traps_64.c:	for (i = 0; i < NR_CPUS; i++) {
... etc etc ...

  most of the occurrences are under arch/.  as you say, after the
rework, most of those should be replaceable.

rday
--

========================================================================
Robert P. J. Day                               Waterloo, Ontario, CANADA

            Linux Consulting, Training and Kernel Pedantry.

Web page:                                          http://crashcourse.ca
Twitter:                                       http://twitter.com/rpjday
========================================================================

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