5:52 p.m.
While trying to make use of dynamic tables as a way to provide driver
initialization properties, I've encountered a couple of issues with the
way dynamic tables are being handled.
The first patch appears to be a bug. There is a mutex release without
a corresponding aquire. This extra release causes a mutex error message
when a table is re-loaded after being unloaded.
The second patch actually unloads a table when table unload is called.
When a table is being dynamically loaded by a driver module and that
driver module is unloaded, the memory for the table would typically be
freed resulting in a table that is marked as unloaded but still pointing
to that freed memory. If the driver module is then re-loaded, the
ACPI table reload function will attempt to access that freed memory when
it does a table compare and results in a oops.
These patches solve the problems I've encountered but may not be the
proper way to solve.
Bob Paauwe (2):
Remove extra release mutex call.
Acutally unload a table.
drivers/acpi/acpica/tbinstal.c | 1 -
drivers/acpi/acpica/tbxfload.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
--
2.1.0
5:52 p.m.
New subject: [PATCH 1/2] Remove extra release mutex call.
Remove the release mutex call when a table is re-loaded. This seems to
be here to release a mutex that was aquired at one time in table verify
function. That aquire is no longer present, so remove the release.
Signed-off-by: Bob Paauwe <bob.j.paauwe(a)intel.com>
---
drivers/acpi/acpica/tbinstal.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index 9bad45e..7fbc2b9 100644
--- a/drivers/acpi/acpica/tbinstal.c
+++ b/drivers/acpi/acpica/tbinstal.c
@@ -346,7 +346,6 @@ acpi_tb_install_standard_table(acpi_physical_address address,
*/
acpi_tb_uninstall_table(&new_table_desc);
*table_index = i;
- (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
return_ACPI_STATUS(AE_OK);
}
}
--
2.1.0
6:49 a.m.
New subject: [PATCH 1/2] Remove extra release mutex call.
This has already been reported by Octavian and the fix has been merged in the ACPICA
upstream:
https://github.com/acpica/acpica/commit/c70434d
Thanks for reporting.
Best regards
-Lv
From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Bob
Paauwe
Sent: Wednesday, April 08, 2015 12:52 AM
Remove the release mutex call when a table is re-loaded. This seems to
be here to release a mutex that was aquired at one time in table verify
function. That aquire is no longer present, so remove the release.
Signed-off-by: Bob Paauwe <bob.j.paauwe(a)intel.com>
---
drivers/acpi/acpica/tbinstal.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index 9bad45e..7fbc2b9 100644
--- a/drivers/acpi/acpica/tbinstal.c
+++ b/drivers/acpi/acpica/tbinstal.c
@@ -346,7 +346,6 @@ acpi_tb_install_standard_table(acpi_physical_address address,
*/
acpi_tb_uninstall_table(&new_table_desc);
*table_index = i;
- (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
return_ACPI_STATUS(AE_OK);
}
}
--
2.1.0
_______________________________________________
Devel mailing list
Devel(a)acpica.org
https://lists.acpica.org/mailman/listinfo/devel
5:52 p.m.
New subject: [PATCH 2/2] Acutally unload a table.
Instead of simply marking it as unloaded. If a table is dynamically
loaded by a module and the module is then unloaded, the table is no
longer valid so it's best to make sure it is unloaded.
Signed-off-by: Bob Paauwe <bob.j.paauwe(a)intel.com>
---
drivers/acpi/acpica/tbxfload.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index aadb300..ce75ca2 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -408,6 +408,7 @@ acpi_status acpi_unload_parent_table(acpi_handle object)
status = acpi_tb_release_owner_id(i);
acpi_tb_set_table_loaded_flag(i, FALSE);
+ acpi_tb_uninstall_table(&acpi_gbl_root_table_list.tables[i]);
break;
}
--
2.1.0
6:50 p.m.
New subject: [PATCH 2/2] Acutally unload a table.
Is this really such a big deal? I don't think that a loaded table is really consuming
much in the way of resources. Sure, it is mapped, but at least some of the internal stuff
is relatively static. Plus, we have to:
1) Add a new external interface which must be documented.
2) Disallow unloading things like DSDT and SSDTs, FADT, FACS.
3) What if a table is used by more than one "module"?
You'll have to convince me that this is worth the trouble.
Bob
-----Original Message-----
From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Bob Paauwe
Sent: Tuesday, April 07, 2015 9:52 AM
To: devel-acpica
Subject: [Devel] [PATCH 2/2] Acutally unload a table.
Instead of simply marking it as unloaded. If a table is dynamically
loaded by a module and the module is then unloaded, the table is no longer
valid so it's best to make sure it is unloaded.
Signed-off-by: Bob Paauwe <bob.j.paauwe(a)intel.com>
---
drivers/acpi/acpica/tbxfload.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/acpi/acpica/tbxfload.c
b/drivers/acpi/acpica/tbxfload.c index aadb300..ce75ca2 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -408,6 +408,7 @@ acpi_status acpi_unload_parent_table(acpi_handle
object)
status = acpi_tb_release_owner_id(i);
acpi_tb_set_table_loaded_flag(i, FALSE);
+ acpi_tb_uninstall_table(&acpi_gbl_root_table_list.tables[i]);
break;
}
--
2.1.0
_______________________________________________
Devel mailing list
Devel(a)acpica.org
https://lists.acpica.org/mailman/listinfo/devel
7:34 p.m.
New subject: [PATCH 2/2] Acutally unload a table.
On Tue, 7 Apr 2015 10:50:55 -0700
"Moore, Robert" <robert.moore(a)intel.com> wrote:
Is this really such a big deal? I don't think that a loaded table
is really consuming much in the way of resources. Sure, it is mapped, but at least some of
the internal stuff is relatively static. Plus, we have to:
1) Add a new external interface which must be documented.
2) Disallow unloading things like DSDT and SSDTs, FADT, FACS.
3) What if a table is used by more than one "module"?
You'll have to convince me that this is worth the trouble.
Bob
I'm doing something that's a bit unconventional so I'm not sure I can
convince you. But maybe I'm doing something really wrong and there's a
better way. So with that.
I'm using the new acpi properties as a way to provide some
configuration data to the driver. Right now, I have three ways to get
these values loaded.
1) They could be inserted into the ACPI firmware for the device. I
think this will work, but I don't have any way of doing this today.
2) I can create a table, compile it, and add it as a firmware blob
in /lib/firmware (or build the blob into the kernel).
3) I can create a table, compile it, and include it in the driver
source as static data.
I'm using #3 right now to test everything. If I do the following:
modprobe i915
acpi_load_table(static table data);
[ use properties to configure driver and test]
acpi_unload_parent_table();
rmmod i915
modprobe i915
acpi_load_table(static table data);
I'll get a kernel null dereference in acpi_load_table() because it
thinks the static table data from the first load is still there, but
since it was part of the module that was unloaded, it's gone.
This patch was the easiest way for me to stop crashing and allow me
to test stuff without having to reboot every time.
Bob
> -----Original Message-----
> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Bob Paauwe
> Sent: Tuesday, April 07, 2015 9:52 AM
> To: devel-acpica
> Subject: [Devel] [PATCH 2/2] Acutally unload a table.
>
> Instead of simply marking it as unloaded. If a table is dynamically
> loaded by a module and the module is then unloaded, the table is no longer
> valid so it's best to make sure it is unloaded.
>
> Signed-off-by: Bob Paauwe <bob.j.paauwe(a)intel.com>
> ---
> drivers/acpi/acpica/tbxfload.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/acpi/acpica/tbxfload.c
> b/drivers/acpi/acpica/tbxfload.c index aadb300..ce75ca2 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -408,6 +408,7 @@ acpi_status acpi_unload_parent_table(acpi_handle
> object)
>
> status = acpi_tb_release_owner_id(i);
> acpi_tb_set_table_loaded_flag(i, FALSE);
> + acpi_tb_uninstall_table(&acpi_gbl_root_table_list.tables[i]);
> break;
> }
>
> --
> 2.1.0
>
> _______________________________________________
> Devel mailing list
> Devel(a)acpica.org
> https://lists.acpica.org/mailman/listinfo/devel
2634
days inactive
2637
days old
5 comments
3 participants
participants (3)
-
Bob Paauwe -
Moore, Robert -
Zheng, Lv