[PATCH 0/9] ACPI: Call acpi_put_table() to release the ACPI table mappings

[PATCH 0/9] ACPI: Call acpi_put_table() to release the ACPI table mappings

[Hanjun Guo]

The acpi_get_table() should be coupled with acpi_put_table() if the
mapped table is not used for runtime after the initialization, or the
error path or the initialization, to release the table mapping.

This patchset just scan for every acpi_get_table() in ACPI subsystem to
see if acpi_put_table() is needed, this is not urgent and I think can
be merged for 5.8 if this patchset makes sense.

Hanjun Guo (9):
  ACPI: LPIT: Put the low power idle table after using it
  ACPI: watchdog: Put the watchdog action table after parsing
  ACPI: APEI: Put the boot error record table after parsing
  ACPI: APEI: Put the error injection table for error path and module
    exit
  ACPI: APEI: Put the error record serialization table for error path
  ACPI: APEI: Put the HEST table for error path
  ACPI: EC: Put the ACPI table after using it
  ACPI: scan: Put SPCR and STAO table after using it
  ACPI: sleep: Put the FACS table after using it

 drivers/acpi/acpi_lpit.c     |  3 ++-
 drivers/acpi/acpi_watchdog.c |  7 +++++--
 drivers/acpi/apei/bert.c     |  6 ++++--
 drivers/acpi/apei/einj.c     |  5 ++++-
 drivers/acpi/apei/erst.c     |  4 +++-
 drivers/acpi/apei/hest.c     |  5 +++--
 drivers/acpi/ec.c            | 14 ++++++++++----
 drivers/acpi/scan.c          | 15 ++++++++++-----
 drivers/acpi/sleep.c         |  4 +++-
 9 files changed, 44 insertions(+), 19 deletions(-)

-- 
1.7.12.4

[PATCH 1/9] ACPI: LPIT: Put the low power idle table after using it

[Hanjun Guo]

The mapped LPIT table is not used for runtime after init,
put the ACPI table to release the table mapping.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/acpi_lpit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_lpit.c b/drivers/acpi/acpi_lpit.c
index 953437a..48e5059 100644
--- a/drivers/acpi/acpi_lpit.c
+++ b/drivers/acpi/acpi_lpit.c
@@ -151,10 +151,11 @@ void acpi_init_lpit(void)
 	struct acpi_table_lpit *lpit;
 
 	status = acpi_get_table(ACPI_SIG_LPIT, 0, (struct acpi_table_header **)&lpit);
-
 	if (ACPI_FAILURE(status))
 		return;
 
 	lpit_process((u64)lpit + sizeof(*lpit),
 		     (u64)lpit + lpit->header.length);
+
+	acpi_put_table((struct acpi_table_header *)lpit);
 }
-- 
1.7.12.4

[PATCH 2/9] ACPI: watchdog: Put the watchdog action table after parsing

[Hanjun Guo]

The mapped watchdog action table should be released after the
successfully parsing, and the failure path in the driver
initialization.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/acpi_watchdog.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 6e9ec6e..5c1e9ea 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -73,6 +73,7 @@ static const struct acpi_table_wdat *acpi_watchdog_get_wdat(void)
 	}
 
 	if (acpi_watchdog_uses_rtc(wdat)) {
+		acpi_put_table((struct acpi_table_header *)wdat);
 		pr_info("Skipping WDAT on this system because it uses RTC SRAM\n");
 		return NULL;
 	}
@@ -117,12 +118,12 @@ void __init acpi_watchdog_init(void)
 
 	/* Watchdog disabled by BIOS */
 	if (!(wdat->flags & ACPI_WDAT_ENABLED))
-		return;
+		goto fail_put_wdat;
 
 	/* Skip legacy PCI WDT devices */
 	if (wdat->pci_segment != 0xff || wdat->pci_bus != 0xff ||
 	    wdat->pci_device != 0xff || wdat->pci_function != 0xff)
-		return;
+		goto fail_put_wdat;
 
 	INIT_LIST_HEAD(&resource_list);
 
@@ -188,4 +189,6 @@ void __init acpi_watchdog_init(void)
 
 fail_free_resource_list:
 	resource_list_free(&resource_list);
+fail_put_wdat:
+	acpi_put_table((struct acpi_table_header *)wdat);
 }
