From mboxrd@z Thu Jan 1 00:00:00 1970 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 * 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