From patchwork Thu Jul 2 06:27:25 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lin Ming X-Patchwork-Id: 33626 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n626QGTn027883 for ; Thu, 2 Jul 2009 06:26:16 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751584AbZGBG0L (ORCPT ); Thu, 2 Jul 2009 02:26:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752217AbZGBG0L (ORCPT ); Thu, 2 Jul 2009 02:26:11 -0400 Received: from mga02.intel.com ([134.134.136.20]:20255 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751584AbZGBG0J (ORCPT ); Thu, 2 Jul 2009 02:26:09 -0400 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 01 Jul 2009 23:16:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.42,331,1243839600"; d="scan'208";a="427054627" Received: from minggr.sh.intel.com (HELO [10.239.13.35]) ([10.239.13.35]) by orsmga002.jf.intel.com with ESMTP; 01 Jul 2009 23:33:16 -0700 Subject: Re: [PATCH] ACPICA: Revert "ACPICA: Remove obsolete acpi_os_validate_address interface" From: Lin Ming To: Len Brown Cc: Thomas Renninger , "Moore, Robert" , "Zhao, Yakui" , linux-acpi In-Reply-To: References: <1246415391.24831.30.camel@minggr.sh.intel.com> Date: Thu, 02 Jul 2009 14:27:25 +0800 Message-Id: <1246516045.3326.0.camel@minggr.sh.intel.com> Mime-Version: 1.0 X-Mailer: Evolution 2.24.1 (2.24.1-2.fc10) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Thu, 2009-07-02 at 10:03 +0800, Len Brown wrote: > I can't apply a patch that adds a known memory leak, > even if it removes a conflict between drivers. > > The reason is that there is a workaround for the driver conflict, > but no workaround for the memory leak. Here is a prototype simple patch, please review to see if it is the right way to go. 1) add a new field "count" to struct acpi_res_list. When inserting, if the region(addr, len) is already in the resource list, we just increase "count", otherwise, the region is inserted with count=1. When deleting, the "count" is decreased, if it's decreased to 0, the region is deleted from the resource list. With "count", the region with same address and length can only be inserted to the resource list once, so prevent potential memory leak. 2) add a new function acpi_os_invalidate_address, which is called when region is deleted. --- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c index bc17103..96e26e7 100644 --- a/drivers/acpi/acpica/utdelete.c +++ b/drivers/acpi/acpica/utdelete.c @@ -215,6 +215,12 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object) ACPI_DEBUG_PRINT((ACPI_DB_ALLOCATIONS, "***** Region %p\n", object)); + /* Invalidate the region address/length via the host OS */ + + acpi_os_invalidate_address(object->region.space_id, + object->region.address, + (acpi_size) object->region.length); + second_desc = acpi_ns_get_secondary_object(object); if (second_desc) { /* diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 7167071..620a65a 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -88,6 +88,7 @@ struct acpi_res_list { char name[5]; /* only can have a length of 4 chars, make use of this one instead of res->name, no need to kalloc then */ struct list_head resource_list; + int count; }; static LIST_HEAD(resource_list_head); @@ -1333,6 +1334,88 @@ acpi_os_validate_interface (char *interface) return AE_SUPPORT; } +static inline int acpi_res_list_add(struct acpi_res_list *res) +{ + struct acpi_res_list *res_list_elem; + + list_for_each_entry(res_list_elem, &resource_list_head, + resource_list) { + + if (res->resource_type == res_list_elem->resource_type && + res->start == res_list_elem->start && + res->end == res_list_elem->end) { + + /* + * The Region(addr,len) already exist in the list, + * just increase the count + */ + + res_list_elem->count++; + return 0; + } + } + + res->count = 1; + list_add(&res->resource_list, &resource_list_head); + return 1; +} + +static inline void acpi_res_list_del(struct acpi_res_list *res) +{ + struct acpi_res_list *res_list_elem; + + list_for_each_entry(res_list_elem, &resource_list_head, + resource_list) { + + if (res->resource_type == res_list_elem->resource_type && + res->start == res_list_elem->start && + res->end == res_list_elem->end) { + + /* + * If the res count is decreased to 0, remove and free it + */ + + if (--res_list_elem->count == 0) { + list_del(&res_list_elem->resource_list); + kfree(res_list_elem); + } + return; + } + } +} + +acpi_status +acpi_os_invalidate_address( + u8 space_id, + acpi_physical_address address, + acpi_size length) +{ + struct acpi_res_list res; + + switch (space_id) { + case ACPI_ADR_SPACE_SYSTEM_IO: + case ACPI_ADR_SPACE_SYSTEM_MEMORY: + /* Only interference checks against SystemIO and SytemMemory + are needed */ + res.start = address; + res.end = address + length - 1; + res.resource_type = space_id; + spin_lock(&acpi_res_lock); + acpi_res_list_del(&res); + spin_unlock(&acpi_res_lock); + break; + case ACPI_ADR_SPACE_PCI_CONFIG: + case ACPI_ADR_SPACE_EC: + case ACPI_ADR_SPACE_SMBUS: + case ACPI_ADR_SPACE_CMOS: + case ACPI_ADR_SPACE_PCI_BAR_TARGET: + case ACPI_ADR_SPACE_DATA_TABLE: + case ACPI_ADR_SPACE_FIXED_HARDWARE: + break; + } + return AE_OK; +} + /****************************************************************************** * * FUNCTION: acpi_os_validate_address @@ -1357,6 +1440,7 @@ acpi_os_validate_address ( char *name) { struct acpi_res_list *res; + int added; if (acpi_enforce_resources == ENFORCE_RESOURCES_NO) return AE_OK; @@ -1374,14 +1458,17 @@ acpi_os_validate_address ( res->end = address + length - 1; res->resource_type = space_id; spin_lock(&acpi_res_lock); - list_add(&res->resource_list, &resource_list_head); + added = acpi_res_list_add(res); spin_unlock(&acpi_res_lock); - pr_debug("Added %s resource: start: 0x%llx, end: 0x%llx, " - "name: %s\n", (space_id == ACPI_ADR_SPACE_SYSTEM_IO) + pr_debug("%s %s resource: start: 0x%llx, end: 0x%llx, " + "name: %s\n", added ? "Added" : "Already exist", + (space_id == ACPI_ADR_SPACE_SYSTEM_IO) ? "SystemIO" : "System Memory", (unsigned long long)res->start, (unsigned long long)res->end, res->name); + if (!added) + kfree(res); break; case ACPI_ADR_SPACE_PCI_CONFIG: case ACPI_ADR_SPACE_EC: diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h index ab0b85c..14d40bf 100644 --- a/include/acpi/acpiosxf.h +++ b/include/acpi/acpiosxf.h @@ -246,6 +246,10 @@ acpi_status acpi_os_validate_address(u8 space_id, acpi_physical_address address, acpi_size length, char *name); +acpi_status +acpi_os_invalidate_address(u8 space_id, acpi_physical_address address, + acpi_size length); + u64 acpi_os_get_timer(void); acpi_status acpi_os_signal(u32 function, void *info);