This patch avoids that the following warning is reported during hibernation:
WARNING: CPU: 0 PID: 612 at kernel/time/timekeeping.c:751 ktime_get+0x116/0x120 RIP: 0010:ktime_get+0x116/0x120 Call Trace: acpi_os_get_timer+0xe/0x30 acpi_ds_exec_begin_control_op+0x175/0x1de acpi_ds_exec_begin_op+0x2c7/0x39a acpi_ps_create_op+0x573/0x5e4 acpi_ps_parse_loop+0x349/0x1220 acpi_ps_parse_aml+0x25b/0x6da acpi_ps_execute_method+0x327/0x41b acpi_ns_evaluate+0x4e9/0x6f5 acpi_ut_evaluate_object+0xd9/0x2f2 acpi_rs_get_method_data+0x8f/0x114 acpi_walk_resources+0x122/0x1b6 acpi_pci_link_get_current.isra.2+0x157/0x280 acpi_pci_link_set+0x32f/0x4a0 irqrouter_resume+0x58/0x80 syscore_resume+0x84/0x380 hibernation_snapshot+0x20c/0x4f0 hibernate+0x22d/0x3a6 state_store+0x99/0xa0 kobj_attr_store+0x37/0x50 sysfs_kf_write+0x87/0xa0 kernfs_fop_write+0x1a5/0x240 __vfs_write+0xd2/0x410 vfs_write+0x101/0x250 ksys_write+0xab/0x120 __x64_sys_write+0x43/0x50 do_syscall_64+0x71/0x220 entry_SYSCALL_64_after_hwframe+0x49/0xbe
Reported-by: Fengguang Wu fengguang.wu@intel.com Fixes: 164a08cee135 ("ACPICA: Dispatcher: Introduce timeout mechanism for infinite loop detection") References: https://lists.01.org/pipermail/lkp/2018-April/008406.html Signed-off-by: Bart Van Assche bvanassche@acm.org Cc: Rafael J. Wysocki rjw@rjwysocki.net Cc: Len Brown lenb@kernel.org Cc: Fengguang Wu fengguang.wu@intel.com Cc: linux-acpi@vger.kernel.org Cc: stable@vger.kernel.org --- drivers/acpi/acpica/aclocal.h | 2 +- drivers/acpi/acpica/dscontrol.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h index 0f28a38a43ea..7b093bcbaef5 100644 --- a/drivers/acpi/acpica/aclocal.h +++ b/drivers/acpi/acpica/aclocal.h @@ -588,7 +588,7 @@ struct acpi_control_state { union acpi_parse_object *predicate_op; u8 *aml_predicate_start; /* Start of if/while predicate */ u8 *package_end; /* End of if/while block */ - u64 loop_timeout; /* While() loop timeout */ + unsigned long loop_timeout; /* While() loop timeout */ };
/* diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 0da96268deb5..9dbea4549484 100644 --- a/drivers/acpi/acpica/dscontrol.c +++ b/drivers/acpi/acpica/dscontrol.c @@ -84,8 +84,8 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, control_state->control.package_end = walk_state->parser_state.pkg_end; control_state->control.opcode = op->common.aml_opcode; - control_state->control.loop_timeout = acpi_os_get_timer() + - (u64)(acpi_gbl_max_loop_iterations * ACPI_100NSEC_PER_SEC); + control_state->control.loop_timeout = jiffies + + acpi_gbl_max_loop_iterations * HZ;
/* Push the control state on this walk's control stack */
@@ -179,9 +179,8 @@ acpi_ds_exec_end_control_op(struct acpi_walk_state *walk_state, * written AML when the hardware does not respond within a while * loop and the loop does not implement a timeout. */ - if (ACPI_TIME_AFTER(acpi_os_get_timer(), - control_state->control. - loop_timeout)) { + if (time_after(jiffies, + control_state->control.loop_timeout)) { status = AE_AML_LOOP_TIMEOUT; break; }
On Tue, Oct 16, 2018 at 12:19 AM Bart Van Assche bvanassche@acm.org wrote:
This patch avoids that the following warning is reported during hibernation:
Well, what exactly is the problem and why is the patch the right way to address it?
WARNING: CPU: 0 PID: 612 at kernel/time/timekeeping.c:751 ktime_get+0x116/0x120 RIP: 0010:ktime_get+0x116/0x120 Call Trace: acpi_os_get_timer+0xe/0x30 acpi_ds_exec_begin_control_op+0x175/0x1de acpi_ds_exec_begin_op+0x2c7/0x39a acpi_ps_create_op+0x573/0x5e4 acpi_ps_parse_loop+0x349/0x1220 acpi_ps_parse_aml+0x25b/0x6da acpi_ps_execute_method+0x327/0x41b acpi_ns_evaluate+0x4e9/0x6f5 acpi_ut_evaluate_object+0xd9/0x2f2 acpi_rs_get_method_data+0x8f/0x114 acpi_walk_resources+0x122/0x1b6 acpi_pci_link_get_current.isra.2+0x157/0x280 acpi_pci_link_set+0x32f/0x4a0 irqrouter_resume+0x58/0x80 syscore_resume+0x84/0x380 hibernation_snapshot+0x20c/0x4f0 hibernate+0x22d/0x3a6 state_store+0x99/0xa0 kobj_attr_store+0x37/0x50 sysfs_kf_write+0x87/0xa0 kernfs_fop_write+0x1a5/0x240 __vfs_write+0xd2/0x410 vfs_write+0x101/0x250 ksys_write+0xab/0x120 __x64_sys_write+0x43/0x50 do_syscall_64+0x71/0x220 entry_SYSCALL_64_after_hwframe+0x49/0xbe
Reported-by: Fengguang Wu fengguang.wu@intel.com Fixes: 164a08cee135 ("ACPICA: Dispatcher: Introduce timeout mechanism for infinite loop detection") References: https://lists.01.org/pipermail/lkp/2018-April/008406.html Signed-off-by: Bart Van Assche bvanassche@acm.org Cc: Rafael J. Wysocki rjw@rjwysocki.net Cc: Len Brown lenb@kernel.org Cc: Fengguang Wu fengguang.wu@intel.com Cc: linux-acpi@vger.kernel.org Cc: stable@vger.kernel.org
drivers/acpi/acpica/aclocal.h | 2 +- drivers/acpi/acpica/dscontrol.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h index 0f28a38a43ea..7b093bcbaef5 100644 --- a/drivers/acpi/acpica/aclocal.h +++ b/drivers/acpi/acpica/aclocal.h @@ -588,7 +588,7 @@ struct acpi_control_state { union acpi_parse_object *predicate_op; u8 *aml_predicate_start; /* Start of if/while predicate */ u8 *package_end; /* End of if/while block */
u64 loop_timeout; /* While() loop timeout */
unsigned long loop_timeout; /* While() loop timeout */};
/* diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 0da96268deb5..9dbea4549484 100644 --- a/drivers/acpi/acpica/dscontrol.c +++ b/drivers/acpi/acpica/dscontrol.c @@ -84,8 +84,8 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, control_state->control.package_end = walk_state->parser_state.pkg_end; control_state->control.opcode = op->common.aml_opcode;
control_state->control.loop_timeout = acpi_os_get_timer() +(u64)(acpi_gbl_max_loop_iterations * ACPI_100NSEC_PER_SEC);
control_state->control.loop_timeout = jiffies +acpi_gbl_max_loop_iterations * HZ;
jiffies is Linux-specific and so it should not be used in the ACPICA code.
/* Push the control state on this walk's control stack */@@ -179,9 +179,8 @@ acpi_ds_exec_end_control_op(struct acpi_walk_state *walk_state, * written AML when the hardware does not respond within a while * loop and the loop does not implement a timeout. */
if (ACPI_TIME_AFTER(acpi_os_get_timer(),control_state->control.loop_timeout)) {
if (time_after(jiffies,control_state->control.loop_timeout)) { status = AE_AML_LOOP_TIMEOUT; break; }--
On Tue, 2018-10-16 at 00:44 +0200, Rafael J. Wysocki wrote:
On Tue, Oct 16, 2018 at 12:19 AM Bart Van Assche bvanassche@acm.org wrote:
This patch avoids that the following warning is reported during hibernation:
Well, what exactly is the problem and why is the patch the right way to address it?
It is not safe to call ktime_get() after having called timekeeping_suspend().
/* diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 0da96268deb5..9dbea4549484 100644 --- a/drivers/acpi/acpica/dscontrol.c +++ b/drivers/acpi/acpica/dscontrol.c @@ -84,8 +84,8 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, control_state->control.package_end = walk_state->parser_state.pkg_end; control_state->control.opcode = op->common.aml_opcode;
control_state->control.loop_timeout = acpi_os_get_timer() +(u64)(acpi_gbl_max_loop_iterations * ACPI_100NSEC_PER_SEC);
control_state->control.loop_timeout = jiffies +acpi_gbl_max_loop_iterations * HZ;jiffies is Linux-specific and so it should not be used in the ACPICA code.
Really? There is plenty of other ACPI code that uses the jiffies counter directly. Why wouldn't it be allowed to use the jiffies counter in this context since there is so much other ACPI code that uses that counter?
$ git grep -nHw jiffies drivers/acpi drivers/acpi/acpi_pad.c:158: if (time_before(expire_time, jiffies)) { drivers/acpi/acpi_pad.c:159: last_jiffies = jiffies; drivers/acpi/acpi_pad.c:165: expire_time = jiffies + HZ * (100 - idle_pct) / 100; drivers/acpi/acpi_pad.c:184: if (time_before(expire_time, jiffies)) { drivers/acpi/apei/ghes.c:744: expire = jiffies + msecs_to_jiffies(g->notify.poll_interval); drivers/acpi/battery.c:29:#include <linux/jiffies.h> drivers/acpi/battery.c:567: time_before(jiffies, battery->update_time + drivers/acpi/battery.c:583: battery->update_time = jiffies; drivers/acpi/dock.c:30:#include <linux/jiffies.h> drivers/acpi/dock.c:365: ds->last_dock_time = jiffies; drivers/acpi/dock.c:389: time_before(jiffies, (ds->last_dock_time + HZ))) drivers/acpi/dock.c:621: dock_station->last_dock_time = jiffies - HZ; drivers/acpi/ec.c:304: ec->timestamp = jiffies; drivers/acpi/ec.c:313: ec->timestamp = jiffies; drivers/acpi/ec.c:320: ec->timestamp = jiffies; drivers/acpi/ec.c:752: } while (time_before(jiffies, timeout)); drivers/acpi/ec.c:762: unsigned long delay = jiffies + drivers/acpi/ec.c:770: } while (time_before(jiffies, delay)); drivers/acpi/ec.c:1350: ec->timestamp = jiffies; drivers/acpi/osl.c:41:#include <linux/jiffies.h> drivers/acpi/osl.c:1230: long jiffies; drivers/acpi/osl.c:1246: jiffies = MAX_SCHEDULE_TIMEOUT; drivers/acpi/osl.c:1248: jiffies = msecs_to_jiffies(timeout); drivers/acpi/osl.c:1250: ret = down_timeout(sem, jiffies); drivers/acpi/sbs.c:31:#include <linux/jiffies.h> drivers/acpi/sbs.c:377: time_before(jiffies, battery->update_time + drivers/acpi/sbs.c:391: battery->update_time = jiffies; drivers/acpi/thermal.c:36:#include <linux/jiffies.h>
Bart.
On Mon, Oct 15, 2018 at 04:01:35PM -0700, Bart Van Assche wrote:
On Tue, 2018-10-16 at 00:44 +0200, Rafael J. Wysocki wrote:
On Tue, Oct 16, 2018 at 12:19 AM Bart Van Assche bvanassche@acm.org wrote:
This patch avoids that the following warning is reported during hibernation:
Well, what exactly is the problem and why is the patch the right way to address it?
It is not safe to call ktime_get() after having called timekeeping_suspend().
/* diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 0da96268deb5..9dbea4549484 100644 --- a/drivers/acpi/acpica/dscontrol.c +++ b/drivers/acpi/acpica/dscontrol.c @@ -84,8 +84,8 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, control_state->control.package_end = walk_state->parser_state.pkg_end; control_state->control.opcode = op->common.aml_opcode;
control_state->control.loop_timeout = acpi_os_get_timer() +(u64)(acpi_gbl_max_loop_iterations * ACPI_100NSEC_PER_SEC);
control_state->control.loop_timeout = jiffies +acpi_gbl_max_loop_iterations * HZ;jiffies is Linux-specific and so it should not be used in the ACPICA code.
Really? There is plenty of other ACPI code that uses the jiffies counter directly. Why wouldn't it be allowed to use the jiffies counter in this context since there is so much other ACPI code that uses that counter?
acpica is different from acpi. It is the "core acpi code" that is shared across other operating systems. That is why jiffies does not work in it :(
thanks,
greg k-h
On Tue, 2018-10-16 at 07:04 +0200, Greg KH wrote:
On Mon, Oct 15, 2018 at 04:01:35PM -0700, Bart Van Assche wrote:
On Tue, 2018-10-16 at 00:44 +0200, Rafael J. Wysocki wrote:
On Tue, Oct 16, 2018 at 12:19 AM Bart Van Assche bvanassche@acm.org wrote:
This patch avoids that the following warning is reported during hibernation:
Well, what exactly is the problem and why is the patch the right way to address it?
It is not safe to call ktime_get() after having called timekeeping_suspend().
/* diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 0da96268deb5..9dbea4549484 100644 --- a/drivers/acpi/acpica/dscontrol.c +++ b/drivers/acpi/acpica/dscontrol.c @@ -84,8 +84,8 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state, control_state->control.package_end = walk_state->parser_state.pkg_end; control_state->control.opcode = op->common.aml_opcode;
control_state->control.loop_timeout = acpi_os_get_timer() +(u64)(acpi_gbl_max_loop_iterations * ACPI_100NSEC_PER_SEC);
control_state->control.loop_timeout = jiffies +acpi_gbl_max_loop_iterations * HZ;jiffies is Linux-specific and so it should not be used in the ACPICA code.
Really? There is plenty of other ACPI code that uses the jiffies counter directly. Why wouldn't it be allowed to use the jiffies counter in this context since there is so much other ACPI code that uses that counter?
acpica is different from acpi. It is the "core acpi code" that is shared across other operating systems. That is why jiffies does not work in it :(
Thanks Greg for the feedback. I wasn't aware of the difference between ACPICA and other ACPI code.
Bart.
linux-stable-mirror@lists.linaro.org
-
Bart Van Assche -
Greg KH -
Rafael J. Wysocki