From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH v5] ACPICA: Tables: Add mechanism to allow to balance late stage acpi_get_table() independently Date: Wed, 7 Jun 2017 23:14:27 +0200 Message-ID: References: <5361b51c7c257b3216475018a3a5cc4f8b6b21c6.1493281247.git.lv.zheng@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: Received: from mail-it0-f49.google.com ([209.85.214.49]:36340 "EHLO mail-it0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751469AbdFGVO2 (ORCPT ); Wed, 7 Jun 2017 17:14:28 -0400 In-Reply-To: Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Dan Williams Cc: Lv Zheng , "Rafael J . Wysocki" , "Rafael J . Wysocki" , Len Brown , Lv Zheng , Linux Kernel Mailing List , Linux ACPI 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