Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754161AbdFWU7s (ORCPT ); Fri, 23 Jun 2017 16:59:48 -0400 Received: from mga07.intel.com ([134.134.136.100]:20112 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753900AbdFWU7q (ORCPT ); Fri, 23 Jun 2017 16:59:46 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,380,1493708400"; d="scan'208";a="118190820" Date: Fri, 23 Jun 2017 13:59:45 -0700 From: "Luck, Tony" To: Dan Williams Cc: "Elliott, Robert (Persistent Memory)" , Naoya Horiguchi , Borislav Petkov , "Hansen, Dave" , "x86@kernel.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "linux-nvdimm@lists.01.org" , "Kani, Toshimitsu" , "Vaden, Tom (HPE Server OS Architecture)" Subject: Re: [PATCH] mm/hwpoison: Clear PRESENT bit for kernel 1:1 mappings of poison pages Message-ID: <20170623205945.ovhfyymzfkevazpd@intel.com> References: <20170616190200.6210-1-tony.luck@intel.com> <20170621021226.GA18024@hori1.linux.bs1.fc.nec.co.jp> <20170621175403.n5kssz32e2oizl7k@intel.com> <3908561D78D1C84285E8C5FCA982C28F612DCCAF@ORSMSX114.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2323 Lines: 49 On Thu, Jun 22, 2017 at 10:07:18PM -0700, Dan Williams wrote: > On Wed, Jun 21, 2017 at 1:30 PM, Luck, Tony wrote: > >> Persistent memory does have unpoisoning and would require this inverse > >> operation - see drivers/nvdimm/pmem.c pmem_clear_poison() and core.c > >> nvdimm_clear_poison(). > > > > Nice. Well this code will need to cooperate with that ... in particular if the page > > is in an area that can be unpoisoned ... then we should do that *instead* of marking > > the page not present (which breaks up huge/large pages and so affects performance). > > > > Instead of calling it "arch_unmap_pfn" it could be called something like arch_handle_poison() > > and do something like: > > > > void arch_handle_poison(unsigned long pfn) > > { > > if this is a pmem page && pmem_clear_poison(pfn) > > return > > if this is a nvdimm page && nvdimm_clear_poison(pfn) > > return > > /* can't clear, map out from 1:1 region */ > > ... code from my patch ... > > } > > > > I'm just not sure how those first two "if" bits work ... particularly in terms of CONFIG dependencies and system > > capabilities. Perhaps each of pmem and nvdimm could register their unpoison functions and this code could > > just call each in turn? > > We don't unpoison pmem without new data to write in it's place. What > context is arch_handle_poison() called? Ideally we only "clear" poison > when we know we are trying to write zero over the poisoned range. Context is that of the process that did the access (but we've moved off the machine check stack and are now in normal kernel context). We are about to unmap this page from all applications that are using it. But they may be running ... so now it a bad time to clear the poison. They might access the page and not get a signal. If I move this code to after all the users PTEs have been cleared and TLBs flushed, then it would be safe to try to unpoison the page and not invalidate from the 1:1 mapping. But I'm not sure what happens next. For a normal DDR4 page I could put it back on the free list and allow it to be re-used. But for PMEM you have some other cleanup that you need to do to mark the block as lost from your file system. Is this too early for you to be able to do that? -Tony