Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753225AbbDBJhs (ORCPT ); Thu, 2 Apr 2015 05:37:48 -0400 Received: from mail-wi0-f179.google.com ([209.85.212.179]:35779 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753171AbbDBJhp (ORCPT ); Thu, 2 Apr 2015 05:37:45 -0400 Date: Thu, 2 Apr 2015 11:37:40 +0200 From: Ingo Molnar To: Christoph Hellwig Cc: Boaz Harrosh , linux-nvdimm@ml01.01.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, axboe@kernel.dk, ross.zwisler@linux.intel.com Subject: Re: [PATCH] SQUASHME: Fixes to e820 handling of pmem Message-ID: <20150402093740.GA3706@gmail.com> References: <1427872339-6688-1-git-send-email-hch@lst.de> <1427872339-6688-2-git-send-email-hch@lst.de> <551BFFD2.20608@plexistor.com> <20150402093037.GA14209@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150402093037.GA14209@lst.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1760 Lines: 51 * Christoph Hellwig wrote: > On Wed, Apr 01, 2015 at 05:25:22PM +0300, Boaz Harrosh wrote: > > pfn = PFN_DOWN(ei->addr + ei->size); > > > > - switch (ei->type) { > > - case E820_RAM: > > - case E820_PRAM: > > - case E820_RESERVED_KERN: > > - break; > > - default: > > + if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN) > > register_nosave_region(PFN_UP(ei->addr), pfn); > > - } > > I guess this makes sense - if the content is persistent already we don't need > to save it. > > > - if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20)) { > > - if (e820.map[i].type != E820_PRAM) > > - res->flags |= IORESOURCE_BUSY; > > + if (((e820.map[i].type != E820_RESERVED) && > > + (e820.map[i].type != E820_PRAM)) || > > + res->start < (1ULL<<20)) { > > So now we also trigger for PRAM regions under 1ULL<<20, was that the > intentional change? Honestly I don't really understand this 1ULL<<20 > magic here even for the existing case. Guess this is magic from the > old ISA PC days? > > > + res->flags |= IORESOURCE_BUSY; > > Guess this is the real change, and I'd love to understand why this > makes a difference for you. IORESOURCE_BUSY is checked almost > never, and is intented to mean it's a driver mapping. So assuming this works on your test setup I'm inclined to squash Boaz's fixes into the original patch, assuming you see no outright bug in them. Anything else can be done as delta improvements. Agreed? Thanks, Ingo -- 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/