-- 
1.7.12.4

[PATCH 3/9] ACPI: APEI: Put the boot error record table after parsing

[Hanjun Guo]

The mapped boot error record table is not used after
bert_init(), release it.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/apei/bert.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/bert.c b/drivers/acpi/apei/bert.c
index 1155fb9..19e50fc 100644
--- a/drivers/acpi/apei/bert.c
+++ b/drivers/acpi/apei/bert.c
@@ -119,7 +119,7 @@ static int __init bert_init(void)
 	rc = bert_check_table(bert_tab);
 	if (rc) {
 		pr_err(FW_BUG "table invalid.\n");
-		return rc;
+		goto out_put_bert_tab;
 	}
 
 	region_len = bert_tab->region_length;
@@ -127,7 +127,7 @@ static int __init bert_init(void)
 	rc = apei_resources_add(&bert_resources, bert_tab->address,
 				region_len, true);
 	if (rc)
-		return rc;
+		goto out_put_bert_tab;
 	rc = apei_resources_request(&bert_resources, "APEI BERT");
 	if (rc)
 		goto out_fini;
@@ -142,6 +142,8 @@ static int __init bert_init(void)
 	apei_resources_release(&bert_resources);
 out_fini:
 	apei_resources_fini(&bert_resources);
+out_put_bert_tab:
+	acpi_put_table((struct acpi_table_header *)bert_tab);
 
 	return rc;
 }
-- 
1.7.12.4

[PATCH 4/9] ACPI: APEI: Put the error injection table for error path and module exit

[Hanjun Guo]

The mapped error injection table will be used after einj_init()
for debugfs, but it should be released for module exit and error
path of einj_init().

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/apei/einj.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 086373f..13315675 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -692,7 +692,7 @@ static int __init einj_init(void)
 	rc = einj_check_table(einj_tab);
 	if (rc) {
 		pr_warn(FW_BUG "Invalid EINJ table.\n");
-		return -EINVAL;
+		goto err_put_table;
 	}
 
 	rc = -ENOMEM;
@@ -760,6 +760,8 @@ static int __init einj_init(void)
 err_fini:
 	apei_resources_fini(&einj_resources);
 	debugfs_remove_recursive(einj_debug_dir);
+err_put_table:
+	acpi_put_table((struct acpi_table_header *)einj_tab);
 
 	return rc;
 }
@@ -780,6 +782,7 @@ static void __exit einj_exit(void)
 	apei_resources_release(&einj_resources);
 	apei_resources_fini(&einj_resources);
 	debugfs_remove_recursive(einj_debug_dir);
+	acpi_put_table((struct acpi_table_header *)einj_tab);
 }
 
 module_init(einj_init);
-- 
1.7.12.4

[PATCH 5/9] ACPI: APEI: Put the error record serialization table for error path

[Hanjun Guo]

The mapped error record serialization table needs to be
released for error path of erst_init().

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/apei/erst.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index 2015a09..2e0b0fc 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -1122,7 +1122,7 @@ static int __init erst_init(void)
 	rc = erst_check_table(erst_tab);
 	if (rc) {
 		pr_err(FW_BUG "ERST table is invalid.\n");
-		goto err;
+		goto err_put_erst_tab;
 	}
 
 	apei_resources_init(&erst_resources);
@@ -1196,6 +1196,8 @@ static int __init erst_init(void)
 	apei_resources_release(&erst_resources);
 err_fini:
 	apei_resources_fini(&erst_resources);
+err_put_erst_tab:
+	acpi_put_table((struct acpi_table_header *)erst_tab);
 err:
 	erst_disable = 1;
 	return rc;
-- 
1.7.12.4

[PATCH 6/9] ACPI: APEI: Put the HEST table for error path

[Hanjun Guo]

hest_tab will be used after hest_init(), but we need to release
it for error path.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/apei/hest.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 8224024..953a2fa 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -243,8 +243,8 @@ void __init acpi_hest_init(void)
 	} else if (ACPI_FAILURE(status)) {
 		const char *msg = acpi_format_exception(status);
 		pr_err(HEST_PFX "Failed to get table, %s\n", msg);
-		rc = -EINVAL;
-		goto err;
+		hest_disable = HEST_DISABLED;
+		return;
 	}
 
 	rc = apei_hest_parse(hest_parse_cmc, NULL);
