From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f49.google.com (mail-oi0-f49.google.com [209.85.218.49]) by kanga.kvack.org (Postfix) with ESMTP id 76CE16B0038 for ; Wed, 9 Dec 2015 16:44:24 -0500 (EST) Received: by oige206 with SMTP id e206so34347353oig.2 for ; Wed, 09 Dec 2015 13:44:24 -0800 (PST) Received: from g9t5009.houston.hp.com (g9t5009.houston.hp.com. [15.240.92.67]) by mx.google.com with ESMTPS id n8si1353030oed.49.2015.12.09.13.44.23 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Dec 2015 13:44:23 -0800 (PST) Message-ID: <1449697455.29051.6.camel@hpe.com> Subject: Re: [PATCH v3 1/3] resource: Add @flags to region_intersects() From: Toshi Kani Date: Wed, 09 Dec 2015 14:44:15 -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> <1449168859.9855.54.camel@hpe.com> <20151203184051.GE3213@pd.tnic> <1449174925.9855.83.camel@hpe.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Dan Williams Cc: Linus Torvalds , Borislav Petkov , Tony Luck , Linux ACPI , "linux-nvdimm@lists.01.org" , "Rafael J. Wysocki" , "linux-kernel@vger.kernel.org" , Linux MM , Andrew Morton On Wed, 2015-12-09 at 08:25 -0800, Dan Williams wrote: > On Thu, Dec 3, 2015 at 12:35 PM, Toshi Kani wrote: > > On Thu, 2015-12-03 at 11:01 -0800, Linus Torvalds wrote: > > > On Thu, Dec 3, 2015 at 10:40 AM, Borislav Petkov > > > wrote: > > > > On Thu, Dec 03, 2015 at 11:54:19AM -0700, Toshi Kani wrote: > > > > > 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. > > > > > > > > What's wrong with adding a new type_flags to struct resource and > > > > not touching IORESOURCE_* at all? > > > > > > Bah. Both of these ideas are bogus. > > > > > > Just add a new flag. The bits are already modifiers that you can > > > *combine* to show what kind of resource it is, and we already have > > > things like IORESOURCE_PREFETCH etc, that are in *addition* to the > > > normal IORESOURCE_MEM bit. > > > > > > Just add another modifier: IORESOURCE_RAM. > > > > > > So it would still show up as IORESOURCE_MEM, but it would have > > > additional information specifying that it's actually RAM. > > > > > > If somebody does something like > > > > > > if (res->flags == IORESOURCE_MEM) > > > > > > then they are already completely broken and won't work *anyway*. It's > > > a bitmask, bit a set of values. > > > > Yes, if we can assign new modifiers, that will be quite simple. :-) I > > assume we can allocate new bits from the remaining free bits as > > follows. > > > > +#define IORESOURCE_SYSTEM_RAM 0x01000000 /* System RAM */ > > +#define IORESOURCE_PMEM 0x02000000 /* Persistent memory */ > > #define IORESOURCE_EXCLUSIVE 0x08000000 /* Userland may not map > > this resource */ > > > > Note, SYSTEM_RAM represents the OS memory, i.e. "System RAM", not any > > RAM ranges. > > > > With the new modifiers, region_intersect() can check these ranges. One > > caveat is that the modifiers are not very extensible for new types as > > they are bit maps. region_intersect() will no longer be capable of > > checking any regions with any given name. I think this is OK since > > this function was introduced recently, and is only used for checking > > "System RAM" and "Persistent Memory" (with this patch series). > > IORESOURCE_PMEM is not descriptive enough for the two different types > of pmem in the kernel. How about we go with just > IORESOURCE_SYSTEM_RAM for now since "is_ram()" checks are common. Let > the rest continue to be checked by strcmp(). > > For example the nvdimm-e820 driver cares about "Persistent Memory > (legacy)", while other forms of pmem may just be "reserved" and only > the driver knows that it is pmem. An IORESOURCE_PMEM would not be > reliable nor descriptive enough. Agreed. I will introduce a new type for System RAM, and leave the strcmp check for other types. Thanks, -Toshi -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org