RE: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Zheng, Lv]

Hi, Dan

> From: Dan Williams [mailto:dan.j.willi...@intel.com]
> 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 <raf...@kernel.org> wrote:
> > On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams <dan.j.willi...@intel.com> 
> > wrote:
> >> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng <lv.zh...@intel.com> 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

RE: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Zheng, Lv]

Hi, Dan

> From: Dan Williams [mailto:dan.j.willi...@intel.com]
> 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  wrote:
> > On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams  
> > wrote:
> >> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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

Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Dan Williams]

On Wed, Jun 7, 2017 at 2:14 PM, Rafael J. Wysocki  wrote:
> On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams  wrote:
>> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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.

Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Dan Williams]

On Wed, Jun 7, 2017 at 2:14 PM, Rafael J. Wysocki  wrote:
> On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams  wrote:
>> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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.

Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Rafael J. Wysocki]

On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams  wrote:
> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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

Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Rafael J. Wysocki]

On Wed, Jun 7, 2017 at 8:41 AM, Dan Williams  wrote:
> On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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

Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Dan Williams]

On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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.

Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Dan Williams]

On Tue, Jun 6, 2017 at 9:54 PM, Lv Zheng  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.

[PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Lv Zheng]

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 
---
 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
+ * 

[PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently

[Lv Zheng]

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 
---
 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