Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753327Ab3HVPwz (ORCPT ); Thu, 22 Aug 2013 11:52:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18679 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752976Ab3HVPwy (ORCPT ); Thu, 22 Aug 2013 11:52:54 -0400 Date: Thu, 22 Aug 2013 11:51:32 -0400 From: Naoya Horiguchi To: Wanpeng Li Cc: Andrew Morton , Andi Kleen , Fengguang Wu , Tony Luck , gong.chen@linux.intel.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Message-ID: <1377186692-rqporagm-mutt-n-horiguchi@ah.jp.nec.com> In-Reply-To: <1377164907-24801-1-git-send-email-liwanp@linux.vnet.ibm.com> References: <1377164907-24801-1-git-send-email-liwanp@linux.vnet.ibm.com> Subject: Re: [PATCH 1/6] mm/hwpoison: fix lose PG_dirty flag for errors on mlocked pages Mime-Version: 1.0 Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Content-Disposition: inline X-Mutt-References: <1377164907-24801-1-git-send-email-liwanp@linux.vnet.ibm.com> X-Mutt-Fcc: ~/Maildir/sent/ User-Agent: Mutt 1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3578 Lines: 97 Hi Wanpeng, On Thu, Aug 22, 2013 at 05:48:22PM +0800, Wanpeng Li wrote: > memory_failure() store the page flag of the error page before doing unmap, > and (only) if the first check with page flags at the time decided the error > page is unknown, it do the second check with the stored page flag since > memory_failure() does unmapping of the error pages before doing page_action(). > This unmapping changes the page state, especially page_remove_rmap() (called > from try_to_unmap_one()) clears PG_mlocked, so page_action() can't catch > mlocked pages after that. > > However, memory_failure() can't handle memory errors on dirty mlocked pages > correctly. try_to_unmap_one will move the dirty bit from pte to the physical > page, the second check lose it since it check the stored page flag. This patch > fix it by restore PG_dirty flag to stored page flag if the page is dirty. Right. And I'm guessing that the discrepancy between pte_dirty and PageDirty can happen on the situations rather than mlocked pages. Anyway, using both of page flags before and after unmapping looks right to me. Reviewed-by: Naoya Horiguchi > Testcase: > > #define _GNU_SOURCE > #include > #include > #include > #include > #include > > #define PAGES_TO_TEST 2 > #define PAGE_SIZE 4096 > > int main(void) > { > char *mem; > int i; > > mem = mmap(NULL, PAGES_TO_TEST * PAGE_SIZE, > PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_LOCKED, 0, 0); > > for (i = 0; i < PAGES_TO_TEST; i++) > mem[i * PAGE_SIZE] = 'a'; > > if (madvise(mem, PAGES_TO_TEST * PAGE_SIZE, MADV_HWPOISON) == -1) > return -1; > > return 0; > } > > Before patch: > > [ 912.839247] Injecting memory failure for page 7dfb8 at 7f6b4e37b000 > [ 912.839257] MCE 0x7dfb8: clean mlocked LRU page recovery: Recovered > [ 912.845550] MCE 0x7dfb8: clean mlocked LRU page still referenced by 1 users > [ 912.852586] Injecting memory failure for page 7e6aa at 7f6b4e37c000 > [ 912.852594] MCE 0x7e6aa: clean mlocked LRU page recovery: Recovered > [ 912.858936] MCE 0x7e6aa: clean mlocked LRU page still referenced by 1 users > > After patch: > > [ 163.590225] Injecting memory failure for page 91bc2f at 7f9f5b0e5000 > [ 163.590264] MCE 0x91bc2f: dirty mlocked LRU page recovery: Recovered > [ 163.596680] MCE 0x91bc2f: dirty mlocked LRU page still referenced by 1 users > [ 163.603831] Injecting memory failure for page 91cdd3 at 7f9f5b0e6000 > [ 163.603852] MCE 0x91cdd3: dirty mlocked LRU page recovery: Recovered > [ 163.610305] MCE 0x91cdd3: dirty mlocked LRU page still referenced by 1 users > > Signed-off-by: Wanpeng Li > --- > mm/memory-failure.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index bee58d8..e156084 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1206,6 +1206,9 @@ int memory_failure(unsigned long pfn, int trapno, int flags) > for (ps = error_states;; ps++) > if ((p->flags & ps->mask) == ps->res) > break; > + > + page_flags |= (p->flags & (1UL << PG_dirty)); > + > if (!ps->mask) > for (ps = error_states;; ps++) > if ((page_flags & ps->mask) == ps->res) > -- > 1.7.7.6 > -- 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/