Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper
On Wed, Mar 02, 2022 at 05:36:20PM +0100, Rafael J. Wysocki wrote: > On Wed, Mar 2, 2022 at 4:50 PM Andy Shevchenko > wrote: > > On Fri, Feb 11, 2022 at 01:04:23PM +0200, Andy Shevchenko wrote: > > > Since we got list_entry_is_head() helper in the generic header, > > > we may switch the ACPI modules to use it. This eliminates the > > > need in additional variable. In some cases it reduces critical > > > sections as well. > > > > Besides the work required in a couple of cases (LKP) there is an > > ongoing discussion about list loops (and this particular API). > > > > Rafael, what do you think is the best course of action here? > > I think the current approach is to do the opposite of what this patch > is attempting to do: avoid using the list iterator outside of the > loop. OK, let's drop this change. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper
On Wed, Mar 2, 2022 at 4:50 PM Andy Shevchenko wrote: > > On Fri, Feb 11, 2022 at 01:04:23PM +0200, Andy Shevchenko wrote: > > Since we got list_entry_is_head() helper in the generic header, > > we may switch the ACPI modules to use it. This eliminates the > > need in additional variable. In some cases it reduces critical > > sections as well. > > Besides the work required in a couple of cases (LKP) there is an > ongoing discussion about list loops (and this particular API). > > Rafael, what do you think is the best course of action here? I think the current approach is to do the opposite of what this patch is attempting to do: avoid using the list iterator outside of the loop.
Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper
On Fri, Feb 11, 2022 at 01:04:23PM +0200, Andy Shevchenko wrote: > Since we got list_entry_is_head() helper in the generic header, > we may switch the ACPI modules to use it. This eliminates the > need in additional variable. In some cases it reduces critical > sections as well. Besides the work required in a couple of cases (LKP) there is an ongoing discussion about list loops (and this particular API). Rafael, what do you think is the best course of action here? -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper
Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on nvdimm/libnvdimm-for-next v5.17-rc3 next-20220211]
[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/Andy-Shevchenko/ACPI-Switch-to-use-list_entry_is_head-helper/20220211-190438
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
linux-next
config: ia64-allmodconfig
(https://download.01.org/0day-ci/archive/20220212/202202120054.idhietld-...@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
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/95f7c8c71bb18e505f5399a87cbb192f481c86fe
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Andy-Shevchenko/ACPI-Switch-to-use-list_entry_is_head-helper/20220211-190438
git checkout 95f7c8c71bb18e505f5399a87cbb192f481c86fe
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/acpi/acpi_ipmi.c: In function 'ipmi_cancel_tx_msg':
>> drivers/acpi/acpi_ipmi.c:369:17: error: expected ')' before 'return'
369 | return;
| ^~
drivers/acpi/acpi_ipmi.c:368:12: note: to match this '('
368 | if (list_entry_is_head(tx_msg, >tx_msg_list, head)
|^
>> drivers/acpi/acpi_ipmi.c:372:1: error: expected expression before '}' token
372 | }
| ^
vim +369 drivers/acpi/acpi_ipmi.c
352
353 static void ipmi_cancel_tx_msg(struct acpi_ipmi_device *ipmi,
354 struct acpi_ipmi_msg *msg)
355 {
356 struct acpi_ipmi_msg *tx_msg, *temp;
357 unsigned long flags;
358
359 spin_lock_irqsave(>tx_msg_lock, flags);
360 list_for_each_entry_safe(tx_msg, temp, >tx_msg_list,
head) {
361 if (msg == tx_msg) {
362 list_del(_msg->head);
363 break;
364 }
365 }
366 spin_unlock_irqrestore(>tx_msg_lock, flags);
367
368 if (list_entry_is_head(tx_msg, >tx_msg_list, head)
> 369 return;
370
371 acpi_ipmi_msg_put(tx_msg);
> 372 }
373
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper
Since we got list_entry_is_head() helper in the generic header,
we may switch the ACPI modules to use it. This eliminates the
need in additional variable. In some cases it reduces critical
sections as well.
Signed-off-by: Andy Shevchenko
---
drivers/acpi/acpi_ipmi.c | 16 ++--
drivers/acpi/glue.c | 8 +++-
drivers/acpi/nfit/core.c | 12 +++-
drivers/acpi/nfit/mce.c | 4 +---
drivers/acpi/resource.c | 9 +++--
drivers/acpi/utils.c | 7 ++-
6 files changed, 18 insertions(+), 38 deletions(-)
diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index a5fe2926bf50..f9e56138f8d1 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -354,27 +354,26 @@ static void ipmi_cancel_tx_msg(struct acpi_ipmi_device
*ipmi,
struct acpi_ipmi_msg *msg)
{
struct acpi_ipmi_msg *tx_msg, *temp;
- bool msg_found = false;
unsigned long flags;
spin_lock_irqsave(>tx_msg_lock, flags);
list_for_each_entry_safe(tx_msg, temp, >tx_msg_list, head) {
if (msg == tx_msg) {
- msg_found = true;
list_del(_msg->head);
break;
}
}
spin_unlock_irqrestore(>tx_msg_lock, flags);
- if (msg_found)
- acpi_ipmi_msg_put(tx_msg);
+ if (list_entry_is_head(tx_msg, >tx_msg_list, head)
+ return;
+
+ acpi_ipmi_msg_put(tx_msg);
}
static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
{
struct acpi_ipmi_device *ipmi_device = user_msg_data;
- bool msg_found = false;
struct acpi_ipmi_msg *tx_msg, *temp;
struct device *dev = ipmi_device->dev;
unsigned long flags;
@@ -389,14 +388,13 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
spin_lock_irqsave(_device->tx_msg_lock, flags);
list_for_each_entry_safe(tx_msg, temp, _device->tx_msg_list, head)
{
if (msg->msgid == tx_msg->tx_msgid) {
- msg_found = true;
list_del(_msg->head);
break;
}
}
spin_unlock_irqrestore(_device->tx_msg_lock, flags);
- if (!msg_found) {
+ if (list_entry_is_head(tx_msg, _device->tx_msg_list, head)) {
dev_warn(dev,
"Unexpected response (msg id %ld) is returned.\n",
msg->msgid);
@@ -483,13 +481,11 @@ static void ipmi_register_bmc(int iface, struct device
*dev)
static void ipmi_bmc_gone(int iface)
{
struct acpi_ipmi_device *ipmi_device, *temp;
- bool dev_found = false;
mutex_lock(_data.ipmi_lock);
list_for_each_entry_safe(ipmi_device, temp,
_data.ipmi_devices, head) {
if (ipmi_device->ipmi_ifnum != iface) {
- dev_found = true;
__ipmi_dev_kill(ipmi_device);
break;
}
@@ -500,7 +496,7 @@ static void ipmi_bmc_gone(int iface)
struct acpi_ipmi_device, head);
mutex_unlock(_data.ipmi_lock);
- if (dev_found) {
+ if (!list_entry_is_head(ipmi_device, _data.ipmi_devices, head)) {
ipmi_flush_tx_msg(ipmi_device);
acpi_ipmi_dev_put(ipmi_device);
}
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index ef104809f27b..ffc0b3ee190b 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -61,17 +61,15 @@ EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);
static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
{
- struct acpi_bus_type *tmp, *ret = NULL;
+ struct acpi_bus_type *tmp;
down_read(_type_sem);
list_for_each_entry(tmp, _type_list, list) {
- if (tmp->match(dev)) {
- ret = tmp;
+ if (tmp->match(dev))
break;
- }
}
up_read(_type_sem);
- return ret;
+ return list_entry_is_head(tmp, _type_list, list) ? NULL : tmp;
}
#define FIND_CHILD_MIN_SCORE 1
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index e5d7f2bda13f..b31c16e5e42c 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1076,8 +1076,8 @@ static void nfit_mem_init_bdw(struct acpi_nfit_desc
*acpi_desc,
static int __nfit_mem_init(struct acpi_nfit_desc *acpi_desc,
struct acpi_nfit_system_address *spa)
{
- struct nfit_mem *nfit_mem, *found;
struct nfit_memdev *nfit_memdev;
+ struct nfit_mem *nfit_mem;
int type = spa ? nfit_spa_type(spa) : 0;
switch (type) {
@@ -1106,19 +1106,13 @@ static int __nfit_mem_init(struct acpi_nfit_desc
*acpi_desc,
continue;
if (!spa &&
