Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752107AbbLCR7V (ORCPT ); Thu, 3 Dec 2015 12:59:21 -0500 Received: from g9t5008.houston.hp.com ([15.240.92.66]:55189 "EHLO g9t5008.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751325AbbLCR7U (ORCPT ); Thu, 3 Dec 2015 12:59:20 -0500 Message-ID: <1449168859.9855.54.camel@hpe.com> Subject: Re: [PATCH v3 1/3] resource: Add @flags to region_intersects() From: Toshi Kani To: Linus Torvalds , Borislav Petkov Cc: Dan Williams , Andrew Morton , "Rafael J. Wysocki" , Tony Luck , Vishal L Verma , Linux MM , "linux-nvdimm@lists.01.org" , Linux ACPI , "linux-kernel@vger.kernel.org" Date: Thu, 03 Dec 2015 11:54:19 -0700 In-Reply-To: References: <1448404418-28800-1-git-send-email-toshi.kani@hpe.com> <1448404418-28800-2-git-send-email-toshi.kani@hpe.com> <20151201135000.GB4341@pd.tnic> <20151201171322.GD4341@pd.tnic> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.5 (3.16.5-3.fc22) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3241 Lines: 74 On Tue, 2015-12-01 at 09:19 -0800, Linus Torvalds wrote: > On Tue, Dec 1, 2015 at 9:13 AM, Borislav Petkov wrote: > > > > Oh sure, I didn't mean you. I was simply questioning that whole > > identify-resource-by-its-name approach. And that came with: > > > > 67cf13ceed89 ("x86: optimize resource lookups for ioremap") > > > > I just think it is silly and that we should be identifying resource > > things in a more robust way. > > I could easily imagine just adding a IORESOURCE_RAM flag (or SYSMEM or > whatever). That sounds sane. I agree that comparing the string is > ugly. > > > Btw, the ->name thing in struct resource has been there since a *long* > > time > > It's pretty much always been there. It is indeed meant for things > like /proc/iomem etc, and as a debug aid when printing conflicts, > yadda yadda. Just showing the numbers is usually useless for figuring > out exactly *what* something conflicts with. I agree that regular memory should have its own type, which separates itself from MMIO. By looking at how IORESOURCE types are used, this change has the following challenges, and I am sure I missed some more. 1. Large number of IORESOURCE_MEM usage Adding a new type for regular memory will require inspecting the codes using IORESOURCE_MEM currently, and modify them to use the new type if their target ranges are regular memory. There are many references to this type across multiple architectures and drivers, which make this inspection and testing challenging. http://lxr.free-electrons.com/ident?i=IORESOURCE_MEM 2. Lack of free flags bit in resource The flags bits are defined in include/linux/ioport.h. The flags are defined as unsigned long, which is 32-bit in 32-bit config. The most of the bits have been assigned already. Bus-specific bits for IORESOURCE_MEM have been assigned mostly as well (line 82). 3. Interaction with pnp subsystem The same IORESOURCE types and bus-specific flags are used by the pnp subsystem. pnp_mem objects represent IORESOURCE_MEM type listed by pnp_dev. Adding a new IORESOURCE type likely requires adding a new object type and its interfaces to pnp. 4. I/O resource names represent allocation types While IORESOURCE types represent hardware types and capabilities, the string names represent resource allocation types and usages. For instance, regular memory is allocated for the OS as "System RAM", kdump as "Crash kernel", FW as "ACPI Tables", and so on. Hence, a new type representing "System RAM" needs to be usage based, which is different from the current IORESOURCE types. I think this work will require a separate patch series at least. For this patch series, supporting error injections to NVDIMM, I propose that we make the change suggested by Dan: "We could define 'const char *system_ram = "System RAM"' somewhere andthen do pointer comparisons to cut down on the thrash of adding newflags to 'struct resource'?" Let me know if you have any suggestions/concerns. Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/