2021-10-17 11:32:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 2/3] ACPI: PM: Fix sharing of wakeup power resources

From: Rafael J. Wysocki <[email protected]>

If an ACPI wakeup power resource is shared between multiple devices,
it may not be managed correctly.

Suppose, for example, that two devices, A and B, share a wakeup power
resource P whose wakeup_enabled flag is 0 initially. Next, suppose
that wakeup power is enabled for A and B, in this order, and disabled
for B. When wakeup power is enabled for A, P will be turned on and
its wakeup_enabled flag will be set. Next, when wakeup power is
enabled for B, P will not be touched, because its wakeup_enabled flag
is set. Now, when wakeup power is disabled for B, P will be turned
off which is incorrect, because A will still need P in order to signal
wakeup.

Moreover, if wakeup power is enabled for A and then disabled for B,
the latter will cause P to be turned off incorrectly (it will be still
needed by A), because acpi_disable_wakeup_device_power() is allowed
to manipulate power resources when the wakeup.prepare_count counter
of the given device is 0.

While the first issue could be addressed by changing the
wakeup_enabled power resource flag into a counter, addressing the
second one requires modifying acpi_disable_wakeup_device_power() to
do nothing when the target device's wakeup.prepare_count reference
counter is zero and that would cause the new counter to be redundant.
Namely, if acpi_disable_wakeup_device_power() is modified as per the
above, every change of the new counter following a wakeup.prepare_count
change would be reflected by the analogous change of the main reference
counter of the given power resource.

Accordingly, modify acpi_disable_wakeup_device_power() to do nothing
when the target device's wakeup.prepare_count reference counter is
zero and drop the power resource wakeup_enabled flag altogether.

While at it, ensure that all of the power resources that can be
turned off will be turned off when disabling device wakeup due to
a power resource manipulation error, to prevent energy from being
wasted.

Fixes: b5d667eb392e ("ACPI / PM: Take unusual configurations of power resources into account")
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/power.c | 73 ++++++++++++++++++---------------------------------
1 file changed, 26 insertions(+), 47 deletions(-)