@@ -266,4 +266,5 @@ void __init acpi_hest_init(void)
 	return;
 err:
 	hest_disable = HEST_DISABLED;
+	acpi_put_table((struct acpi_table_header *)hest_tab);
 }
-- 
1.7.12.4

[PATCH 7/9] ACPI: EC: Put the ACPI table after using it

[Hanjun Guo]

The embedded controller boot resources table needs to be
released after using it.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/ec.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b4c0152e..281a9bf 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1784,12 +1784,15 @@ static void __init acpi_ec_ecdt_start(void)
 
 	status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
 	if (ACPI_FAILURE(status))
-		return;
+		goto out;
 
 	boot_ec->handle = handle;
 
 	/* Add a special ACPI device object to represent the boot EC. */
 	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
+
+out:
+	acpi_put_table((struct acpi_table_header *)ecdt_ptr);
 }
 
 /*
@@ -1891,12 +1894,12 @@ void __init acpi_ec_ecdt_probe(void)
 		 * Asus X50GL:
 		 * https://bugzilla.kernel.org/show_bug.cgi?id=11880
 		 */
-		return;
+		goto out;
 	}
 
 	ec = acpi_ec_alloc();
 	if (!ec)
-		return;
+		goto out;
 
 	if (EC_FLAGS_CORRECT_ECDT) {
 		ec->command_addr = ecdt_ptr->data.address;
@@ -1922,13 +1925,16 @@ void __init acpi_ec_ecdt_probe(void)
 	ret = acpi_ec_setup(ec, NULL);
 	if (ret) {
 		acpi_ec_free(ec);
-		return;
+		goto out;
 	}
 
 	boot_ec = ec;
 	boot_ec_is_ecdt = true;
 
 	pr_info("Boot ECDT EC used to handle transactions\n");
+
+out:
+	acpi_put_table((struct acpi_table_header *)ecdt_ptr);
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
1.7.12.4

[PATCH 8/9] ACPI: scan: Put SPCR and STAO table after using it

[Hanjun Guo]

The SPCR and STAO table needs to be released after
using it.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/scan.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6d34488..5287ab9 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2157,10 +2157,13 @@ static void __init acpi_get_spcr_uart_addr(void)
 
 	status = acpi_get_table(ACPI_SIG_SPCR, 0,
 				(struct acpi_table_header **)&spcr_ptr);
-	if (ACPI_SUCCESS(status))
-		spcr_uart_addr = spcr_ptr->serial_port.address;
-	else
-		printk(KERN_WARNING PREFIX "STAO table present, but SPCR is missing\n");
+	if (ACPI_FAILURE(status)) {
+		pr_warn(PREFIX "STAO table present, but SPCR is missing\n");
+		return;
+	}
+
+	spcr_uart_addr = spcr_ptr->serial_port.address;
+	acpi_put_table((struct acpi_table_header *)spcr_ptr);
 }
 
 static bool acpi_scan_initialized;
@@ -2196,10 +2199,12 @@ int __init acpi_scan_init(void)
 				(struct acpi_table_header **)&stao_ptr);
 	if (ACPI_SUCCESS(status)) {
 		if (stao_ptr->header.length > sizeof(struct acpi_table_stao))
-			printk(KERN_INFO PREFIX "STAO Name List not yet supported.");
+			pr_info(PREFIX "STAO Name List not yet supported.\n");
 
 		if (stao_ptr->ignore_uart)
 			acpi_get_spcr_uart_addr();
+
+		acpi_put_table((struct acpi_table_header *)stao_ptr);
 	}
 
 	acpi_gpe_apply_masked_gpes();
-- 
1.7.12.4

[PATCH 9/9] ACPI: sleep: Put the FACS table after using it

[Hanjun Guo]

Put the FACS table after using it to release the table
mapping.

Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
 drivers/acpi/sleep.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 4edc8a3..30e2df9 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -1297,8 +1297,10 @@ static void acpi_sleep_hibernate_setup(void)
 		return;
 
 	acpi_get_table(ACPI_SIG_FACS, 1, (struct acpi_table_header **)&facs);
