[PATCH] PPC64 mmu_context_init needs to run earlier

[PATCH] PPC64 mmu_context_init needs to run earlier

[Paul Mackerras]

This patch is from Nathan Lynch <nathanl@austin.ibm.com>.

This patch changes mmu_context_init to be called as a core_initcall
rather than an arch_initcall, since mmu_context_init needs to run
before we try to run any userspace processes, and arch_initcall was
found to be too late.

Signed-off-by: Nathan Lynch <nathanl@austin.ibm.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>

diff -puN arch/ppc64/mm/init.c~ppc64-make-mmu_context_init-core_initcall arch/ppc64/mm/init.c
--- linux-2.6.10-rc1-bk11/arch/ppc64/mm/init.c~ppc64-make-mmu_context_init-core_initcall	2004-11-01 19:51:46.000000000 -0600
+++ linux-2.6.10-rc1-bk11-nathanl/arch/ppc64/mm/init.c	2004-11-01 19:53:24.000000000 -0600
@@ -529,7 +529,7 @@ static int __init mmu_context_init(void)
 
 	return 0;
 }
-arch_initcall(mmu_context_init);
+core_initcall(mmu_context_init);
 
 /*
  * Do very early mm setup.

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Andrew Morton]

Paul Mackerras <paulus@samba.org> wrote:
>
> This patch changes mmu_context_init to be called as a core_initcall
>  rather than an arch_initcall, since mmu_context_init needs to run
>  before we try to run any userspace processes, and arch_initcall was
>  found to be too late.

Here we go again...

I don't see why your patch fixes the problem.  do_basic_setup() calls
driver_init() prior to any initcalls being run, and driver_init() can call
/sbin/hotplug.

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Nathan Lynch]

On Tue, 2004-11-02 at 00:13, Andrew Morton wrote:
> Paul Mackerras <paulus@samba.org> wrote:
> >
> > This patch changes mmu_context_init to be called as a core_initcall
> >  rather than an arch_initcall, since mmu_context_init needs to run
> >  before we try to run any userspace processes, and arch_initcall was
> >  found to be too late.
> 
> Here we go again...
> 
> I don't see why your patch fixes the problem.  do_basic_setup() calls
> driver_init() prior to any initcalls being run, and driver_init() can call
> /sbin/hotplug.

My original submission was more of a "works for me, not sure it's
correct" sort of thing.  Sorry I wasn't clear.

How about this instead:


Using idr_get_new_above in init_new_context lets us get rid of an
awkward init function which wasn't running early enough in boot
anyway.

Signed-off-by: Nathan Lynch <nathanl@austin.ibm.com>


---


diff -puN arch/ppc64/mm/init.c~ppc64-mmu-context-use-idr_get_new_above arch/ppc64/mm/init.c
--- linux-2.6.10-rc1-bk12/arch/ppc64/mm/init.c~ppc64-mmu-context-use-idr_get_new_above	2004-11-02 14:22:23.000000000 -0600
+++ linux-2.6.10-rc1-bk12-nathanl/arch/ppc64/mm/init.c	2004-11-02 14:29:09.000000000 -0600
@@ -489,7 +489,7 @@ again:
 		return -ENOMEM;
 
 	spin_lock(&mmu_context_lock);
-	err = idr_get_new(&mmu_context_idr, NULL, &index);
+	err = idr_get_new_above(&mmu_context_idr, NULL, 0, &index);
 	spin_unlock(&mmu_context_lock);
 
 	if (err == -EAGAIN)
@@ -518,19 +518,6 @@ void destroy_context(struct mm_struct *m
 	hugetlb_mm_free_pgd(mm);
 }
 
-static int __init mmu_context_init(void)
-{
-	int index;
-
-	/* Reserve the first (invalid) context*/
-	idr_pre_get(&mmu_context_idr, GFP_KERNEL);
-	idr_get_new(&mmu_context_idr, NULL, &index);
-	BUG_ON(0 != index);
-
-	return 0;
-}
-arch_initcall(mmu_context_init);
-
 /*
  * Do very early mm setup.
  */

_

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Linus Torvalds]

On Tue, 2 Nov 2004, Nathan Lynch wrote:
> 
> Using idr_get_new_above in init_new_context lets us get rid of an
> awkward init function which wasn't running early enough in boot
> anyway.

Ok, call me stupid, but what's the difference between

	idr_get_new(&mmu_context_idr, NULL, &index);

and

	idr_get_new_above(&mmu_context_idr, NULL, 0, &index);

because as far as I can tell, they are exactly the same.

They both just do a "idr_get_new_above_int(idp, ptr, 0)".

So I don't see why one would need an awkward init function, and the other
wouldn't..

That said, maybe the problem is that we shouldn't even get far enough into 
the fork() logic to ever get into a new MMU context if driver_init ends up 
being called before we're ready. 

		Linus

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Benjamin Herrenschmidt]

> That said, maybe the problem is that we shouldn't even get far enough into 
> the fork() logic to ever get into a new MMU context if driver_init ends up 
> being called before we're ready. 

Agreed. I chatted with Andrew about this, and I think we need
call_usermodehelper to be in a "plugged" state during boot, where it
queues up events but doesn't exec's userland. It remains to be decided
at what point during boot (during initcalls ? after initcalls) we can
"unplug" it tho...

I think it's definitely bogus to try to run userland in the middle of
arch_initcall's.... 

We need this plug/unplug logic (as I wrote separately to linux-pm) for
suspend as well, since we can't affort calling userlands once we have
started suspending devices (and frozen userland).

Ben.

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Nathan Lynch]

