From patchwork Mon Nov 7 16:45:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 9415501 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7084460512 for ; Mon, 7 Nov 2016 16:46:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6378C28B58 for ; Mon, 7 Nov 2016 16:46:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5802E28B5E; Mon, 7 Nov 2016 16:46:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EE41D28B58 for ; Mon, 7 Nov 2016 16:45:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752174AbcKGQpq (ORCPT ); Mon, 7 Nov 2016 11:45:46 -0500 Received: from mga11.intel.com ([192.55.52.93]:35102 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750791AbcKGQpo (ORCPT ); Mon, 7 Nov 2016 11:45:44 -0500 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP; 07 Nov 2016 08:45:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,606,1473145200"; d="scan'208";a="1056386440" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by orsmga001.jf.intel.com with SMTP; 07 Nov 2016 08:45:21 -0800 Received: by stinkbox (sSMTP sendmail emulation); Mon, 07 Nov 2016 18:45:19 +0200 Date: Mon, 7 Nov 2016 18:45:19 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Thomas Gleixner Cc: Feng Tang , feng.tang@intel.com, "Rafael J. Wysocki" , "Rafael J. Wysocki" , Steven Rostedt , Sebastian Andrzej Siewior , linux-arch@vger.kernel.org, Rik van Riel , "Srivatsa S. Bhat" , Peter Zijlstra , Arjan van de Ven , Rusty Russell , Oleg Nesterov , Tejun Heo , Andrew Morton , Paul McKenney , Linus Torvalds , Paul Turner , Linux Kernel Mailing List , "Zhang, Rui" , Len Brown , Linux PM , Linux ACPI Subject: Re: S3 resume regression [1cf4f629d9d2 ("cpu/hotplug: Move online calls to hotplugged cpu")] Message-ID: <20161107164519.GA4617@intel.com> References: <20161027192006.GF4617@intel.com> <20161027203745.GH4617@intel.com> <20161028155603.GI4617@intel.com> <20161101204737.GB4617@intel.com> <20161107114937.GX4617@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Nov 07, 2016 at 02:07:43PM +0100, Thomas Gleixner wrote: > On Mon, 7 Nov 2016, Ville Syrjälä wrote: > > I didn't manage to find a lot of time to play around with this, but it > > definitely looks like the SMM trap is the problem here. I repeated my > > pm_trace experiemnts and when it gets stuck it is trying to execute the > > _WAK ACPI method which is where the SMM trap happens. > > > > Maybe the SMM code was written with the expectation of a periodic tick > > or something like that? > > Can you try the untested hack below, please? It should confirm that. > > Thanks, > > tglx > > 8<--------------- > --- a/drivers/acpi/acpica/hwsleep.c > +++ b/drivers/acpi/acpica/hwsleep.c > @@ -269,6 +269,17 @@ acpi_status acpi_hw_legacy_wake_prep(u8 > return_ACPI_STATUS(status); > } > > +static const ktime_t time10ms = { .tv64 = 10 * NSEC_PER_MSEC }; > + > +static enum hrtimer_restart acpi_hw_legacy_tmr(struct hrtimer *tmr) > +{ > + hrtimer_forward_now(tmr, time10ms); > + > + return HRTIMER_RESTART; > +} > + > + > + > /******************************************************************************* > * > * FUNCTION: acpi_hw_legacy_wake > @@ -284,6 +295,7 @@ acpi_status acpi_hw_legacy_wake_prep(u8 > > acpi_status acpi_hw_legacy_wake(u8 sleep_state) > { > + struct hrtimer timer; > acpi_status status; > > ACPI_FUNCTION_TRACE(hw_legacy_wake); > @@ -311,12 +323,18 @@ acpi_status acpi_hw_legacy_wake(u8 sleep > return_ACPI_STATUS(status); > } > > + hrtimer_init_on_stack(&timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); > + timer.function = acpi_hw_legacy_tmr; > + hrtimer_start(&timer, time10ms, HRTIMER_MODE_REL); > + > /* > * Now we can execute _WAK, etc. Some machines require that the GPEs > * are enabled before the wake methods are executed. > */ > acpi_hw_execute_sleep_method(METHOD_PATHNAME__WAK, sleep_state); > > + hrtimer_cancel(&timer); > + > /* > * Some BIOS code assumes that WAK_STS will be cleared on resume > * and use it to determine whether the system is rebooting or Doesn't really seem to help. I did get a few random successes, but mostly it just fails. Tossing that on top shows the trace before the _WAK being the last one executed. Adding an msleep() before the _WAK does make the trace from the timer handler show up so at least the timer seems to be ticking up until some point. diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c index 3c9c10bd49e9..950319b619f1 100644 --- a/drivers/acpi/acpica/hwsleep.c +++ b/drivers/acpi/acpica/hwsleep.c @@ -44,6 +44,8 @@ #include #include +#include +#include #include "accommon.h" #define _COMPONENT ACPI_HARDWARE @@ -275,6 +277,8 @@ static enum hrtimer_restart acpi_hw_legacy_tmr(struct hrtimer *tmr) { hrtimer_forward_now(tmr, time10ms); + TRACE_RESUME(0); + return HRTIMER_RESTART; } @@ -327,14 +331,17 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state) timer.function = acpi_hw_legacy_tmr; hrtimer_start(&timer, time10ms, HRTIMER_MODE_REL); + TRACE_RESUME(0); /* * Now we can execute _WAK, etc. Some machines require that the GPEs * are enabled before the wake methods are executed. */ acpi_hw_execute_sleep_method(METHOD_PATHNAME__WAK, sleep_state); + TRACE_RESUME(0); hrtimer_cancel(&timer); + TRACE_RESUME(0); /* * Some BIOS code assumes that WAK_STS will be cleared on resume * and use it to determine whether the system is rebooting or