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

[Zheng, Lv]

Hi, Rafael and Paul

> From: Paul E. McKenney [mailto:paulmck@xxxxxxxxxxxxxxxxxx]
> 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 02:27:16AM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 10, 2017 at 12:52 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> > > 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!
> > >
> > > :-)))
> >
> > Well, if the https://patchwork.kernel.org/patch/9504277/ patch from Lv
> > worked, the attached one should work too (please test), but it can be
> > justified in a slightly more convincing way.
> >
> > Namely, the idea is that acpi_os_read/write_memory() should never be
> > used before invoking acpi_os_initialize() and since those are the only
> > places where the list of memory regions is walked under RCU without
> > extra locking, it is safe to skip the RCU synchronization until that
> > happens.
> >
> > Thanks,
> > Rafael
> 
> Makes sense to me!

Also looks good to me.

> 
> It looks like I can make the grace-period-free boot-time window
> for CONFIG_PREEMPT=y kernels quite a bit narrower, but it does not
> look like something suitable for jamming into 4.10.

OK, we can have this fixed in ACPI layer first.

Thanks
Lv

> 
> 							Thanx, Paul
> 
> > ---
> >  drivers/acpi/osl.c |    8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/acpi/osl.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/osl.c
> > +++ linux-pm/drivers/acpi/osl.c
> > @@ -378,7 +378,9 @@ static void acpi_os_drop_map_ref(struct
> >  static void acpi_os_map_cleanup(struct acpi_ioremap *map)
> >  {
> >  	if (!map->refcount) {
> > -		synchronize_rcu_expedited();
> > +		if (acpi_os_initialized)
> > +			synchronize_rcu_expedited();
> > +
> >  		acpi_unmap(map->phys, map->virt);
> >  		kfree(map);
> >  	}
> > @@ -671,6 +673,8 @@ acpi_os_read_memory(acpi_physical_addres
> >  	bool unmap = false;
> >  	u64 dummy;
> >
> > +	WARN_ON_ONCE(!acpi_os_initialized);
> > +
> >  	rcu_read_lock();
> >  	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
> >  	if (!virt_addr) {
> > @@ -716,6 +720,8 @@ acpi_os_write_memory(acpi_physical_addre
> >  	unsigned int size = width / 8;
> >  	bool unmap = false;
> >
> > +	WARN_ON_ONCE(!acpi_os_initialized);
> > +
> >  	rcu_read_lock();
> >  	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
> >  	if (!virt_addr) {