This patch is from Nathan Lynch <[email protected]>.
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 <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
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.
Paul Mackerras <[email protected]> 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.
On Tue, 2004-11-02 at 00:13, Andrew Morton wrote:
> Paul Mackerras <[email protected]> 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 <[email protected]>
---
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.
*/
_
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
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 <[email protected]>
---
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.
*/
_
> 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.
Paul Mackerras <[email protected]> wrote:
>
> This patch is from Nathan Lynch <[email protected]>.
>
> 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.
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 <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
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.
*/