LMB bits

LMB bits

[Benjamin Herrenschmidt]

So I've pushed the memblocks rename and the modified version of my
patches that Yinghai produced in the "memblock" branch of the
powerpc.git tree, based on top of current Linus tree.

I've left out all the other patches for x86.

I did some compile tests and all my test configs on powerpc pass. I
won't have time to do in depth runtime tests but I will eventually get
to it.

Now we really need the other archs affected by my LMB rework to test
that I haven't broken things for them :-)

I'm especially worried with things like sparc NUMA bits. I think I got
it right but I couldn't test.

So those patches aren't yet candidate for next, though they will be with
a tad more testing.

Cheers,
Ben.

Re: LMB bits

[Ingo Molnar]

* Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> So I've pushed the memblocks rename and the modified version of my patches 
> that Yinghai produced in the "memblock" branch of the powerpc.git tree, 
> based on top of current Linus tree.

I guess that means this URI:

 git pull git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git memblock

right?

> I've left out all the other patches for x86.
> 
> I did some compile tests and all my test configs on powerpc pass. I won't 
> have time to do in depth runtime tests but I will eventually get to it.
> 
> Now we really need the other archs affected by my LMB rework to test that I 
> haven't broken things for them :-)
> 
> I'm especially worried with things like sparc NUMA bits. I think I got it 
> right but I couldn't test.
> 
> So those patches aren't yet candidate for next, though they will be with a 
> tad more testing.

Thanks Ben.

Dave, can you confirm that it works fine on Sparc? If it does then i'd like to 
pull your tree and use it as a base from that point on. I can push it to 
linux-next as well and treat it together with Yinghai's x86 changes. Would 
that be fine with you, or would you like to push it to Linus?

Also, after some testing we could send the tree cut at this point:

  506e7de: lmb: rename to memblock

to Linus straight away - to get the big rename out of the way:

 74 files changed, 1039 insertions(+), 1039 deletions(-)

Hm?

	Ingo

Re: LMB bits

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 9 Jul 2010 09:24:08 +0200

> Dave, can you confirm that it works fine on Sparc?

I'm about to hit bed, so I'll test it out first thing when I wake
up.

Re: LMB bits

[Ingo Molnar]

* David Miller <davem@davemloft.net> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Date: Fri, 9 Jul 2010 09:24:08 +0200
> 
> > Dave, can you confirm that it works fine on Sparc?
> 
> I'm about to hit bed, so I'll test it out first thing when I wake
> up.

Thanks Dave!

	Ingo

Re: LMB bits

[Geert Uytterhoeven]

On Fri, Jul 9, 2010 at 09:24, Ingo Molnar <mingo@elte.hu> wrote:
> * Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> So I've pushed the memblocks rename and the modified version of my patches
>> that Yinghai produced in the "memblock" branch of the powerpc.git tree,
>> based on top of current Linus tree.

> to Linus straight away - to get the big rename out of the way:
>
>  74 files changed, 1039 insertions(+), 1039 deletions(-)

Seems I have missed that part of the discussion: why is lmb renamed?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: LMB bits

[Ingo Molnar]

* Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On Fri, Jul 9, 2010 at 09:24, Ingo Molnar <mingo@elte.hu> wrote:
> > * Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> >> So I've pushed the memblocks rename and the modified version of my patches
> >> that Yinghai produced in the "memblock" branch of the powerpc.git tree,
> >> based on top of current Linus tree.
> 
> > to Linus straight away - to get the big rename out of the way:
> >
> > ??74 files changed, 1039 insertions(+), 1039 deletions(-)
> 
> Seems I have missed that part of the discussion: why is lmb renamed?

The reason is to make it more apparent at first glance what it is all about, 
to reduce the pool of silly abbreviations that only mean something to those 
deeply involved with that code.

Thanks,

	Ingo

Re: LMB bits

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 9 Jul 2010 09:24:08 +0200

> Dave, can you confirm that it works fine on Sparc?

It hangs very eary in the boot, I'll try to bisect.

Re: LMB bits

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Fri, 09 Jul 2010 17:20:50 -0700 (PDT)

> From: Ingo Molnar <mingo@elte.hu>
> Date: Fri, 9 Jul 2010 09:24:08 +0200
> 
>> Dave, can you confirm that it works fine on Sparc?
> 
> It hangs very eary in the boot, I'll try to bisect.

A quick update, I'm still chipping away to find the culprit,
I know so far that things are ok at least 14 patches into the
series.

I'll work more to try and track down the exact problem tomorrow.

Re: LMB bits

[Benjamin Herrenschmidt]