On Tue, 2004-11-02 at 16:10, Linus Torvalds wrote:
> On Tue, 2 Nov 2004, Nathan Lynch wrote:
> > 
> > Using idr_get_new_above in init_new_context lets us get rid of an
> > awkward init function which wasn't running early enough in boot
> > anyway.
> 
> Ok, call me stupid, but what's the difference between
> 
> 	idr_get_new(&mmu_context_idr, NULL, &index);
> 
> and
> 
> 	idr_get_new_above(&mmu_context_idr, NULL, 0, &index);
> 
> because as far as I can tell, they are exactly the same.
> 
> They both just do a "idr_get_new_above_int(idp, ptr, 0)".

Right, bad patch.  I was confused by this bit in idr.c:

/**
 * idr_get_new_above - allocate new idr entry above a start id

into thinking the new entry would be strictly greater than the start id.

Here's another attempt, using 1 instead of 0 for the start id.


Using idr_get_new_above in init_new_context lets us get rid of an
awkward init function which wasn't running early enough in boot
anyway.

Signed-off-by: Nathan Lynch <nathanl@austin.ibm.com>


---


diff -puN arch/ppc64/mm/init.c~ppc64-mmu-context-use-idr_get_new_above
arch/ppc64/mm/init.c
---
linux-2.6.10-rc1-bk12/arch/ppc64/mm/init.c~ppc64-mmu-context-use-idr_get_new_above	2004-11-02 16:28:57.000000000 -0600
+++ linux-2.6.10-rc1-bk12-nathanl/arch/ppc64/mm/init.c	2004-11-02
17:49:08.000000000 -0600
@@ -489,7 +489,7 @@ again:
 		return -ENOMEM;
 
 	spin_lock(&mmu_context_lock);
-	err = idr_get_new(&mmu_context_idr, NULL, &index);
+	err = idr_get_new_above(&mmu_context_idr, NULL, 1, &index);
 	spin_unlock(&mmu_context_lock);
 
 	if (err == -EAGAIN)
@@ -518,19 +518,6 @@ void destroy_context(struct mm_struct *m
 	hugetlb_mm_free_pgd(mm);
 }
 
-static int __init mmu_context_init(void)
-{
-	int index;
-
-	/* Reserve the first (invalid) context*/
-	idr_pre_get(&mmu_context_idr, GFP_KERNEL);
-	idr_get_new(&mmu_context_idr, NULL, &index);
-	BUG_ON(0 != index);
-
-	return 0;
-}
-arch_initcall(mmu_context_init);
-
 /*
  * Do very early mm setup.
  */

_

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Andrew Morton]

Paul Mackerras <paulus@samba.org> wrote:
>
> This patch is from Nathan Lynch <nathanl@austin.ibm.com>.
> 
> This patch changes mmu_context_init to be called as a core_initcall
> rather than an arch_initcall, since mmu_context_init needs to run
> before we try to run any userspace processes, and arch_initcall was
> found to be too late.
> 

This all seemed to peter out.  Did we end up with a more convincing patch?

> 
> diff -puN arch/ppc64/mm/init.c~ppc64-make-mmu_context_init-core_initcall arch/ppc64/mm/init.c
> --- linux-2.6.10-rc1-bk11/arch/ppc64/mm/init.c~ppc64-make-mmu_context_init-core_initcall	2004-11-01 19:51:46.000000000 -0600
> +++ linux-2.6.10-rc1-bk11-nathanl/arch/ppc64/mm/init.c	2004-11-01 19:53:24.000000000 -0600
> @@ -529,7 +529,7 @@ static int __init mmu_context_init(void)
>  
>  	return 0;
>  }
> -arch_initcall(mmu_context_init);
> +core_initcall(mmu_context_init);
>  
>  /*
>   * Do very early mm setup.

Re: [PATCH] PPC64 mmu_context_init needs to run earlier

[Paul Mackerras]

Andrew Morton writes:

> This all seemed to peter out.  Did we end up with a more convincing patch?

Yes, the one from Nathan Lynch that uses idr_get_new_above with an
argument of 1 rather than 0.  Here is the patch:

Using idr_get_new_above in init_new_context lets us get rid of an
awkward init function which wasn't running early enough in boot
anyway.

Signed-off-by: Nathan Lynch <nathanl@austin.ibm.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>

diff -puN arch/ppc64/mm/init.c~ppc64-mmu-context-use-idr_get_new_above
arch/ppc64/mm/init.c
---
linux-2.6.10-rc1-bk12/arch/ppc64/mm/init.c~ppc64-mmu-context-use-idr_get_new_above	2004-11-02 16:28:57.000000000 -0600
+++ linux-2.6.10-rc1-bk12-nathanl/arch/ppc64/mm/init.c	2004-11-02
17:49:08.000000000 -0600
@@ -489,7 +489,7 @@ again:
 		return -ENOMEM;
 
 	spin_lock(&mmu_context_lock);
-	err = idr_get_new(&mmu_context_idr, NULL, &index);
+	err = idr_get_new_above(&mmu_context_idr, NULL, 1, &index);
 	spin_unlock(&mmu_context_lock);
 
 	if (err == -EAGAIN)
@@ -518,19 +518,6 @@ void destroy_context(struct mm_struct *m
 	hugetlb_mm_free_pgd(mm);
 }
 
-static int __init mmu_context_init(void)
-{
-	int index;
-
-	/* Reserve the first (invalid) context*/
-	idr_pre_get(&mmu_context_idr, GFP_KERNEL);
-	idr_get_new(&mmu_context_idr, NULL, &index);
-	BUG_ON(0 != index);
-
-	return 0;
-}
-arch_initcall(mmu_context_init);
-
 /*
  * Do very early mm setup.
  */