| Message ID | 4200238.ejJDZkT8p0@kreacher (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | ACPICA: ACPICA 20220331 | expand |
Hi "Rafael,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on linus/master linux/master v5.18-rc2 next-20220411]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Rafael-J-Wysocki/ACPICA-ACPICA-20220331/20220412-030922
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-a006-20220411 (https://download.01.org/0day-ci/archive/20220412/202204121322.P9yX0gKP-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/22c298d3a077e7ea8503e9acf7ac83e6b1e10148
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rafael-J-Wysocki/ACPICA-ACPICA-20220331/20220412-030922
git checkout 22c298d3a077e7ea8503e9acf7ac83e6b1e10148
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Note: the linux-review/Rafael-J-Wysocki/ACPICA-ACPICA-20220331/20220412-030922 HEAD 32181ae3d3173aeee41f709612dfa4d52951b39d builds fine.
It only hurts bisectability.
All errors (new ones prefixed by >>):
drivers/acpi/acpica/exsystem.c:140:7: error: use of undeclared identifier 'how_long_US'; did you mean 'how_long_us'?
if (how_long_US > 100) {
^~~~~~~~~~~
how_long_us
drivers/acpi/acpica/exsystem.c:123:41: note: 'how_long_us' declared here
acpi_status acpi_ex_system_do_stall(u32 how_long_us)
^
>> drivers/acpi/acpica/exsystem.c:179:10: error: use of undeclared identifier 'how_long_us'; did you mean 'how_long_ms'?
how_long_us));
^~~~~~~~~~~
how_long_ms
include/acpi/acoutput.h:203:54: note: expanded from macro 'ACPI_WARNING'
#define ACPI_WARNING(plist) acpi_warning plist
^
drivers/acpi/acpica/exsystem.c:164:41: note: 'how_long_ms' declared here
acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
^
2 errors generated.
vim +179 drivers/acpi/acpica/exsystem.c
150
151 /*******************************************************************************
152 *
153 * FUNCTION: acpi_ex_system_do_sleep
154 *
155 * PARAMETERS: how_long_ms - The amount of time to sleep,
156 * in milliseconds
157 *
158 * RETURN: None
159 *
160 * DESCRIPTION: Sleep the running thread for specified amount of time.
161 *
162 ******************************************************************************/
163
164 acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
165 {
166 ACPI_FUNCTION_ENTRY();
167
168 /* Since this thread will sleep, we must release the interpreter */
169
170 acpi_ex_exit_interpreter();
171
172 /*
173 * Warn users about excessive sleep times, so ASL code can be improved to
174 * use polling or similar techniques.
175 */
176 if (how_long_ms > 10) {
177 ACPI_WARNING((AE_INFO,
178 "Firmware issue: Excessive sleep time (%llu ms > 10 ms) in ACPI Control Method",
> 179 how_long_us));
180 }
181
182 /*
183 * For compatibility with other ACPI implementations and to prevent
184 * accidental deep sleeps, limit the sleep time to something reasonable.
185 */
186 if (how_long_ms > ACPI_MAX_SLEEP) {
187 how_long_ms = ACPI_MAX_SLEEP;
188 }
189
190 acpi_os_sleep(how_long_ms);
191
192 /* And now we must get the interpreter again */
193
194 acpi_ex_enter_interpreter();
195 return (AE_OK);
196 }
197
On Mon, Apr 11, 2022 at 9:04 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Paul Menzel <pmenzel@molgen.mpg.de> > > ACPICA commit 2a0d1d475e7ea1c815bee1e0692d81db9a7c909c > > Quick boottime is important, so warn about sleeps greater than 10 ms. > > Distribution Linux kernels reach initrd in 350 ms, so excessive delays > should be called out. 10 ms is chosen randomly, but three of such delays > would already make up ten percent of the boottime. > > Link: https://github.com/acpica/acpica/commit/2a0d1d47 > Signed-off-by: Bob Moore <robert.moore@intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> I have decided to revert this, because it spams logs with unuseful warnings on many production systems. Power management AML uses sleeps above 10 ms for PCI spec compliance and polling is not useful in those cases. I will submit an analogous revert for upstream ACPICA. > --- > exsystem.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff -Nurp linux.before_name/drivers/acpi/acpica/exsystem.c linux.after_name/drivers/acpi/acpica/exsystem.c > --- linux.before_name/drivers/acpi/acpica/exsystem.c 2022-04-01 18:26:54.958046947 +0200 > +++ linux.after_name/drivers/acpi/acpica/exsystem.c 2022-04-01 18:26:51.760086923 +0200 > @@ -170,6 +170,16 @@ acpi_status acpi_ex_system_do_sleep(u64 > acpi_ex_exit_interpreter(); > > /* > + * Warn users about excessive sleep times, so ASL code can be improved to > + * use polling or similar techniques. > + */ > + if (how_long_ms > 10) { > + ACPI_WARNING((AE_INFO, > + "Firmware issue: Excessive sleep time (%llu ms > 10 ms) in ACPI Control Method", > + how_long_us)); > + } > + > + /* > * For compatibility with other ACPI implementations and to prevent > * accidental deep sleeps, limit the sleep time to something reasonable. > */ > > >
Dear Rafael, Am 21.05.22 um 18:11 schrieb Rafael J. Wysocki: > On Mon, Apr 11, 2022 at 9:04 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> >> From: Paul Menzel <pmenzel@molgen.mpg.de> >> >> ACPICA commit 2a0d1d475e7ea1c815bee1e0692d81db9a7c909c >> >> Quick boottime is important, so warn about sleeps greater than 10 ms. >> >> Distribution Linux kernels reach initrd in 350 ms, so excessive delays >> should be called out. 10 ms is chosen randomly, but three of such delays >> would already make up ten percent of the boottime. >> >> Link: https://github.com/acpica/acpica/commit/2a0d1d47 >> Signed-off-by: Bob Moore <robert.moore@intel.com> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > I have decided to revert this, because it spams logs with unuseful > warnings on many production systems. Thank you for the information. Can you please give an example? > Power management AML uses sleeps above 10 ms for PCI spec compliance > and polling is not useful in those cases. Can you please tell me what delays are used? Maybe we can increase the threshold to the one required by the PCI spec? Kind regards, Paul > I will submit an analogous revert for upstream ACPICA. > >> --- >> exsystem.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff -Nurp linux.before_name/drivers/acpi/acpica/exsystem.c linux.after_name/drivers/acpi/acpica/exsystem.c >> --- linux.before_name/drivers/acpi/acpica/exsystem.c 2022-04-01 18:26:54.958046947 +0200 >> +++ linux.after_name/drivers/acpi/acpica/exsystem.c 2022-04-01 18:26:51.760086923 +0200 >> @@ -170,6 +170,16 @@ acpi_status acpi_ex_system_do_sleep(u64 >> acpi_ex_exit_interpreter(); >> >> /* >> + * Warn users about excessive sleep times, so ASL code can be improved to >> + * use polling or similar techniques. >> + */ >> + if (how_long_ms > 10) { >> + ACPI_WARNING((AE_INFO, >> + "Firmware issue: Excessive sleep time (%llu ms > 10 ms) in ACPI Control Method", >> + how_long_us)); >> + } >> + >> + /* >> * For compatibility with other ACPI implementations and to prevent >> * accidental deep sleeps, limit the sleep time to something reasonable. >> */ >> >> >>
Hi Paul, Sorry for the delay. On Sun, May 22, 2022 at 1:28 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Dear Rafael, > > > Am 21.05.22 um 18:11 schrieb Rafael J. Wysocki: > > On Mon, Apr 11, 2022 at 9:04 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > >> > >> From: Paul Menzel <pmenzel@molgen.mpg.de> > >> > >> ACPICA commit 2a0d1d475e7ea1c815bee1e0692d81db9a7c909c > >> > >> Quick boottime is important, so warn about sleeps greater than 10 ms. > >> > >> Distribution Linux kernels reach initrd in 350 ms, so excessive delays > >> should be called out. 10 ms is chosen randomly, but three of such delays > >> would already make up ten percent of the boottime. > >> > >> Link: https://github.com/acpica/acpica/commit/2a0d1d47 > >> Signed-off-by: Bob Moore <robert.moore@intel.com> > >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > I have decided to revert this, because it spams logs with unuseful > > warnings on many production systems. > > Thank you for the information. Can you please give an example? Personally, I saw this on Dell XPS13 9360 and 9380 machines in my office, but it has been reported to be that it was visible on multiple systems in the Linux client power lab at Intel. You can also see here that Linux is not the only affected OS: https://github.com/acpica/acpica/pull/780 Thanks!
diff -Nurp linux.before_name/drivers/acpi/acpica/exsystem.c linux.after_name/drivers/acpi/acpica/exsystem.c --- linux.before_name/drivers/acpi/acpica/exsystem.c 2022-04-01 18:26:54.958046947 +0200 +++ linux.after_name/drivers/acpi/acpica/exsystem.c 2022-04-01 18:26:51.760086923 +0200 @@ -170,6 +170,16 @@ acpi_status acpi_ex_system_do_sleep(u64 acpi_ex_exit_interpreter(); /* + * Warn users about excessive sleep times, so ASL code can be improved to + * use polling or similar techniques. + */ + if (how_long_ms > 10) { + ACPI_WARNING((AE_INFO, + "Firmware issue: Excessive sleep time (%llu ms > 10 ms) in ACPI Control Method", + how_long_us)); + } + + /* * For compatibility with other ACPI implementations and to prevent * accidental deep sleeps, limit the sleep time to something reasonable. */