On Fri, 2010-07-09 at 20:55 -0700, David Miller wrote:
> A quick update, I'm still chipping away to find the culprit,
> I know so far that things are ok at least 14 patches into the
> series.
> 
> I'll work more to try and track down the exact problem tomorrow. 

Thanks Dave !

To save you that burden in the future, do you know if there's a sparc64
box somewhere I can access to do tests ? Or an emulator maybe ?

Cheers,
Ben.

Re: LMB bits

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Sat, 10 Jul 2010 21:01:59 +1000

> On Fri, 2010-07-09 at 20:55 -0700, David Miller wrote:
>> A quick update, I'm still chipping away to find the culprit,
>> I know so far that things are ok at least 14 patches into the
>> series.
>> 
>> I'll work more to try and track down the exact problem tomorrow. 
> 
> Thanks Dave !

The first guilty commit seems to be:

--------------------
From 869146ec57f449a5dd4bb5d939361841f4f7f3c6 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 6 Jul 2010 15:39:08 -0700
Subject: [PATCH 01/12] memblock: Make memblock_find_region() out of memblock_alloc_region()

This function will be used to locate a free area to put the new memblock
arrays when attempting to resize them. memblock_alloc_region() is gone,
the two callsites now call memblock_add_region().

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
--------------------

I wonder if there is some subtle way you've changed behavior here?
Previously if alloc block fails to do the add, it continues and
keeps trying.

Whereas now this add call happens a level higher and it's immediately
signalled as a hard failure.

Oh I see, your conversion of membase_alloc_nid_region() is buggy.
Instead of passing in "ret" as the base address to memblock_add_region(),
you're passing in the original value, "start".

The other conversion was done correctly, which is why you didn't see
this problem obviously :)

I'll make this fix and see how much further I can get.

> To save you that burden in the future, do you know if there's a sparc64
> box somewhere I can access to do tests ? Or an emulator maybe ?

Contact me on IRC sometime and I'll try to set you up with a machine
you can easily remotely reboot, power-on, and power-off to test
kernels on.

Re: LMB bits

[Yinghai Lu]

On 07/10/2010 01:14 PM, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Sat, 10 Jul 2010 21:01:59 +1000
> 
>> On Fri, 2010-07-09 at 20:55 -0700, David Miller wrote:
>>> A quick update, I'm still chipping away to find the culprit,
>>> I know so far that things are ok at least 14 patches into the
>>> series.
>>>
>>> I'll work more to try and track down the exact problem tomorrow. 
>>
>> Thanks Dave !
> 
> The first guilty commit seems to be:
> 
> --------------------
>>From 869146ec57f449a5dd4bb5d939361841f4f7f3c6 Mon Sep 17 00:00:00 2001
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Tue, 6 Jul 2010 15:39:08 -0700
> Subject: [PATCH 01/12] memblock: Make memblock_find_region() out of memblock_alloc_region()
> 
> This function will be used to locate a free area to put the new memblock
> arrays when attempting to resize them. memblock_alloc_region() is gone,
> the two callsites now call memblock_add_region().
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> --------------------
> 
> I wonder if there is some subtle way you've changed behavior here?
> Previously if alloc block fails to do the add, it continues and
> keeps trying.
> 
> Whereas now this add call happens a level higher and it's immediately
> signalled as a hard failure.
> 
> Oh I see, your conversion of membase_alloc_nid_region() is buggy.
> Instead of passing in "ret" as the base address to memblock_add_region(),
> you're passing in the original value, "start".
> 
> The other conversion was done correctly, which is why you didn't see
> this problem obviously :)
> 
> I'll make this fix and see how much further I can get.

Ben/David, 

can you fold in following patch in your patchset too?

also please use -v2 [PATCH] lmb: rename to memblock
that is removing one wrong change with microblaze dts

Thanks

Yinghai


Subject: [PATCH] memblock: memblock_find_base() should return MEMBLOCK_ERROR on failing path

all callees assume it return MEMBLOCK_ERROR when it fail to find a range

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 mm/memblock.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -154,7 +154,7 @@ static phys_addr_t __init memblock_find_
 		if (found != MEMBLOCK_ERROR)
 			return found;
 	}
-	return 0;
+	return MEMBLOCK_ERROR;
 }
 
 static void memblock_remove_region(struct memblock_type *type, unsigned long r)

Re: LMB bits

[Benjamin Herrenschmidt]

On Sat, 2010-07-10 at 13:53 -0700, Yinghai Lu wrote:

