Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752507AbbBXH7R (ORCPT ); Tue, 24 Feb 2015 02:59:17 -0500 Received: from mail-wg0-f44.google.com ([74.125.82.44]:37520 "EHLO mail-wg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752445AbbBXH7O (ORCPT ); Tue, 24 Feb 2015 02:59:14 -0500 Message-ID: <54EC2F4E.1080606@plexistor.com> Date: Tue, 24 Feb 2015 09:59:10 +0200 From: Boaz Harrosh User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Dan Williams CC: Ingo Molnar , Ross Zwisler , x86@kernel.org, linux-kernel , "Roger C. Pao" , Thomas Gleixner , Linus Torvalds , linux-nvdimm , "H. Peter Anvin" , Matthew Wilcox , Andy Lutomirski , Christoph Hellwig Subject: Re: [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY References: <54EB1D33.3050107@plexistor.com> <54EB1E03.4010306@plexistor.com> <1424751767.9050.4.camel@intel.com> In-Reply-To: <1424751767.9050.4.camel@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4207 Lines: 117 On 02/24/2015 06:22 AM, Dan Williams wrote: <> >> By Popular demand An Extra WARNING message is printed if >> an "UNKNOWN" is found. It will look like this: >> e820: WARNING [mem 0x100000000-0x1ffffffff] is unknown type 12 > > I don't think we need to warn that an unknown range was published, just > warn if it is consumed. > I did not have it at first, Ingo asked for it. I don't mind having it and I don't mind not. I'd say it is Ingo's call. > Something like these incremental changes. I don't see the need for > patch 2 or either version of patch 3. > > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c > index 1afa5518baa6..2e755a92d84f 100644 > --- a/arch/x86/kernel/e820.c > +++ b/arch/x86/kernel/e820.c > @@ -134,11 +134,6 @@ static void __init __e820_add_region(struct e820map *e820x, u64 start, u64 size, > return; > } > > - if (unlikely(_is_unknown_type(type))) > - pr_warn("e820: WARNING [mem %#010llx-%#010llx] is unknown type %d\n", > - (unsigned long long) start, > - (unsigned long long) (start + size - 1), type); > - Again Ingo's call > e820x->map[x].addr = start; > e820x->map[x].size = size; > e820x->map[x].type = type; > @@ -938,7 +933,7 @@ static inline const char *e820_type_to_string(int e820_type) > case E820_NVS: return "ACPI Non-volatile Storage"; > case E820_UNUSABLE: return "Unusable memory"; > case E820_RESERVED: return "reserved"; > - default: return "reserved-unkown"; > + default: return iomem_unknown_resource_name; > } > } > > diff --git a/include/linux/ioport.h b/include/linux/ioport.h > index 2c5250222278..d857e79b4bf2 100644 > --- a/include/linux/ioport.h > +++ b/include/linux/ioport.h > @@ -194,6 +194,9 @@ extern struct resource * __request_region(struct resource *, > resource_size_t n, > const char *name, int flags); > > +/* For uniquely tagging unknown memory so we can warn when it is consumed */ > +extern const char iomem_unknown_resource_name[]; > + > /* Compatibility cruft */ > #define release_region(start,n) __release_region(&ioport_resource, (start), (n)) > #define check_mem_region(start,n) __check_region(&iomem_resource, (start), (n)) > diff --git a/kernel/resource.c b/kernel/resource.c > index 0bcebffc4e77..38b36c212a48 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -1040,6 +1040,8 @@ resource_size_t resource_alignment(struct resource *res) > > static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait); > > +const char iomem_unknown_resource_name[] = { "reserved-unknown" }; > + > /** > * __request_region - create a new busy resource region > * @parent: parent resource descriptor > @@ -1092,6 +1094,15 @@ struct resource * __request_region(struct resource *parent, > break; > } > write_unlock(&resource_lock); > + > + if (res && res->parent > + && res->parent->name == iomem_unknown_resource_name) { No, this is a complete HACK, since when do we hard code specific (GLOBAL) ARCHs strings in common code. Please look at linux/ioport.h see the richness of options for all kind of buses and systems. The flag system works perfectly and I just continue this here. And really DAN, you prefer a global string that's dead garbage in 99% of arches to a simple bit flag definition that costs nothing? I don't think so. > + add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK); NACK!! > + pr_warn("request unknown region [mem %#010llx-%#010llx] %s\n", > + res->start, res->end, > + res->name); > + } > + > return res; > } > EXPORT_SYMBOL(__request_region); > > I do not understand you guys. Really. Dan you are a Linux Kernel developer why do you want to go ask some committee an approval for each new type of device that you want to develop. Why not have a system where the BIOS just puts up a BAR and an ID, and the rest is up to the drivers you write in C in the Kernel? What is the motivation of the complication? I would really like to understand? Thanks Boaz -- 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/