In the Linux kernel side, acpi_get_table() clones haven't been fully
balanced by acpi_put_table() invocations. In ACPICA side, due to the design
change, there are also unbalanced acpi_get_table_by_index() invocations
requiring special care to be cleaned up.
So it is not a good timing to report acpi_get_table() counting errors for
this period. The strict balanced validation count check should only be
enabled after confirming that all invocations are safe and compliant to
their designed purposes.
Thus this patch removes the fatal error along with lthe error report to
fix this issue. Reported by Dan Williams, fixed by Lv Zheng.
Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel")
Reported-by: Dan Williams <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/acpica/tbutils.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index 5a968a7..8175c70 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -418,11 +418,13 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
table_desc->validation_count++;
if (table_desc->validation_count == 0) {
+ table_desc->validation_count--;
+#if 0
ACPI_ERROR((AE_INFO,
"Table %p, Validation count is zero after increment\n",
table_desc));
- table_desc->validation_count--;
return_ACPI_STATUS(AE_LIMIT);
+#endif
}
*out_table = table_desc->pointer;
--
2.7.4
This patch adds balanced acpi_get_table()/acpi_put_table() support for
sysfs table dumping code so that no need to call
acpi_get_validated_table().
Since ACPICA does not use all of the tables, this can help to reduce some
usless memory mappings by utilizing the new table handling APIs.
The original sysfs dumpable ACPI table implementation forces tables to be
mapped after a read operation and never unmaps them again whatever there
are no users in the kernel interested in these tables. With new balanced
table handling APIs, tables are unmapped after the read operation.
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/sysfs.c | 51 ++++++++++++++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 15 deletions(-)
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 1b5ee1e..c3bb6ce 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -333,21 +333,34 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
container_of(bin_attr, struct acpi_table_attr, attr);
struct acpi_table_header *table_header = NULL;
acpi_status status;
+ ssize_t len;
status = acpi_get_table(table_attr->name, table_attr->instance,
&table_header);
if (ACPI_FAILURE(status))
return -ENODEV;
+ len = memory_read_from_buffer(buf, count, &offset,
+ table_header, table_header->length);
+ acpi_put_table(table_header);
+ return len;
+}
+
+static bool acpi_table_has_multiple_instances(char *signature)
+{
+ acpi_status status;
+ struct acpi_table_header *header;
- return memory_read_from_buffer(buf, count, &offset,
- table_header, table_header->length);
+ status = acpi_get_table(signature, 2, &header);
+ if (ACPI_FAILURE(status))
+ return false;
+ acpi_put_table(header);
+ return true;
}
static int acpi_table_attr_init(struct kobject *tables_obj,
struct acpi_table_attr *table_attr,
struct acpi_table_header *table_header)
{
- struct acpi_table_header *header = NULL;
struct acpi_table_attr *attr = NULL;
char instance_str[ACPI_INST_SIZE];
@@ -368,9 +381,9 @@ static int acpi_table_attr_init(struct kobject *tables_obj,
ACPI_MOVE_NAME(table_attr->filename, table_header->signature);
table_attr->filename[ACPI_NAME_SIZE] = '\0';
- if (table_attr->instance > 1 || (table_attr->instance == 1 &&
- !acpi_get_table
- (table_header->signature, 2, &header))) {
+ if (table_attr->instance > 1 ||
+ (table_attr->instance == 1 &&
+ acpi_table_has_multiple_instances(table_header->signature))) {
snprintf(instance_str, sizeof(instance_str), "%u",
table_attr->instance);
strcat(table_attr->filename, instance_str);
@@ -419,11 +432,11 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
static int acpi_tables_sysfs_init(void)
{
- struct acpi_table_attr *table_attr;
+ struct acpi_table_attr *table_attr = NULL;
struct acpi_table_header *table_header = NULL;
int table_index;
acpi_status status;
- int ret;
+ int ret = 0;
tables_kobj = kobject_create_and_add("tables", acpi_kobj);
if (!tables_kobj)
@@ -435,24 +448,32 @@ static int acpi_tables_sysfs_init(void)
for (table_index = 0;; table_index++) {
status = acpi_get_table_by_index(table_index, &table_header);
-
if (status == AE_BAD_PARAMETER)
break;
-
if (ACPI_FAILURE(status))
- continue;
+ goto next_table;
table_attr = kzalloc(sizeof(*table_attr), GFP_KERNEL);
- if (!table_attr)
- return -ENOMEM;
+ if (!table_attr) {
+ ret = -ENOMEM;
+ goto next_table;
+ }
ret = acpi_table_attr_init(tables_kobj,
table_attr, table_header);
+ if (ret)
+ goto next_table;
+ list_add_tail(&table_attr->node, &acpi_table_attr_list);
+
+next_table:
+ acpi_put_table(table_header);
if (ret) {
- kfree(table_attr);
+ if (table_attr) {
+ kfree(table_attr);
+ table_attr = NULL;
+ }
return ret;
}
- list_add_tail(&table_attr->node, &acpi_table_attr_list);
}
kobject_uevent(tables_kobj, KOBJ_ADD);
--
2.7.4
On Thursday, April 27, 2017 04:22:44 PM Lv Zheng wrote:
> In the Linux kernel side, acpi_get_table() clones haven't been fully
> balanced by acpi_put_table() invocations. In ACPICA side, due to the design
> change, there are also unbalanced acpi_get_table_by_index() invocations
> requiring special care to be cleaned up.
>
> So it is not a good timing to report acpi_get_table() counting errors for
> this period. The strict balanced validation count check should only be
> enabled after confirming that all invocations are safe and compliant to
> their designed purposes.
>
> Thus this patch removes the fatal error along with lthe error report to
> fix this issue. Reported by Dan Williams, fixed by Lv Zheng.
>
> Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel")
> Reported-by: Dan Williams <[email protected]>
> Signed-off-by: Lv Zheng <[email protected]>
Please do not add #if 0 anywhere to the kernel code base.
If you need to drop some piece of code, just drop it.
And in this particular case validation_count should be dropped from the data
type definition too.
> ---
> drivers/acpi/acpica/tbutils.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
> index 5a968a7..8175c70 100644
> --- a/drivers/acpi/acpica/tbutils.c
> +++ b/drivers/acpi/acpica/tbutils.c
> @@ -418,11 +418,13 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
>
> table_desc->validation_count++;
> if (table_desc->validation_count == 0) {
> + table_desc->validation_count--;
> +#if 0
> ACPI_ERROR((AE_INFO,
> "Table %p, Validation count is zero after increment\n",
> table_desc));
> - table_desc->validation_count--;
> return_ACPI_STATUS(AE_LIMIT);
> +#endif
> }
>
> *out_table = table_desc->pointer;
>
Thanks,
Rafael
On Thursday, April 27, 2017 04:22:50 PM Lv Zheng wrote:
> This patch adds balanced acpi_get_table()/acpi_put_table() support for
> sysfs table dumping code so that no need to call
> acpi_get_validated_table().
>
> Since ACPICA does not use all of the tables, this can help to reduce some
> usless memory mappings by utilizing the new table handling APIs.
>
> The original sysfs dumpable ACPI table implementation forces tables to be
> mapped after a read operation and never unmaps them again whatever there
> are no users in the kernel interested in these tables. With new balanced
> table handling APIs, tables are unmapped after the read operation.
>
> Signed-off-by: Lv Zheng <[email protected]>
> ---
> drivers/acpi/sysfs.c | 51 ++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index 1b5ee1e..c3bb6ce 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -333,21 +333,34 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
> container_of(bin_attr, struct acpi_table_attr, attr);
> struct acpi_table_header *table_header = NULL;
> acpi_status status;
> + ssize_t len;
>
> status = acpi_get_table(table_attr->name, table_attr->instance,
> &table_header);
> if (ACPI_FAILURE(status))
> return -ENODEV;
> + len = memory_read_from_buffer(buf, count, &offset,
> + table_header, table_header->length);
> + acpi_put_table(table_header);
> + return len;
> +}
The above seems to be taken verbatim from the Dan's patch.
If that's the case, please do the below as a separate patch on top of the Dan's
one.
> +
> +static bool acpi_table_has_multiple_instances(char *signature)
> +{
> + acpi_status status;
> + struct acpi_table_header *header;
>
> - return memory_read_from_buffer(buf, count, &offset,
> - table_header, table_header->length);
> + status = acpi_get_table(signature, 2, &header);
> + if (ACPI_FAILURE(status))
> + return false;
> + acpi_put_table(header);
> + return true;
> }
>
> static int acpi_table_attr_init(struct kobject *tables_obj,
> struct acpi_table_attr *table_attr,
> struct acpi_table_header *table_header)
> {
> - struct acpi_table_header *header = NULL;
> struct acpi_table_attr *attr = NULL;
> char instance_str[ACPI_INST_SIZE];
>
> @@ -368,9 +381,9 @@ static int acpi_table_attr_init(struct kobject *tables_obj,
>
> ACPI_MOVE_NAME(table_attr->filename, table_header->signature);
> table_attr->filename[ACPI_NAME_SIZE] = '\0';
> - if (table_attr->instance > 1 || (table_attr->instance == 1 &&
> - !acpi_get_table
> - (table_header->signature, 2, &header))) {
> + if (table_attr->instance > 1 ||
> + (table_attr->instance == 1 &&
> + acpi_table_has_multiple_instances(table_header->signature))) {
> snprintf(instance_str, sizeof(instance_str), "%u",
> table_attr->instance);
> strcat(table_attr->filename, instance_str);
> @@ -419,11 +432,11 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
>
> static int acpi_tables_sysfs_init(void)
> {
> - struct acpi_table_attr *table_attr;
> + struct acpi_table_attr *table_attr = NULL;
> struct acpi_table_header *table_header = NULL;
> int table_index;
> acpi_status status;
> - int ret;
> + int ret = 0;
>
> tables_kobj = kobject_create_and_add("tables", acpi_kobj);
> if (!tables_kobj)
> @@ -435,24 +448,32 @@ static int acpi_tables_sysfs_init(void)
>
> for (table_index = 0;; table_index++) {
> status = acpi_get_table_by_index(table_index, &table_header);
> -
> if (status == AE_BAD_PARAMETER)
> break;
> -
> if (ACPI_FAILURE(status))
> - continue;
> + goto next_table;
>
> table_attr = kzalloc(sizeof(*table_attr), GFP_KERNEL);
> - if (!table_attr)
> - return -ENOMEM;
> + if (!table_attr) {
> + ret = -ENOMEM;
> + goto next_table;
> + }
>
> ret = acpi_table_attr_init(tables_kobj,
> table_attr, table_header);
> + if (ret)
> + goto next_table;
> + list_add_tail(&table_attr->node, &acpi_table_attr_list);
> +
> +next_table:
> + acpi_put_table(table_header);
> if (ret) {
> - kfree(table_attr);
> + if (table_attr) {
> + kfree(table_attr);
> + table_attr = NULL;
> + }
> return ret;
> }
> - list_add_tail(&table_attr->node, &acpi_table_attr_list);
> }
>
> kobject_uevent(tables_kobj, KOBJ_ADD);
>
Thanks,
Rafael
Hi, Rafael
> From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J.
> Wysocki
> Subject: Re: [PATCH v2 1/2] ACPICA: Tables: Fix regression introduced by a too early mechanism
> enabling
>
> On Thursday, April 27, 2017 04:22:44 PM Lv Zheng wrote:
> > In the Linux kernel side, acpi_get_table() clones haven't been fully
> > balanced by acpi_put_table() invocations. In ACPICA side, due to the design
> > change, there are also unbalanced acpi_get_table_by_index() invocations
> > requiring special care to be cleaned up.
> >
> > So it is not a good timing to report acpi_get_table() counting errors for
> > this period. The strict balanced validation count check should only be
> > enabled after confirming that all invocations are safe and compliant to
> > their designed purposes.
> >
> > Thus this patch removes the fatal error along with lthe error report to
> > fix this issue. Reported by Dan Williams, fixed by Lv Zheng.
> >
> > Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel")
> > Reported-by: Dan Williams <[email protected]>
> > Signed-off-by: Lv Zheng <[email protected]>
>
> Please do not add #if 0 anywhere to the kernel code base.
>
> If you need to drop some piece of code, just drop it.
>
> And in this particular case validation_count should be dropped from the data
> type definition too.
Then I would prefer to fix all acpi_get_table() clones in ACPICA.
That could avoid removing the useful code.
Without fixing sysfs/Load/table_handler ones, there will always cases frequently increasing/decreasing the count.
Others seem to be not urgent.
I'll post the fix as an urgent fix here with the ACPICA upstream process bypassed and back port it to ACPICA later.
If there are changes during the back port, I'll take care of making the differences eliminated in the corresponding ACPICA release cycle.
Thanks
Lv
>
> > ---
> > drivers/acpi/acpica/tbutils.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
> > index 5a968a7..8175c70 100644
> > --- a/drivers/acpi/acpica/tbutils.c
> > +++ b/drivers/acpi/acpica/tbutils.c
> > @@ -418,11 +418,13 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
> >
> > table_desc->validation_count++;
> > if (table_desc->validation_count == 0) {
> > + table_desc->validation_count--;
> > +#if 0
> > ACPI_ERROR((AE_INFO,
> > "Table %p, Validation count is zero after increment\n",
> > table_desc));
> > - table_desc->validation_count--;
> > return_ACPI_STATUS(AE_LIMIT);
> > +#endif
> > }
> >
> > *out_table = table_desc->pointer;
> >
>
> Thanks,
> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Rafael
I reconsidered your comments.
Seems there are several problems you might not be aware of.
Let me reply again.
> From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J.
> Wysocki
> Subject: Re: [PATCH v2 1/2] ACPICA: Tables: Fix regression introduced by a too early mechanism
> enabling
>
> On Thursday, April 27, 2017 04:22:44 PM Lv Zheng wrote:
> > In the Linux kernel side, acpi_get_table() clones haven't been fully
> > balanced by acpi_put_table() invocations. In ACPICA side, due to the design
> > change, there are also unbalanced acpi_get_table_by_index() invocations
> > requiring special care to be cleaned up.
> >
> > So it is not a good timing to report acpi_get_table() counting errors for
> > this period. The strict balanced validation count check should only be
> > enabled after confirming that all invocations are safe and compliant to
> > their designed purposes.
> >
> > Thus this patch removes the fatal error along with lthe error report to
> > fix this issue. Reported by Dan Williams, fixed by Lv Zheng.
> >
> > Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel")
> > Reported-by: Dan Williams <[email protected]>
> > Signed-off-by: Lv Zheng <[email protected]>
>
> Please do not add #if 0 anywhere to the kernel code base.
>
> If you need to drop some piece of code, just drop it.
OK. This comment is addressable.
>
> And in this particular case validation_count should be dropped from the data
> type definition too.
I don't think this comment is addressable.
===========================================================================
The validation_count is not only used for the late stage, it's also used
for the early boot stage. And during the early boot stage, I'm sure the
validation count is used by us now to correctly unmap early maps. If it is
dropped, the early maps will be leaked to the late stage, leading to some
kernel crashes.
The use case has already been validated to be working in previous Linux
release cycles, also with you, Dan and the other driver maintainers' help.
All crashing early cases are identified and the validation counts are
ensured to be balanced during the early stage.
===========================================================================
Then I thought again if you did have some special concerns for using this
mechanism in the late stage without making the validation count balanced.
The only case we need to worry about is when acpi_get_table() is invoked
more than 65535 times, then more than 65535 acpi_put_table() could
potentially release the table mapping while there are still users using
the mapped table, this could cause kernel crashes.
What does this problem mean to us? It mean: For all frequently invoked
acpi_get_table(), without fixing them altogether, we SHOULD NOT add
acpi_put_table() for them.
As far as I can see, the followings are frequent acpi_get_table() invoking
cases, they need to be fixed altogether:
1. sysfs table access
2. load/unload tables, could potentially be invoked in a
per-cpu-hotplugging frequent style.
3. get/put in some loadable kernel modules (this actually is not such
frequent)
4. tables used across suspend/resume (I checked, FACS used in this case is
safe)
I began to doubt the correctness of fixing only the sysfs case without
fixing all the other "frequent acpi_get_table() cases". It seems fixing it
only adds unwanted acpi_put_table() during the special period, and this
actually breaks the planned way, and can potentially break the end users.
===========================================================================
Then I checked how we could make Load/Unload invocations balanced inside of
ACPICA. It looks we have the following things that must be done to achieve
the balanced validation count inside of ACPICA:
1. There are many "table handler" invocations, we need a single place to
invoke get/put before/after invoking the handler.
2. There is an acpi_tb_set_table_loaded_flag() API, we need to invoke
get/put inside of it when the flag is set/unset.
3. For all other acpi_get_table() clone invocations, we need to add
balanced put(s) to the get(s) except those pinned as global maps.
Then we'll soon figure out that we need to do the following prerequisites:
1. Collect table handlers into 1 function;
2. Should stop invoking acpi_ns_load_table() from other places, but only
invoke it from acpi_tb_load_table();
3. After changing due to 1 and 2, we need to make sure the table mutex is
still correctly used;
4. After changing due to 2 and 3, we actually extends some sanity checks
to the processes originally not covered, and we need to make sure the
changed sanity checks is still working.
So you'll see the entire work must be based on what Hans De Geode is
asking for: "https://github.com/acpica/acpica/pull/121". This pull request
is still pending for further Windows behavior validation for how signature
is checked by Windows. And on top of that, several changes are still
required for achieving the balanced validation count in ACPICA.
So I really don't think upstreaming all the required changes is a short
period.
===========================================================================
My conclusion is:
We should go back to our planned way, and the only safe regression fix for
the reported issue is to remove the error logs and the failure returning
code from acpi_tb_get_table(). While the error in acpi_tb_put_table()
should be kept, it alerts us that there are too many unexpected
acpi_tb_put_table() invocations.
If we don't stick to the planned way, we possibly should add code in
acpi_tb_put_table() that when the get side is about to expre, stop do any
put side decrement to prevent unwanted unmaps.
Thanks and best regards
Lv
>
> > ---
> > drivers/acpi/acpica/tbutils.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
> > index 5a968a7..8175c70 100644
> > --- a/drivers/acpi/acpica/tbutils.c
> > +++ b/drivers/acpi/acpica/tbutils.c
> > @@ -418,11 +418,13 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
> >
> > table_desc->validation_count++;
> > if (table_desc->validation_count == 0) {
> > + table_desc->validation_count--;
> > +#if 0
> > ACPI_ERROR((AE_INFO,
> > "Table %p, Validation count is zero after increment\n",
> > table_desc));
> > - table_desc->validation_count--;
> > return_ACPI_STATUS(AE_LIMIT);
> > +#endif
> > }
> >
> > *out_table = table_desc->pointer;
> >
>
> Thanks,
> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
In the Linux kernel side, acpi_get_table() clones haven't been fully
balanced by acpi_put_table() invocations. In ACPICA side, due to the design
change, there are also unbalanced acpi_get_table_by_index() invocations
requiring special care.
So it is not a good timing to report acpi_get_table() counting errors. The
strict balanced validation count check should only be enabled after
confirming that all invocations are safe and compliant to their designed
purposes.
Thus this patch removes the fatal error along with the error report to
fix this issue. Reported by Dan Williams, fixed by Lv Zheng.
Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel")
Cc: Dan Williams <[email protected]>
Reported-by: Anush Seetharaman <[email protected]>
Reported-by: Dan Williams <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/acpica/tbutils.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index 5a968a7..7abe665 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -418,11 +418,7 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
table_desc->validation_count++;
if (table_desc->validation_count == 0) {
- ACPI_ERROR((AE_INFO,
- "Table %p, Validation count is zero after increment\n",
- table_desc));
table_desc->validation_count--;
- return_ACPI_STATUS(AE_LIMIT);
}
*out_table = table_desc->pointer;
--
2.7.4
In the Linux kernel side, acpi_get_table() clones haven't been fully
balanced by acpi_put_table() invocations. In ACPICA side, due to the design
change, there are also unbalanced acpi_get_table_by_index() invocations
requiring special care.
So it is not a good timing to report acpi_get_table() counting errors. The
strict balanced validation count check should only be enabled after
confirming that all invocations are safe and compliant to their designed
purposes.
Thus this patch removes the fatal error along with the error report to
fix this issue. Reported by Dan Williams, fixed by Lv Zheng.
Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel")
Cc: Anush Seetharaman <[email protected]>
Cc: Dan Williams <[email protected]>
Reported-by: Anush Seetharaman <[email protected]>
Reported-by: Dan Williams <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/acpica/tbutils.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index 5a968a7..7abe665 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -418,11 +418,7 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
table_desc->validation_count++;
if (table_desc->validation_count == 0) {
- ACPI_ERROR((AE_INFO,
- "Table %p, Validation count is zero after increment\n",
- table_desc));
table_desc->validation_count--;
- return_ACPI_STATUS(AE_LIMIT);
}
*out_table = table_desc->pointer;
--
2.7.4
For all frequent late stage acpi_get_table() clone invocations, we should
only fix them altogether, otherwise, excessive acpi_put_table() could
unexpectedly unmap the table used by the other users. Thus the current plan
is to fix all acpi_get_table() clones together or to fix none of them. This
prevents kernel developers from improving the late stage code quality
without waiting for the ACPICA upstream to improve first.
This patch adds a mechanism to stop decrementing validation count to
prevent the table unmapping operations so that acpi_put_table() balance
fixes can be done independently to each others.
Cc: Dan Williams <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/acpica/tbutils.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index 7abe665..b517bd0 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -445,12 +445,18 @@ void acpi_tb_put_table(struct acpi_table_desc *table_desc)
ACPI_FUNCTION_TRACE(acpi_tb_put_table);
- if (table_desc->validation_count == 0) {
+ if ((table_desc->validation_count + 1) == 0) {
ACPI_WARNING((AE_INFO,
- "Table %p, Validation count is zero before decrement\n",
+ "Table %p, Validation count is about to expire, decrement is unsafe\n",
table_desc));
return_VOID;
}
+ if (table_desc->validation_count == 0) {
+ ACPI_ERROR((AE_INFO,
+ "Table %p, Validation count is zero before decrement\n",
+ table_desc));
+ return_VOID;
+ }
table_desc->validation_count--;
if (table_desc->validation_count == 0) {
--
2.7.4
From: Dan Williams <[email protected]>
Reading an ACPI table through the /sys/firmware/acpi/tables interface
more than 65,536 times leads to the following log message:
ACPI Error: Table ffff88033595eaa8, Validation count is zero after
increment
(20170119/tbutils-423)
Add the missing acpi_put_table() so the table ->validation_count is
decremented after each read.
Reported-by: Anush Seetharaman <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/sysfs.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 1b5ee1e..2bbf722 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -333,14 +333,17 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
container_of(bin_attr, struct acpi_table_attr, attr);
struct acpi_table_header *table_header = NULL;
acpi_status status;
+ ssize_t len;
status = acpi_get_table(table_attr->name, table_attr->instance,
&table_header);
if (ACPI_FAILURE(status))
return -ENODEV;
- return memory_read_from_buffer(buf, count, &offset,
- table_header, table_header->length);
+ len = memory_read_from_buffer(buf, count, &offset,
+ table_header, table_header->length);
+ acpi_put_table(table_header);
+ return len;
}
static int acpi_table_attr_init(struct kobject *tables_obj,
--
2.7.4
This patch adds acpi_put_table() to make all acpi_get_table() clone
invocations balanced for sysfs ACPI table dump code.
Since Linux does not use all of the tables, this can help to reduce some
usless memory mappings.
While originally, all tables will be remained to be mapped after a
userspace acpidump execution, potentially causing problem on server
platforms. With the new APIs, it is possible to release such useless table
mappings.
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/sysfs.c | 41 +++++++++++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 2bbf722..14425dc 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -346,11 +346,22 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
return len;
}
+static bool acpi_table_has_multiple_instances(char *signature)
+{
+ acpi_status status;
+ struct acpi_table_header *header;
+
+ status = acpi_get_table(signature, 2, &header);
+ if (ACPI_FAILURE(status))
+ return false;
+ acpi_put_table(header);
+ return true;
+}
+
static int acpi_table_attr_init(struct kobject *tables_obj,
struct acpi_table_attr *table_attr,
struct acpi_table_header *table_header)
{
- struct acpi_table_header *header = NULL;
struct acpi_table_attr *attr = NULL;
char instance_str[ACPI_INST_SIZE];
@@ -371,9 +382,9 @@ static int acpi_table_attr_init(struct kobject *tables_obj,
ACPI_MOVE_NAME(table_attr->filename, table_header->signature);
table_attr->filename[ACPI_NAME_SIZE] = '\0';
- if (table_attr->instance > 1 || (table_attr->instance == 1 &&
- !acpi_get_table
- (table_header->signature, 2, &header))) {
+ if (table_attr->instance > 1 ||
+ (table_attr->instance == 1 &&
+ acpi_table_has_multiple_instances(table_header->signature))) {
snprintf(instance_str, sizeof(instance_str), "%u",
table_attr->instance);
strcat(table_attr->filename, instance_str);
@@ -422,11 +433,11 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
static int acpi_tables_sysfs_init(void)
{
- struct acpi_table_attr *table_attr;
+ struct acpi_table_attr *table_attr = NULL;
struct acpi_table_header *table_header = NULL;
int table_index;
acpi_status status;
- int ret;
+ int ret = 0;
tables_kobj = kobject_create_and_add("tables", acpi_kobj);
if (!tables_kobj)
@@ -446,16 +457,26 @@ static int acpi_tables_sysfs_init(void)
continue;
table_attr = kzalloc(sizeof(*table_attr), GFP_KERNEL);
- if (!table_attr)
- return -ENOMEM;
+ if (!table_attr) {
+ ret = -ENOMEM;
+ goto next_table;
+ }
ret = acpi_table_attr_init(tables_kobj,
table_attr, table_header);
+ if (ret)
+ goto next_table;
+ list_add_tail(&table_attr->node, &acpi_table_attr_list);
+
+next_table:
+ acpi_put_table(table_header);
if (ret) {
- kfree(table_attr);
+ if (table_attr) {
+ kfree(table_attr);
+ table_attr = NULL;
+ }
return ret;
}
- list_add_tail(&table_attr->node, &acpi_table_attr_list);
}
kobject_uevent(tables_kobj, KOBJ_ADD);
--
2.7.4
On Friday, April 28, 2017 01:30:20 PM Lv Zheng wrote:
> For all frequent late stage acpi_get_table() clone invocations, we should
> only fix them altogether, otherwise, excessive acpi_put_table() could
> unexpectedly unmap the table used by the other users. Thus the current plan
> is to fix all acpi_get_table() clones together or to fix none of them.
I honestly don't think that fixing none of them is a valid option here.
> This prevents kernel developers from improving the late stage code quality
> without waiting for the ACPICA upstream to improve first.
>
> This patch adds a mechanism to stop decrementing validation count to
> prevent the table unmapping operations so that acpi_put_table() balance
> fixes can be done independently to each others.
>
> Cc: Dan Williams <[email protected]>
> Signed-off-by: Lv Zheng <[email protected]>
> ---
> drivers/acpi/acpica/tbutils.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
> index 7abe665..b517bd0 100644
> --- a/drivers/acpi/acpica/tbutils.c
> +++ b/drivers/acpi/acpica/tbutils.c
> @@ -445,12 +445,18 @@ void acpi_tb_put_table(struct acpi_table_desc *table_desc)
>
> ACPI_FUNCTION_TRACE(acpi_tb_put_table);
>
> - if (table_desc->validation_count == 0) {
> + if ((table_desc->validation_count + 1) == 0) {
This means that validation_count has reached the maximum value, right?
> ACPI_WARNING((AE_INFO,
> - "Table %p, Validation count is zero before decrement\n",
> + "Table %p, Validation count is about to expire, decrement is unsafe\n",
> table_desc));
So why is it unsafe to decrement it?
> return_VOID;
> }
> + if (table_desc->validation_count == 0) {
> + ACPI_ERROR((AE_INFO,
> + "Table %p, Validation count is zero before decrement\n",
> + table_desc));
> + return_VOID;
> + }
> table_desc->validation_count--;
>
> if (table_desc->validation_count == 0) {
>
Thanks,
Rafael
Considering this case:
1. A program opens a sysfs table file 65535 times, it can increase
validation_count and first increment cause the table to be mapped:
validation_count = 65535
2. AML execution causes "Load" to be executed on the same table, this time
it cannot increase validation_count, so validation_count remains:
validation_count = 65535
3. The program closes sysfs table file 65535 times, it can decrease
validation_count and the last decrement cause the table to be unmapped:
validation_count = 0
4. AML code still accessing the loaded table, kernel crash can be observed.
This is because orginally ACPICA doesn't support unmapping tables during
OS late stage. So the current code only allows unmapping tables during OS
early stage, and for late stage, no acpi_put_table() clones should be
invoked, especially cases that can trigger frequent invocations of
acpi_get_table()/acpi_put_table() are forbidden:
1. sysfs table accesses
2. dynamic Load/Unload opcode executions
3. acpi_load_table()
4. etc.
Such frequent acpi_put_table() balance changes have to be done altogether.
This philosophy is not convenient for Linux driver writers. Since the API
is just there, developers will start to use acpi_put_table() during late
stage. So we need to consider a better mechanism to allow them to safely
invoke acpi_put_table().
This patch provides such a mechanism by adding a validation_count
threashold. When it is reached, the validation_count can no longer be
incremented/decremented to invalidate the table descriptor (means
preventing table unmappings) so that acpi_put_table() balance changes can be
done independently to each others.
Note: code added in acpi_tb_put_table() is actually a no-op but changes the
warning message into a warning once message. Lv Zheng.
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/acpica/tbutils.c | 36 +++++++++++++++++++++++++++---------
include/acpi/actbl.h | 13 +++++++++++++
2 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index cd96026..4048523 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -416,9 +416,19 @@ acpi_tb_get_table(struct acpi_table_desc *table_desc,
}
}
- table_desc->validation_count++;
- if (table_desc->validation_count == 0) {
- table_desc->validation_count--;
+ if (table_desc->validation_count < ACPI_MAX_TABLE_VALIDATIONS) {
+ table_desc->validation_count++;
+
+ /*
+ * Ensure the warning message can only be displayed once. The
+ * warning message occurs when the "get" operations are performed
+ * more than "put" operations, causing count overflow.
+ */
+ if (table_desc->validation_count >= ACPI_MAX_TABLE_VALIDATIONS) {
+ ACPI_WARNING((AE_INFO,
+ "Table %p, Validation count overflows\n",
+ table_desc));
+ }
}
*out_table = table_desc->pointer;
@@ -445,13 +455,21 @@ void acpi_tb_put_table(struct acpi_table_desc *table_desc)
ACPI_FUNCTION_TRACE(acpi_tb_put_table);
- if (table_desc->validation_count == 0) {
- ACPI_WARNING((AE_INFO,
- "Table %p, Validation count is zero before decrement\n",
- table_desc));
- return_VOID;
+ if (table_desc->validation_count < ACPI_MAX_TABLE_VALIDATIONS) {
+ table_desc->validation_count--;
+
+ /*
+ * Ensure the warning message can only be displayed once. The
+ * warning message occurs when the "put" operations are performed
+ * more than "get" operations, causing count underflow.
+ */
+ if (table_desc->validation_count >= ACPI_MAX_TABLE_VALIDATIONS) {
+ ACPI_WARNING((AE_INFO,
+ "Table %p, Validation count underflows\n",
+ table_desc));
+ return_VOID;
+ }
}
- table_desc->validation_count--;
if (table_desc->validation_count == 0) {
diff --git a/include/acpi/actbl.h b/include/acpi/actbl.h
index d92543f..f42e6d5 100644
--- a/include/acpi/actbl.h
+++ b/include/acpi/actbl.h
@@ -374,6 +374,19 @@ struct acpi_table_desc {
u16 validation_count;
};
+/*
+ * Maximum validation count, when it is reached, validation count can no
+ * longer be changed. Which means, the table can no longer be invalidated.
+ * This mechanism is implemented for backward compatibility, where in OS
+ * late stage, old drivers are not facilitated with paired validations and
+ * invalidations.
+ * The maximum validation count can be defined to any value, but should be
+ * greater than the maximum number of OS early stage mapping slots as it
+ * must be ensured that no early stage mappings can be leaked to the late
+ * stage.
+ */
+#define ACPI_MAX_TABLE_VALIDATIONS ACPI_UINT16_MAX
+
/* Masks for Flags field above */
#define ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL (0) /* Virtual address, external maintained */
--
2.7.4
On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng <[email protected]> wrote:
> Considering this case:
> 1. A program opens a sysfs table file 65535 times, it can increase
> validation_count and first increment cause the table to be mapped:
> validation_count = 65535
> 2. AML execution causes "Load" to be executed on the same table, this time
> it cannot increase validation_count, so validation_count remains:
> validation_count = 65535
> 3. The program closes sysfs table file 65535 times, it can decrease
> validation_count and the last decrement cause the table to be unmapped:
> validation_count = 0
> 4. AML code still accessing the loaded table, kernel crash can be observed.
>
> This is because orginally ACPICA doesn't support unmapping tables during
> OS late stage. So the current code only allows unmapping tables during OS
> early stage, and for late stage, no acpi_put_table() clones should be
> invoked, especially cases that can trigger frequent invocations of
> acpi_get_table()/acpi_put_table() are forbidden:
> 1. sysfs table accesses
> 2. dynamic Load/Unload opcode executions
> 3. acpi_load_table()
> 4. etc.
> Such frequent acpi_put_table() balance changes have to be done altogether.
>
> This philosophy is not convenient for Linux driver writers. Since the API
> is just there, developers will start to use acpi_put_table() during late
> stage. So we need to consider a better mechanism to allow them to safely
> invoke acpi_put_table().
>
> This patch provides such a mechanism by adding a validation_count
> threashold. When it is reached, the validation_count can no longer be
> incremented/decremented to invalidate the table descriptor (means
> preventing table unmappings) so that acpi_put_table() balance changes can be
> done independently to each others.
>
> Note: code added in acpi_tb_put_table() is actually a no-op but changes the
> warning message into a warning once message. Lv Zheng.
>
This still seems to be unnecessary gymnastics to keep the validation
count around and make it work for random drivers. Which ACPI tables
might be hot removed? If it's only a small handful of tables why not
teach the code that handles those exceptional cases to manage a
dedicated reference count mechanism? That way the other cases can be
left alone and not worry about balancing their references.
On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams <[email protected]> wrote:
> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng <[email protected]> wrote:
>> Considering this case:
>> 1. A program opens a sysfs table file 65535 times, it can increase
>> validation_count and first increment cause the table to be mapped:
>> validation_count = 65535
>> 2. AML execution causes "Load" to be executed on the same table, this time
>> it cannot increase validation_count, so validation_count remains:
>> validation_count = 65535
>> 3. The program closes sysfs table file 65535 times, it can decrease
>> validation_count and the last decrement cause the table to be unmapped:
>> validation_count = 0
>> 4. AML code still accessing the loaded table, kernel crash can be observed.
>>
>> This is because orginally ACPICA doesn't support unmapping tables during
>> OS late stage. So the current code only allows unmapping tables during OS
>> early stage, and for late stage, no acpi_put_table() clones should be
>> invoked, especially cases that can trigger frequent invocations of
>> acpi_get_table()/acpi_put_table() are forbidden:
>> 1. sysfs table accesses
>> 2. dynamic Load/Unload opcode executions
>> 3. acpi_load_table()
>> 4. etc.
>> Such frequent acpi_put_table() balance changes have to be done altogether.
>>
>> This philosophy is not convenient for Linux driver writers. Since the API
>> is just there, developers will start to use acpi_put_table() during late
>> stage. So we need to consider a better mechanism to allow them to safely
>> invoke acpi_put_table().
>>
>> This patch provides such a mechanism by adding a validation_count
>> threashold. When it is reached, the validation_count can no longer be
>> incremented/decremented to invalidate the table descriptor (means
>> preventing table unmappings) so that acpi_put_table() balance changes can be
>> done independently to each others.
>>
>> Note: code added in acpi_tb_put_table() is actually a no-op but changes the
>> warning message into a warning once message. Lv Zheng.
>>
>
> This still seems to be unnecessary gymnastics to keep the validation
> count around and make it work for random drivers.
Well, I'm not sure I agree here.
If we can make it work at one point, it should not be too hard to
maintain that status.
Thanks,
Rafael
On Wed, Jun 7, 2017 at 2:14 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams <[email protected]> wrote:
>> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng <[email protected]> wrote:
>>> Considering this case:
>>> 1. A program opens a sysfs table file 65535 times, it can increase
>>> validation_count and first increment cause the table to be mapped:
>>> validation_count = 65535
>>> 2. AML execution causes "Load" to be executed on the same table, this time
>>> it cannot increase validation_count, so validation_count remains:
>>> validation_count = 65535
>>> 3. The program closes sysfs table file 65535 times, it can decrease
>>> validation_count and the last decrement cause the table to be unmapped:
>>> validation_count = 0
>>> 4. AML code still accessing the loaded table, kernel crash can be observed.
>>>
>>> This is because orginally ACPICA doesn't support unmapping tables during
>>> OS late stage. So the current code only allows unmapping tables during OS
>>> early stage, and for late stage, no acpi_put_table() clones should be
>>> invoked, especially cases that can trigger frequent invocations of
>>> acpi_get_table()/acpi_put_table() are forbidden:
>>> 1. sysfs table accesses
>>> 2. dynamic Load/Unload opcode executions
>>> 3. acpi_load_table()
>>> 4. etc.
>>> Such frequent acpi_put_table() balance changes have to be done altogether.
>>>
>>> This philosophy is not convenient for Linux driver writers. Since the API
>>> is just there, developers will start to use acpi_put_table() during late
>>> stage. So we need to consider a better mechanism to allow them to safely
>>> invoke acpi_put_table().
>>>
>>> This patch provides such a mechanism by adding a validation_count
>>> threashold. When it is reached, the validation_count can no longer be
>>> incremented/decremented to invalidate the table descriptor (means
>>> preventing table unmappings) so that acpi_put_table() balance changes can be
>>> done independently to each others.
>>>
>>> Note: code added in acpi_tb_put_table() is actually a no-op but changes the
>>> warning message into a warning once message. Lv Zheng.
>>>
>>
>> This still seems to be unnecessary gymnastics to keep the validation
>> count around and make it work for random drivers.
>
> Well, I'm not sure I agree here.
>
> If we can make it work at one point, it should not be too hard to
> maintain that status.
>
I agree with that, my concern was with driver writers needing to be
worried about when it is safe to call acpi_put_table(). This reference
count behaves differently than other reference counts like kobjects.
The difference is not necessarily bad, but hopefully it can be
contained within the acpi core.
Hi, Dan
> From: Dan Williams [mailto:[email protected]]
> Subject: Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table()
> independently
>
> On Wed, Jun 7, 2017 at 2:14 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams <[email protected]> wrote:
> >> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng <[email protected]> wrote:
> >>> Considering this case:
> >>> 1. A program opens a sysfs table file 65535 times, it can increase
> >>> validation_count and first increment cause the table to be mapped:
> >>> validation_count = 65535
> >>> 2. AML execution causes "Load" to be executed on the same table, this time
> >>> it cannot increase validation_count, so validation_count remains:
> >>> validation_count = 65535
> >>> 3. The program closes sysfs table file 65535 times, it can decrease
> >>> validation_count and the last decrement cause the table to be unmapped:
> >>> validation_count = 0
> >>> 4. AML code still accessing the loaded table, kernel crash can be observed.
> >>>
> >>> This is because orginally ACPICA doesn't support unmapping tables during
> >>> OS late stage. So the current code only allows unmapping tables during OS
> >>> early stage, and for late stage, no acpi_put_table() clones should be
> >>> invoked, especially cases that can trigger frequent invocations of
> >>> acpi_get_table()/acpi_put_table() are forbidden:
> >>> 1. sysfs table accesses
> >>> 2. dynamic Load/Unload opcode executions
> >>> 3. acpi_load_table()
> >>> 4. etc.
> >>> Such frequent acpi_put_table() balance changes have to be done altogether.
> >>>
> >>> This philosophy is not convenient for Linux driver writers. Since the API
> >>> is just there, developers will start to use acpi_put_table() during late
> >>> stage. So we need to consider a better mechanism to allow them to safely
> >>> invoke acpi_put_table().
> >>>
> >>> This patch provides such a mechanism by adding a validation_count
> >>> threashold. When it is reached, the validation_count can no longer be
> >>> incremented/decremented to invalidate the table descriptor (means
> >>> preventing table unmappings) so that acpi_put_table() balance changes can be
> >>> done independently to each others.
> >>>
> >>> Note: code added in acpi_tb_put_table() is actually a no-op but changes the
> >>> warning message into a warning once message. Lv Zheng.
> >>>
> >>
> >> This still seems to be unnecessary gymnastics to keep the validation
> >> count around and make it work for random drivers.
> >
> > Well, I'm not sure I agree here.
> >
> > If we can make it work at one point, it should not be too hard to
> > maintain that status.
> >
>
> I agree with that, my concern was with driver writers needing to be
> worried about when it is safe to call acpi_put_table(). This reference
> count behaves differently than other reference counts like kobjects.
I don't think they behave differently.
"kref" needn't consider unbalanced "get/put".
Because when the drivers(users) are deploying "kref",
they are responsible for ensuring balanced "get/put".
"kref" needn't take too much care about "overflow/underflow"
as if all users ensure balanced "get/put",
"overflow/underflow" is not possible.
Occurrence of "overflow/underflow" means bugs.
And can be further captured as "panic".
If "kref" considers to "warn_once" overflow/underflow users,
the logic in this commit can also be introduced to kref.
However it's useless as all users have ensured balanced "get/put".
Putting useless check than panic on hot path could be a waste.
> The difference is not necessarily bad, but hopefully it can be
> contained within the acpi core.
The old warning logic for table desc is just derived from utdelete.c.
Which reduces communication cost when the mechanism is upstreamed.
ACPICA table "validation_count" is deployed on top of old design.
Where "table unmap" is forbidden for late stage.
Thus there is no users ensuring balanced "get/put".
Under this circumstances, when we start to deploy balanced "get/put",
we need to consider all users as a whole.
You cannot say current unbalanced "get/put" users have bugs.
They are there just because of historical reasons.
Fortunately after applying this patch,
drivers should be able to have a better environment to use the new APIs.
Cheers,
Lv