> Ben/David, 
> 
> can you fold in following patch in your patchset too?
> 
> also please use -v2 [PATCH] lmb: rename to memblock
> that is removing one wrong change with microblaze dts

Will do. I'm also fixing another bug David found. I'll have an updated
branch later today.

Cheers,
Ben.

> Thanks
> 
> Yinghai
> 
> 
> Subject: [PATCH] memblock: memblock_find_base() should return MEMBLOCK_ERROR on failing path
> 
> all callees assume it return MEMBLOCK_ERROR when it fail to find a range
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  mm/memblock.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/mm/memblock.c
> ===================================================================
> --- linux-2.6.orig/mm/memblock.c
> +++ linux-2.6/mm/memblock.c
> @@ -154,7 +154,7 @@ static phys_addr_t __init memblock_find_
>  		if (found != MEMBLOCK_ERROR)
>  			return found;
>  	}
> -	return 0;
> +	return MEMBLOCK_ERROR;
>  }
>  
>  static void memblock_remove_region(struct memblock_type *type, unsigned long r)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arch" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: LMB bits

[Benjamin Herrenschmidt]

On Mon, 2010-07-12 at 13:24 +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2010-07-10 at 13:53 -0700, Yinghai Lu wrote:
> 
> > Ben/David, 
> > 
> > can you fold in following patch in your patchset too?
> > 
> > also please use -v2 [PATCH] lmb: rename to memblock
> > that is removing one wrong change with microblaze dts
> 
> Will do. I'm also fixing another bug David found. I'll have an updated
> branch later today.

Ok pushed.

Yinghai, I've left your patch on the top of the branch rather than
squashing it for now.

Cheers,
Ben.

Re: LMB bits

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 12 Jul 2010 17:38:06 +1000

> Ok pushed.

Your current tree headed by 94f4de71740edb80dfbd3916fe83308d545a4419
works fine for me.

Re: LMB bits

[Benjamin Herrenschmidt]

On Mon, 2010-07-12 at 15:34 -0700, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Mon, 12 Jul 2010 17:38:06 +1000
> 
> > Ok pushed.
> 
> Your current tree headed by 94f4de71740edb80dfbd3916fe83308d545a4419
> works fine for me.

Thanks Dave.

Ingo, Linus, how to you want to proceed ? We could get the rename patch
in now, that would make things easier for the ARM folks who have started
using LMB as well and are currently clashing in linux-next, and keep the
rest of the rework for the merge window ? Or keep the whole thing for
the merge window ?

Cheers,
Ben.

Re: LMB bits

[Ingo Molnar]

* Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Mon, 2010-07-12 at 15:34 -0700, David Miller wrote:
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Date: Mon, 12 Jul 2010 17:38:06 +1000
> > 
> > > Ok pushed.
> > 
> > Your current tree headed by 94f4de71740edb80dfbd3916fe83308d545a4419
> > works fine for me.
> 
> Thanks Dave.
> 
> Ingo, Linus, how to you want to proceed ? We could get the rename patch in 
> now, that would make things easier for the ARM folks who have started using 
> LMB as well and are currently clashing in linux-next, and keep the rest of 
> the rework for the merge window ? Or keep the whole thing for the merge 
> window ?

I'd suggest you send a pull request to Linus with the rename - it is purely 
mechanical, tested and it will make life much easier in the merge window. 
Also, we are shortly after -rc5 now so should there be any fallout, there's 
enough time until -rc6.

Thanks,

	Ingo

Re: LMB bits

[Linus Torvalds]

On Tue, Jul 13, 2010 at 12:12 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> Ingo, Linus, how to you want to proceed ? We could get the rename patch
> in now, that would make things easier for the ARM folks who have started
> using LMB as well and are currently clashing in linux-next, and keep the
> rest of the rework for the merge window ? Or keep the whole thing for
> the merge window ?

If it's just a rename, and the accidental lmb renames have been all
sorted out, then I'm ok with taking a pure rename that changes nothing
else now, just to make things easier to merge later.

                        Linus

Re: LMB bits

[David Miller]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 13 Jul 2010 08:11:57 -0700

> On Tue, Jul 13, 2010 at 12:12 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>>
>> Ingo, Linus, how to you want to proceed ? We could get the rename patch
>> in now, that would make things easier for the ARM folks who have started
>> using LMB as well and are currently clashing in linux-next, and keep the
>> rest of the rework for the merge window ? Or keep the whole thing for
>> the merge window ?
> 
> If it's just a rename, and the accidental lmb renames have been all
> sorted out, then I'm ok with taking a pure rename that changes nothing
> else now, just to make things easier to merge later.

I'd also be real happy if we did this now too.