[PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper

[PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper

[Andy Shevchenko]

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 <andriy.shevchenko@linux.intel.com>
---
 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(&ipmi->tx_msg_lock, flags);
 	list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) {
 		if (msg == tx_msg) {
-			msg_found = true;
 			list_del(&tx_msg->head);
 			break;
 		}
 	}
 	spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags);
 
-	if (msg_found)
-		acpi_ipmi_msg_put(tx_msg);
+	if (list_entry_is_head(tx_msg, &ipmi->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(&ipmi_device->tx_msg_lock, flags);
 	list_for_each_entry_safe(tx_msg, temp, &ipmi_device->tx_msg_list, head) {
 		if (msg->msgid == tx_msg->tx_msgid) {
-			msg_found = true;
 			list_del(&tx_msg->head);
 			break;
 		}
 	}
 	spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags);
 
-	if (!msg_found) {
+	if (list_entry_is_head(tx_msg, &ipmi_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(&driver_data.ipmi_lock);
 	list_for_each_entry_safe(ipmi_device, temp,
 				 &driver_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(&driver_data.ipmi_lock);
 
-	if (dev_found) {
+	if (!list_entry_is_head(ipmi_device, &driver_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(&bus_type_sem);
 	list_for_each_entry(tmp, &bus_type_list, list) {
-		if (tmp->match(dev)) {
-			ret = tmp;
+		if (tmp->match(dev))
 			break;
-		}
 	}
 	up_read(&bus_type_sem);
-	return ret;
+	return list_entry_is_head(tmp, &bus_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 && nfit_memdev->memdev->range_index)
 			continue;
-		found = NULL;
 		dcr = nfit_memdev->memdev->region_index;
 		device_handle = nfit_memdev->memdev->device_handle;
 		list_for_each_entry(nfit_mem, &acpi_desc->dimms, list)
-			if (__to_nfit_memdev(nfit_mem)->device_handle
-					== device_handle) {
-				found = nfit_mem;
+			if (__to_nfit_memdev(nfit_mem)->device_handle == device_handle)
 				break;
-			}
 
-		if (found)
-			nfit_mem = found;
-		else {
+		if (list_entry_is_head(nfit_mem, &acpi_desc->dimms, list)) {
 			nfit_mem = devm_kzalloc(acpi_desc->dev,
 					sizeof(*nfit_mem), GFP_KERNEL);
 			if (!nfit_mem)
diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c
index ee8d9973f60b..dbe70ebdfc79 100644
--- a/drivers/acpi/nfit/mce.c
+++ b/drivers/acpi/nfit/mce.c
@@ -33,7 +33,6 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val,
 	mutex_lock(&acpi_desc_lock);
 	list_for_each_entry(acpi_desc, &acpi_descs, list) {
 		struct device *dev = acpi_desc->dev;
-		int found_match = 0;
 
 		mutex_lock(&acpi_desc->init_mutex);
 		list_for_each_entry(nfit_spa, &acpi_desc->spas, list) {
@@ -46,7 +45,6 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val,
 				continue;
 			if ((spa->address + spa->length - 1) < mce->addr)
 				continue;
-			found_match = 1;
 			dev_dbg(dev, "addr in SPA %d (0x%llx, 0x%llx)\n",
 				spa->range_index, spa->address, spa->length);
 			/*
@@ -58,7 +56,7 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val,
 		}
 		mutex_unlock(&acpi_desc->init_mutex);
 
-		if (!found_match)
+		if (list_entry_is_head(nfit_spa, &acpi_desc->spas, list))
 			continue;
 
 		/* If this fails due to an -ENOMEM, there is little we can do */
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index c2d494784425..90ef0629737d 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -767,7 +767,7 @@ static int acpi_dev_consumes_res(struct acpi_device *adev, struct resource *res)
 {
 	struct list_head resource_list;
 	struct resource_entry *rentry;
-	int ret, found = 0;
+	int ret;
 
 	INIT_LIST_HEAD(&resource_list);
 	ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
@@ -775,15 +775,12 @@ static int acpi_dev_consumes_res(struct acpi_device *adev, struct resource *res)
 		return 0;
 
 	list_for_each_entry(rentry, &resource_list, node) {
-		if (resource_contains(rentry->res, res)) {
-			found = 1;
+		if (resource_contains(rentry->res, res))
 			break;
-		}
-
 	}
 
 	acpi_dev_free_resource_list(&resource_list);
-	return found;
+	return !list_entry_is_head(rentry, &resource_list, node);
 }
 
 static acpi_status acpi_res_consumer_cb(acpi_handle handle, u32 depth,
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index d5cedffeeff9..9dcebb4421a0 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -771,17 +771,14 @@ EXPORT_SYMBOL(acpi_dev_hid_uid_match);
 bool acpi_dev_found(const char *hid)
 {
 	struct acpi_device_bus_id *acpi_device_bus_id;
-	bool found = false;
 
 	mutex_lock(&acpi_device_lock);
 	list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node)
-		if (!strcmp(acpi_device_bus_id->bus_id, hid)) {
-			found = true;
+		if (!strcmp(acpi_device_bus_id->bus_id, hid))
 			break;
-		}
 	mutex_unlock(&acpi_device_lock);
 
-	return found;
+	return !list_entry_is_head(acpi_device_bus_id, &acpi_bus_id_list, node);
 }
 EXPORT_SYMBOL(acpi_dev_found);
 
-- 
2.34.1

Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper

[kernel test robot]

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-lkp@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 <lkp@intel.com>

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, &ipmi->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(&ipmi->tx_msg_lock, flags);
   360		list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) {
   361			if (msg == tx_msg) {
   362				list_del(&tx_msg->head);
   363				break;
   364			}
   365		}
   366		spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags);
   367	
   368		if (list_entry_is_head(tx_msg, &ipmi->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-all@lists.01.org

Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2941 bytes --]

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-lkp(a)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 <lkp@intel.com>

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, &ipmi->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(&ipmi->tx_msg_lock, flags);
   360		list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) {
   361			if (msg == tx_msg) {
   362				list_del(&tx_msg->head);
   363				break;
   364			}
   365		}
   366		spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags);
   367	
   368		if (list_entry_is_head(tx_msg, &ipmi->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-all(a)lists.01.org

Re: [PATCH v1 1/1] ACPI: Switch to use list_entry_is_head() helper

[Andy Shevchenko]

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

[Rafael J. Wysocki]

On Wed, Mar 2, 2022 at 4:50 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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

[Andy Shevchenko]

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
> <andriy.shevchenko@linux.intel.com> 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