Index: linux-pm/drivers/acpi/power.c
===================================================================
--- linux-pm.orig/drivers/acpi/power.c
+++ linux-pm/drivers/acpi/power.c
@@ -52,7 +52,6 @@ struct acpi_power_resource {
u32 order;
unsigned int ref_count;
u8 state;
- bool wakeup_enabled;
struct mutex resource_lock;
struct list_head dependents;
};
@@ -710,8 +709,7 @@ int acpi_device_sleep_wake(struct acpi_d
*/
int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
{
- struct acpi_power_resource_entry *entry;
- int err = 0;
+ int err;

if (!dev || !dev->wakeup.flags.valid)
return -EINVAL;
@@ -721,26 +719,13 @@ int acpi_enable_wakeup_device_power(stru
if (dev->wakeup.prepare_count++)
goto out;

- list_for_each_entry(entry, &dev->wakeup.resources, node) {
- struct acpi_power_resource *resource = entry->resource;
-
- mutex_lock(&resource->resource_lock);
-
- if (!resource->wakeup_enabled) {
- err = acpi_power_on_unlocked(resource);
- if (!err)
- resource->wakeup_enabled = true;
- }
-
- mutex_unlock(&resource->resource_lock);
-
- if (err) {
- dev_err(&dev->dev,
- "Cannot turn wakeup power resources on\n");
- dev->wakeup.flags.valid = 0;
- goto out;
- }
+ err = acpi_power_on_list(&dev->wakeup.resources);
+ if (err) {
+ dev_err(&dev->dev, "Cannot turn on wakeup power resources\n");
+ dev->wakeup.flags.valid = 0;
+ goto out;
}
+
/*
* Passing 3 as the third argument below means the device may be
* put into arbitrary power state afterward.
@@ -763,46 +748,40 @@ int acpi_enable_wakeup_device_power(stru
int acpi_disable_wakeup_device_power(struct acpi_device *dev)
{
struct acpi_power_resource_entry *entry;
- int err = 0;
+ int err;

if (!dev || !dev->wakeup.flags.valid)
return -EINVAL;

mutex_lock(&acpi_device_lock);

- if (--dev->wakeup.prepare_count > 0)
+ if (dev->wakeup.prepare_count > 1) {
+ dev->wakeup.prepare_count--;
goto out;
+ }

- /*
- * Executing the code below even if prepare_count is already zero when
- * the function is called may be useful, for example for initialisation.
- */
- if (dev->wakeup.prepare_count < 0)
- dev->wakeup.prepare_count = 0;
+ /* Do nothing if wakeup power has not been enabled for this device. */
+ if (!dev->wakeup.prepare_count)
+ goto out;

err = acpi_device_sleep_wake(dev, 0, 0, 0);
if (err)
goto out;

+ /*
+ * All of the power resources in the list need to be turned off even if
+ * there are errors.
+ */
list_for_each_entry(entry, &dev->wakeup.resources, node) {
- struct acpi_power_resource *resource = entry->resource;
-
- mutex_lock(&resource->resource_lock);
-
- if (resource->wakeup_enabled) {
- err = acpi_power_off_unlocked(resource);
- if (!err)
- resource->wakeup_enabled = false;
- }
-
- mutex_unlock(&resource->resource_lock);
+ int ret;

- if (err) {
- dev_err(&dev->dev,
- "Cannot turn wakeup power resources off\n");
- dev->wakeup.flags.valid = 0;
- break;
- }
+ ret = acpi_power_off(entry->resource);
+ if (ret && !err)
+ err = ret;
+ }
+ if (err) {
+ dev_err(&dev->dev, "Cannot turn off wakeup power resources\n");
+ dev->wakeup.flags.valid = 0;
}

out:




2021-10-18 02:17:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] ACPI: PM: Fix sharing of wakeup power resources

Hi "Rafael,

I love your patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on linux/master linus/master v5.15-rc5 next-20211015]
[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/0day-ci/linux/commits/Rafael-J-Wysocki/ACPI-PM-Address-issues-related-to-managing-wakeup-power-resources/20211016-010527
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-r035-20211015 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
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/0day-ci/linux/commit/5e93f177b80cbc9b9ee6ffc15ff9ad0ad23f2a7a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafael-J-Wysocki/ACPI-PM-Address-issues-related-to-managing-wakeup-power-resources/20211016-010527
git checkout 5e93f177b80cbc9b9ee6ffc15ff9ad0ad23f2a7a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/acpi/power.c:719:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (dev->wakeup.prepare_count++)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:739:9: note: uninitialized use occurs here
return err;
^~~
drivers/acpi/power.c:719:2: note: remove the 'if' if its condition is always false
if (dev->wakeup.prepare_count++)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:712:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
drivers/acpi/power.c:764:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!dev->wakeup.prepare_count)
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:789:9: note: uninitialized use occurs here
return err;
^~~
drivers/acpi/power.c:764:2: note: remove the 'if' if its condition is always false
if (!dev->wakeup.prepare_count)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:758:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (dev->wakeup.prepare_count > 1) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:789:9: note: uninitialized use occurs here
return err;
^~~
drivers/acpi/power.c:758:2: note: remove the 'if' if its condition is always false
if (dev->wakeup.prepare_count > 1) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:751:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
3 warnings generated.


vim +719 drivers/acpi/power.c

77e766099efc29 Rafael J. Wysocki 2008-07-07 703
^1da177e4c3f41 Linus Torvalds 2005-04-16 704 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 705 * Prepare a wakeup device, two steps (Ref ACPI 2.0:P229):
^1da177e4c3f41 Linus Torvalds 2005-04-16 706 * 1. Power on the power resources required for the wakeup device
77e766099efc29 Rafael J. Wysocki 2008-07-07 707 * 2. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
77e766099efc29 Rafael J. Wysocki 2008-07-07 708 * State Wake) for the device, if present
^1da177e4c3f41 Linus Torvalds 2005-04-16 709 */
77e766099efc29 Rafael J. Wysocki 2008-07-07 710 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
^1da177e4c3f41 Linus Torvalds 2005-04-16 711 {
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 712 int err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 713
^1da177e4c3f41 Linus Torvalds 2005-04-16 714 if (!dev || !dev->wakeup.flags.valid)
77e766099efc29 Rafael J. Wysocki 2008-07-07 715 return -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 716
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 717 mutex_lock(&acpi_device_lock);
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 718
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 @719 if (dev->wakeup.prepare_count++)
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 720 goto out;
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 721
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 722 err = acpi_power_on_list(&dev->wakeup.resources);
993cbe595dda73 Rafael J. Wysocki 2013-01-17 723 if (err) {
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 724 dev_err(&dev->dev, "Cannot turn on wakeup power resources\n");
^1da177e4c3f41 Linus Torvalds 2005-04-16 725 dev->wakeup.flags.valid = 0;
b5d667eb392ed9 Rafael J. Wysocki 2013-02-23 726 goto out;
b5d667eb392ed9 Rafael J. Wysocki 2013-02-23 727 }
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 728
77e766099efc29 Rafael J. Wysocki 2008-07-07 729 /*
993cbe595dda73 Rafael J. Wysocki 2013-01-17 730 * Passing 3 as the third argument below means the device may be
993cbe595dda73 Rafael J. Wysocki 2013-01-17 731 * put into arbitrary power state afterward.
77e766099efc29 Rafael J. Wysocki 2008-07-07 732 */
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 733 err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 734 if (err)
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 735 dev->wakeup.prepare_count = 0;
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 736
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 737 out:
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 738 mutex_unlock(&acpi_device_lock);
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 739 return err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 740 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 741

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (6.47 kB)
.config.gz (38.73 kB)
Download all attachments

2021-10-18 03:32:28

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v2 2/3] ACPI: PM: Fix sharing of wakeup power resources

From: Rafael J. Wysocki <[email protected]>

If an ACPI wakeup power resource is shared between multiple devices,
it may not be managed correctly.

Suppose, for example, that two devices, A and B, share a wakeup power
resource P whose wakeup_enabled flag is 0 initially. Next, suppose
that wakeup power is enabled for A and B, in this order, and disabled
for B. When wakeup power is enabled for A, P will be turned on and
its wakeup_enabled flag will be set. Next, when wakeup power is
enabled for B, P will not be touched, because its wakeup_enabled flag
is set. Now, when wakeup power is disabled for B, P will be turned
off which is incorrect, because A will still need P in order to signal
wakeup.

Moreover, if wakeup power is enabled for A and then disabled for B,
the latter will cause P to be turned off incorrectly (it will be still
needed by A), because acpi_disable_wakeup_device_power() is allowed
to manipulate power resources when the wakeup.prepare_count counter
of the given device is 0.

While the first issue could be addressed by changing the
wakeup_enabled power resource flag into a counter, addressing the
second one requires modifying acpi_disable_wakeup_device_power() to
do nothing when the target device's wakeup.prepare_count reference
counter is zero and that would cause the new counter to be redundant.
Namely, if acpi_disable_wakeup_device_power() is modified as per the
above, every change of the new counter following a wakeup.prepare_count
change would be reflected by the analogous change of the main reference
counter of the given power resource.

Accordingly, modify acpi_disable_wakeup_device_power() to do nothing
when the target device's wakeup.prepare_count reference counter is
zero and drop the power resource wakeup_enabled flag altogether.

While at it, ensure that all of the power resources that can be
turned off will be turned off when disabling device wakeup due to
a power resource manipulation error, to prevent energy from being
wasted.

Fixes: b5d667eb392e ("ACPI / PM: Take unusual configurations of power resources into account")
Signed-off-by: Rafael J. Wysocki <[email protected]>
---

v1 -> v2:
* Restore the initialization of err in two places removed by v1 by mistake.

---
drivers/acpi/power.c | 69 +++++++++++++++++----------------------------------
1 file changed, 24 insertions(+), 45 deletions(-)

Index: linux-pm/drivers/acpi/power.c
===================================================================
--- linux-pm.orig/drivers/acpi/power.c
+++ linux-pm/drivers/acpi/power.c
@@ -52,7 +52,6 @@ struct acpi_power_resource {
u32 order;
unsigned int ref_count;
u8 state;
- bool wakeup_enabled;
struct mutex resource_lock;
struct list_head dependents;
};
@@ -710,7 +709,6 @@ int acpi_device_sleep_wake(struct acpi_d
*/
int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
{
- struct acpi_power_resource_entry *entry;
int err = 0;

if (!dev || !dev->wakeup.flags.valid)
@@ -721,26 +719,13 @@ int acpi_enable_wakeup_device_power(stru
if (dev->wakeup.prepare_count++)
goto out;

- list_for_each_entry(entry, &dev->wakeup.resources, node) {
- struct acpi_power_resource *resource = entry->resource;
-
- mutex_lock(&resource->resource_lock);
-
- if (!resource->wakeup_enabled) {
- err = acpi_power_on_unlocked(resource);
- if (!err)
- resource->wakeup_enabled = true;
- }
-
- mutex_unlock(&resource->resource_lock);
-
- if (err) {
- dev_err(&dev->dev,
- "Cannot turn wakeup power resources on\n");
- dev->wakeup.flags.valid = 0;
- goto out;
- }
+ err = acpi_power_on_list(&dev->wakeup.resources);
+ if (err) {
+ dev_err(&dev->dev, "Cannot turn on wakeup power resources\n");
+ dev->wakeup.flags.valid = 0;
+ goto out;
}
+
/*
* Passing 3 as the third argument below means the device may be
* put into arbitrary power state afterward.
@@ -770,39 +755,33 @@ int acpi_disable_wakeup_device_power(str

mutex_lock(&acpi_device_lock);

- if (--dev->wakeup.prepare_count > 0)
+ if (dev->wakeup.prepare_count > 1) {
+ dev->wakeup.prepare_count--;
goto out;
+ }

- /*
- * Executing the code below even if prepare_count is already zero when
- * the function is called may be useful, for example for initialisation.
- */
- if (dev->wakeup.prepare_count < 0)
- dev->wakeup.prepare_count = 0;
+ /* Do nothing if wakeup power has not been enabled for this device. */
+ if (!dev->wakeup.prepare_count)
+ goto out;

err = acpi_device_sleep_wake(dev, 0, 0, 0);
if (err)
goto out;

+ /*
+ * All of the power resources in the list need to be turned off even if
+ * there are errors.
+ */
list_for_each_entry(entry, &dev->wakeup.resources, node) {
- struct acpi_power_resource *resource = entry->resource;
-
- mutex_lock(&resource->resource_lock);
-
- if (resource->wakeup_enabled) {
- err = acpi_power_off_unlocked(resource);
- if (!err)
- resource->wakeup_enabled = false;
- }
-
- mutex_unlock(&resource->resource_lock);
+ int ret;

- if (err) {
- dev_err(&dev->dev,
- "Cannot turn wakeup power resources off\n");
- dev->wakeup.flags.valid = 0;
- break;
- }
+ ret = acpi_power_off(entry->resource);
+ if (ret && !err)
+ err = ret;
+ }
+ if (err) {
+ dev_err(&dev->dev, "Cannot turn off wakeup power resources\n");
+ dev->wakeup.flags.valid = 0;
}

out:




2021-10-19 07:28:29

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] ACPI: PM: Fix sharing of wakeup power resources

Hi "Rafael,

I love your patch! Yet something to improve:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on linux/master linus/master v5.15-rc6 next-20211018]
[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/0day-ci/linux/commits/Rafael-J-Wysocki/ACPI-PM-Address-issues-related-to-managing-wakeup-power-resources/20211016-010527
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-buildonly-randconfig-r003-20211019 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d245f2e8597bfb52c34810a328d42b990e4af1a4)
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/0day-ci/linux/commit/5e93f177b80cbc9b9ee6ffc15ff9ad0ad23f2a7a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafael-J-Wysocki/ACPI-PM-Address-issues-related-to-managing-wakeup-power-resources/20211016-010527
git checkout 5e93f177b80cbc9b9ee6ffc15ff9ad0ad23f2a7a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/acpi/power.c:719:6: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (dev->wakeup.prepare_count++)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:739:9: note: uninitialized use occurs here
return err;
^~~
drivers/acpi/power.c:719:2: note: remove the 'if' if its condition is always false
if (dev->wakeup.prepare_count++)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:712:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
drivers/acpi/power.c:764:6: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!dev->wakeup.prepare_count)
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:789:9: note: uninitialized use occurs here
return err;
^~~
drivers/acpi/power.c:764:2: note: remove the 'if' if its condition is always false
if (!dev->wakeup.prepare_count)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:758:6: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (dev->wakeup.prepare_count > 1) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:789:9: note: uninitialized use occurs here
return err;
^~~
drivers/acpi/power.c:758:2: note: remove the 'if' if its condition is always false
if (dev->wakeup.prepare_count > 1) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:751:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
3 errors generated.


vim +719 drivers/acpi/power.c

77e766099efc29 Rafael J. Wysocki 2008-07-07 703
^1da177e4c3f41 Linus Torvalds 2005-04-16 704 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 705 * Prepare a wakeup device, two steps (Ref ACPI 2.0:P229):
^1da177e4c3f41 Linus Torvalds 2005-04-16 706 * 1. Power on the power resources required for the wakeup device
77e766099efc29 Rafael J. Wysocki 2008-07-07 707 * 2. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
77e766099efc29 Rafael J. Wysocki 2008-07-07 708 * State Wake) for the device, if present
^1da177e4c3f41 Linus Torvalds 2005-04-16 709 */
77e766099efc29 Rafael J. Wysocki 2008-07-07 710 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
^1da177e4c3f41 Linus Torvalds 2005-04-16 711 {
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 712 int err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 713
^1da177e4c3f41 Linus Torvalds 2005-04-16 714 if (!dev || !dev->wakeup.flags.valid)
77e766099efc29 Rafael J. Wysocki 2008-07-07 715 return -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 716
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 717 mutex_lock(&acpi_device_lock);
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 718
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 @719 if (dev->wakeup.prepare_count++)
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 720 goto out;
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 721
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 722 err = acpi_power_on_list(&dev->wakeup.resources);
993cbe595dda73 Rafael J. Wysocki 2013-01-17 723 if (err) {
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 724 dev_err(&dev->dev, "Cannot turn on wakeup power resources\n");
^1da177e4c3f41 Linus Torvalds 2005-04-16 725 dev->wakeup.flags.valid = 0;
b5d667eb392ed9 Rafael J. Wysocki 2013-02-23 726 goto out;
b5d667eb392ed9 Rafael J. Wysocki 2013-02-23 727 }
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 728
77e766099efc29 Rafael J. Wysocki 2008-07-07 729 /*
993cbe595dda73 Rafael J. Wysocki 2013-01-17 730 * Passing 3 as the third argument below means the device may be
993cbe595dda73 Rafael J. Wysocki 2013-01-17 731 * put into arbitrary power state afterward.
77e766099efc29 Rafael J. Wysocki 2008-07-07 732 */
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 733 err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 734 if (err)
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 735 dev->wakeup.prepare_count = 0;
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 736
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 737 out:
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 738 mutex_unlock(&acpi_device_lock);
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 739 return err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 740 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 741

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (6.49 kB)
.config.gz (39.12 kB)
Download all attachments

2021-10-19 07:32:23

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] ACPI: PM: Fix sharing of wakeup power resources

Hi Rafael,

Thanks for your patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on linux/master linus/master v5.15-rc5 next-20211015]
[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/0day-ci/linux/commits/Rafael-J-Wysocki/ACPI-PM-Address-issues-related-to-managing-wakeup-power-resources/20211016-010527
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-c007-20211015 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
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/0day-ci/linux/commit/5e93f177b80cbc9b9ee6ffc15ff9ad0ad23f2a7a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafael-J-Wysocki/ACPI-PM-Address-issues-related-to-managing-wakeup-power-resources/20211016-010527
git checkout 5e93f177b80cbc9b9ee6ffc15ff9ad0ad23f2a7a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


clang-analyzer warnings:

>> drivers/acpi/power.c:739:2: warning: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn]
return err;
^ ~~~
drivers/acpi/power.c:712:2: note: 'err' declared without an initial value
int err;
^~~~~~~
drivers/acpi/power.c:714:6: note: Assuming 'dev' is non-null
if (!dev || !dev->wakeup.flags.valid)
^~~~
drivers/acpi/power.c:714:6: note: Left side of '||' is false
drivers/acpi/power.c:714:14: note: Assuming field 'valid' is not equal to 0
if (!dev || !dev->wakeup.flags.valid)
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:714:2: note: Taking false branch
if (!dev || !dev->wakeup.flags.valid)
^
drivers/acpi/power.c:719:6: note: Assuming the condition is true
if (dev->wakeup.prepare_count++)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/power.c:719:2: note: Taking true branch
if (dev->wakeup.prepare_count++)
^
drivers/acpi/power.c:720:3: note: Control jumps to line 738
goto out;
^
drivers/acpi/power.c:739:2: note: Undefined or garbage value returned to caller
return err;
^ ~~~


