diff mbox

174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

Message ID 20170109234039.mfefmv5dv4shxnfn@pd.tnic (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Borislav Petkov Jan. 9, 2017, 11:40 p.m. UTC
On Mon, Jan 09, 2017 at 03:32:04PM -0800, Paul E. McKenney wrote:
> We could move rcu_scheduler_starting() later, as long as there
> is no chance of preemption or context switch before it is invoked.
> Would that help in this case, or are we already context switching before
> acpi_os_map_cleanup() is invoked?  (If we are already context switching,
> short-circuiting synchronize_rcu_expedited() would be a bug.)

Hmm, how about the below?

It would still happen before

        /*
         * The boot idle thread must execute schedule()
         * at least once to get things moving:
         */
        init_idle_bootup_task(current);
        schedule_preempt_disabled();

in rest_init() and right after native_smp_prepare_cpus() which is where
we're splatting.

Lemme run it.

Even if it works, we would have to stress-test this seriously...

---

Comments

Paul E. McKenney Jan. 9, 2017, 11:52 p.m. UTC | #1
On Tue, Jan 10, 2017 at 12:40:39AM +0100, Borislav Petkov wrote:
> On Mon, Jan 09, 2017 at 03:32:04PM -0800, Paul E. McKenney wrote:
> > We could move rcu_scheduler_starting() later, as long as there
> > is no chance of preemption or context switch before it is invoked.
> > Would that help in this case, or are we already context switching before
> > acpi_os_map_cleanup() is invoked?  (If we are already context switching,
> > short-circuiting synchronize_rcu_expedited() would be a bug.)
> 
> Hmm, how about the below?
> 
> It would still happen before
> 
>         /*
>          * The boot idle thread must execute schedule()
>          * at least once to get things moving:
>          */
>         init_idle_bootup_task(current);
>         schedule_preempt_disabled();
> 
> in rest_init() and right after native_smp_prepare_cpus() which is where
> we're splatting.
> 
> Lemme run it.
> 
> Even if it works, we would have to stress-test this seriously...

Yeah, the call to wait_for_completion() at the beginning of
kernel_init_freeable() makes me extremely nervous.  Even if it does
happen to work, this looks like an accident waiting to happen.

Is it possible to instead move the ACPI initialization to follow the
workqueue initialization?

							Thanx, Paul

> ---
> diff --git a/init/main.c b/init/main.c
> index b0c9d6facef9..9be221cc87c3 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -385,7 +385,6 @@ static noinline void __ref rest_init(void)
>  {
>  	int pid;
> 
> -	rcu_scheduler_starting();
>  	/*
>  	 * We need to spawn init first so that it obtains pid 1, however
>  	 * the init task will end up wanting to create kthreads, which, if
> @@ -1019,6 +1018,8 @@ static noinline void __init kernel_init_freeable(void)
> 
>  	smp_prepare_cpus(setup_max_cpus);
> 
> +	rcu_scheduler_starting();
> +
>  	workqueue_init();
> 
>  	do_pre_smp_initcalls();
> 
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov Jan. 9, 2017, 11:52 p.m. UTC | #2
On Tue, Jan 10, 2017 at 12:40:39AM +0100, Borislav Petkov wrote:
> Lemme run it.

Well, it boots but I get:

[    0.291447] ------------[ cut here ]------------
[    0.291702] WARNING: CPU: 0 PID: 1 at kernel/rcu/tree.c:3993 rcu_scheduler_starting+0x5c/0x70
[    0.292107] Modules linked in:
[    0.292277] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.10.0-rc3+ #21
[    0.292540] Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.08 01/28/2016
[    0.292893] Call Trace:
[    0.293072]  ? dump_stack+0x46/0x63
[    0.293285]  ? __warn+0xec/0x110
[    0.293487]  ? rcu_scheduler_starting+0x5c/0x70
[    0.293735]  ? kernel_init_freeable+0x58/0x19a
[    0.293976]  ? rest_init+0x80/0x80
[    0.294153]  ? kernel_init+0xa/0x100
[    0.294334]  ? ret_from_fork+0x22/0x30
[    0.294525] ---[ end trace 4c0fe009ed4dc740 ]---

TBH, I like Rafael's suggestion in the other mail to stick with fixing
this in ACPI, especially this is an ACPI problem, not RCU. Well,
more or less: RCU could be taught to *not* do schedule_work() if
workqueue_init() hasn't happened yet but that's a tangential.

So, I'm going to bed. When I wake up, I want to see working fixes!

:-)))

Thanks dudes!
Lv Zheng Jan. 10, 2017, 12:44 a.m. UTC | #3
Hi,

Can the attached patch makes something different?

Thanks and best regards
Lv

> From: Borislav Petkov [mailto:bp@alien8.de]

> Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and

> early_acpi_os_unmap_memory() from Linux kernel

> 

> On Tue, Jan 10, 2017 at 12:40:39AM +0100, Borislav Petkov wrote:

> > Lemme run it.

> 

> Well, it boots but I get:

> 

> [    0.291447] ------------[ cut here ]------------

> [    0.291702] WARNING: CPU: 0 PID: 1 at kernel/rcu/tree.c:3993 rcu_scheduler_starting+0x5c/0x70

> [    0.292107] Modules linked in:

> [    0.292277] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.10.0-rc3+ #21

> [    0.292540] Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.08 01/28/2016

> [    0.292893] Call Trace:

> [    0.293072]  ? dump_stack+0x46/0x63

> [    0.293285]  ? __warn+0xec/0x110

> [    0.293487]  ? rcu_scheduler_starting+0x5c/0x70

> [    0.293735]  ? kernel_init_freeable+0x58/0x19a

> [    0.293976]  ? rest_init+0x80/0x80

> [    0.294153]  ? kernel_init+0xa/0x100

> [    0.294334]  ? ret_from_fork+0x22/0x30

> [    0.294525] ---[ end trace 4c0fe009ed4dc740 ]---

> 

> TBH, I like Rafael's suggestion in the other mail to stick with fixing

> this in ACPI, especially this is an ACPI problem, not RCU. Well,

> more or less: RCU could be taught to *not* do schedule_work() if

> workqueue_init() hasn't happened yet but that's a tangential.

> 

> So, I'm going to bed. When I wake up, I want to see working fixes!

> 

> :-)))

> 

> Thanks dudes!

> 

> --

> Regards/Gruss,

>     Boris.

> 

> Good mailing practices for 400: avoid top-posting and trim the reply.
diff mbox

Patch

diff --git a/init/main.c b/init/main.c
index b0c9d6facef9..9be221cc87c3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -385,7 +385,6 @@  static noinline void __ref rest_init(void)
 {
 	int pid;
 
-	rcu_scheduler_starting();
 	/*
 	 * We need to spawn init first so that it obtains pid 1, however
 	 * the init task will end up wanting to create kthreads, which, if
@@ -1019,6 +1018,8 @@  static noinline void __init kernel_init_freeable(void)
 
 	smp_prepare_cpus(setup_max_cpus);
 
+	rcu_scheduler_starting();
+
 	workqueue_init();
 
 	do_pre_smp_initcalls();