-	if (facs)
+	if (facs) {
 		s4_hardware_signature = facs->hardware_signature;
+		acpi_put_table((struct acpi_table_header *)facs);
+	}
 }
 #else /* !CONFIG_HIBERNATION */
 static inline void acpi_sleep_hibernate_setup(void) {}
-- 
1.7.12.4

Re: [PATCH 7/9] ACPI: EC: Put the ACPI table after using it

[Rafael J. Wysocki]

On Thursday, May 7, 2020 11:09:19 AM CEST Hanjun Guo wrote:
> The embedded controller boot resources table needs to be
> released after using it.
> 
> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
> ---
>  drivers/acpi/ec.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index b4c0152e..281a9bf 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -1784,12 +1784,15 @@ static void __init acpi_ec_ecdt_start(void)
>  
>  	status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
>  	if (ACPI_FAILURE(status))
> -		return;
> +		goto out;
>  

So to avoid adding a new label I would prefer

if (ACPI_SUCCESS(status)) {
	boot_ec->handle = handle;

	/* Add a special ACPI device object to represent the boot EC. */
	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
}

>  	boot_ec->handle = handle;
>  
>  	/* Add a special ACPI device object to represent the boot EC. */
>  	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
> +
> +out:
> +	acpi_put_table((struct acpi_table_header *)ecdt_ptr);
>  }
>  

Re: [PATCH 7/9] ACPI: EC: Put the ACPI table after using it

[Rafael J. Wysocki]

On Saturday, May 9, 2020 11:11:26 AM CEST Rafael J. Wysocki wrote:
> On Thursday, May 7, 2020 11:09:19 AM CEST Hanjun Guo wrote:
> > The embedded controller boot resources table needs to be
> > released after using it.
> > 
> > Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
> > ---
> >  drivers/acpi/ec.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> > index b4c0152e..281a9bf 100644
> > --- a/drivers/acpi/ec.c
> > +++ b/drivers/acpi/ec.c
> > @@ -1784,12 +1784,15 @@ static void __init acpi_ec_ecdt_start(void)
> >  
> >  	status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
> >  	if (ACPI_FAILURE(status))
> > -		return;
> > +		goto out;
> >  
> 
> So to avoid adding a new label I would prefer
> 
> if (ACPI_SUCCESS(status)) {
> 	boot_ec->handle = handle;
> 
> 	/* Add a special ACPI device object to represent the boot EC. */
> 	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
> }
> 
> >  	boot_ec->handle = handle;
> >  
> >  	/* Add a special ACPI device object to represent the boot EC. */
> >  	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
> > +
> > +out:
> > +	acpi_put_table((struct acpi_table_header *)ecdt_ptr);
> >  }
> >  

I've made the above change manually and applied the whole series as 5.8
material.

Thanks!

Re: [PATCH 7/9] ACPI: EC: Put the ACPI table after using it

[Hanjun Guo]

On 2020/5/9 17:30, Rafael J. Wysocki wrote:
> On Saturday, May 9, 2020 11:11:26 AM CEST Rafael J. Wysocki wrote:
>> On Thursday, May 7, 2020 11:09:19 AM CEST Hanjun Guo wrote:
>>> The embedded controller boot resources table needs to be
>>> released after using it.
>>>
>>> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
>>> ---
>>>   drivers/acpi/ec.c | 14 ++++++++++----
>>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
>>> index b4c0152e..281a9bf 100644
>>> --- a/drivers/acpi/ec.c
>>> +++ b/drivers/acpi/ec.c
>>> @@ -1784,12 +1784,15 @@ static void __init acpi_ec_ecdt_start(void)
>>>   
>>>   	status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
>>>   	if (ACPI_FAILURE(status))
>>> -		return;
>>> +		goto out;
>>>   
>>
>> So to avoid adding a new label I would prefer
>>
>> if (ACPI_SUCCESS(status)) {
>> 	boot_ec->handle = handle;
>>
>> 	/* Add a special ACPI device object to represent the boot EC. */
>> 	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
>> }
>>
>>>   	boot_ec->handle = handle;
>>>   
>>>   	/* Add a special ACPI device object to represent the boot EC. */
>>>   	acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
>>> +
>>> +out:
>>> +	acpi_put_table((struct acpi_table_header *)ecdt_ptr);
>>>   }
>>>   
> 
> I've made the above change manually and applied the whole series as 5.8
> material.

Thank you very much!