vim +739 drivers/acpi/power.c

77e766099efc29 Rafael J. Wysocki 2008-07-07 703
^1da177e4c3f41 Linus Torvalds 2005-04-16 704 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 705 * Prepare a wakeup device, two steps (Ref ACPI 2.0:P229):
^1da177e4c3f41 Linus Torvalds 2005-04-16 706 * 1. Power on the power resources required for the wakeup device
77e766099efc29 Rafael J. Wysocki 2008-07-07 707 * 2. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
77e766099efc29 Rafael J. Wysocki 2008-07-07 708 * State Wake) for the device, if present
^1da177e4c3f41 Linus Torvalds 2005-04-16 709 */
77e766099efc29 Rafael J. Wysocki 2008-07-07 710 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
^1da177e4c3f41 Linus Torvalds 2005-04-16 711 {
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 712 int err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 713
^1da177e4c3f41 Linus Torvalds 2005-04-16 714 if (!dev || !dev->wakeup.flags.valid)
77e766099efc29 Rafael J. Wysocki 2008-07-07 715 return -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 716
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 717 mutex_lock(&acpi_device_lock);
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 718
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 719 if (dev->wakeup.prepare_count++)
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 720 goto out;
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 721
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 722 err = acpi_power_on_list(&dev->wakeup.resources);
993cbe595dda73 Rafael J. Wysocki 2013-01-17 723 if (err) {
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 724 dev_err(&dev->dev, "Cannot turn on wakeup power resources\n");
^1da177e4c3f41 Linus Torvalds 2005-04-16 725 dev->wakeup.flags.valid = 0;
b5d667eb392ed9 Rafael J. Wysocki 2013-02-23 726 goto out;
b5d667eb392ed9 Rafael J. Wysocki 2013-02-23 727 }
5e93f177b80cbc Rafael J. Wysocki 2021-10-15 728
77e766099efc29 Rafael J. Wysocki 2008-07-07 729 /*
993cbe595dda73 Rafael J. Wysocki 2013-01-17 730 * Passing 3 as the third argument below means the device may be
993cbe595dda73 Rafael J. Wysocki 2013-01-17 731 * put into arbitrary power state afterward.
77e766099efc29 Rafael J. Wysocki 2008-07-07 732 */
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 733 err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 734 if (err)
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 735 dev->wakeup.prepare_count = 0;
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 736
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 737 out:
9b83ccd2f14f64 Rafael J. Wysocki 2009-09-08 738 mutex_unlock(&acpi_device_lock);
0af4b8c4fb3119 Rafael J. Wysocki 2008-07-07 @739 return err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 740 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 741

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
.config.gz (33.41 kB)