All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@suse.de>
To: Andi Kleen <andi@firstfloor.org>
Cc: hugh.dickins@tiscali.co.uk, riel@redhat.com,
	chris.mason@oracle.com, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	fengguang.wu@intel.com, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] [13/16] HWPOISON: The high level memory error handler in the VM v5
Date: Tue, 9 Jun 2009 13:14:32 +0200	[thread overview]
Message-ID: <20090609111432.GL14820@wotan.suse.de> (raw)
In-Reply-To: <20090609095155.GA14820@wotan.suse.de>

On Tue, Jun 09, 2009 at 11:51:55AM +0200, Nick Piggin wrote:
> On Wed, Jun 03, 2009 at 08:46:47PM +0200, Andi Kleen wrote:
> > +static int me_pagecache_clean(struct page *p, unsigned long pfn)
> > +{
> > +	struct address_space *mapping;
> > +
> > +	if (!isolate_lru_page(p))
> > +		page_cache_release(p);
> > +
> > +	/*
> > +	 * Now truncate the page in the page cache. This is really
> > +	 * more like a "temporary hole punch"
> > +	 * Don't do this for block devices when someone else
> > +	 * has a reference, because it could be file system metadata
> > +	 * and that's not safe to truncate.
> > +	 */
> > +	mapping = page_mapping(p);
> > +	if (mapping && S_ISBLK(mapping->host->i_mode) && page_count(p) > 1) {
> > +		printk(KERN_ERR
> > +			"MCE %#lx: page looks like a unsupported file system metadata page\n",
> > +			pfn);
> > +		return FAILED;
> > +	}
> 
> page_count check is racy. Hmm, S_ISBLK should handle xfs's private mapping.
> AFAIK btrfs has a similar private mapping but a quick grep does not show
> up S_IFBLK anywhere, so I don't know what the situation is there.
> 
> Unfortunately though, the linear mapping is not the only metadata mapping
> a filesystem might have. Many work on directories in seperate mappings
> (ext2, for example, which is where I first looked and will still oops with
> your check).
> 
> Also, others may have other interesting inodes they use for metadata. Do
> any of them go through the pagecache? I dont know. The ext3 journal,
> for example? How does that work?
> 
> Unfortunately I don't know a good way to detect regular data mappings
> easily. Ccing linux-fsdevel. Until that is worked out, you'd need to
> use the safe pagecache invalidate rather than unsafe truncate.

Maybe just testing S_ISREG would be better. Definitely safer than
ISBLK.

Note that for !ISREG files, then you can still attempt the
non-destructive invalidate (after extracting a suitable function
similarly to the truncate one). Most likely the fs is not using
the page right now, so it should give bit more coverage.

I still don't exactly know about, say, ext3 journal. Probably
it doesn't use pagecache anyway. Do any other filesystems do
crazy things with S_ISREG files? They probably deserve to oops
if they do ;)


WARNING: multiple messages have this Message-ID (diff)
From: Nick Piggin <npiggin@suse.de>
To: Andi Kleen <andi@firstfloor.org>
Cc: hugh.dickins@tiscali.co.uk, riel@redhat.com,
	chris.mason@oracle.com, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	fengguang.wu@intel.com, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] [13/16] HWPOISON: The high level memory error handler in the VM v5
Date: Tue, 9 Jun 2009 13:14:32 +0200	[thread overview]
Message-ID: <20090609111432.GL14820@wotan.suse.de> (raw)
In-Reply-To: <20090609095155.GA14820@wotan.suse.de>

On Tue, Jun 09, 2009 at 11:51:55AM +0200, Nick Piggin wrote:
> On Wed, Jun 03, 2009 at 08:46:47PM +0200, Andi Kleen wrote:
> > +static int me_pagecache_clean(struct page *p, unsigned long pfn)
> > +{
> > +	struct address_space *mapping;
> > +
> > +	if (!isolate_lru_page(p))
> > +		page_cache_release(p);
> > +
> > +	/*
> > +	 * Now truncate the page in the page cache. This is really
> > +	 * more like a "temporary hole punch"
> > +	 * Don't do this for block devices when someone else
> > +	 * has a reference, because it could be file system metadata
> > +	 * and that's not safe to truncate.
> > +	 */
> > +	mapping = page_mapping(p);
> > +	if (mapping && S_ISBLK(mapping->host->i_mode) && page_count(p) > 1) {
> > +		printk(KERN_ERR
> > +			"MCE %#lx: page looks like a unsupported file system metadata page\n",
> > +			pfn);
> > +		return FAILED;
> > +	}
> 
> page_count check is racy. Hmm, S_ISBLK should handle xfs's private mapping.
> AFAIK btrfs has a similar private mapping but a quick grep does not show
> up S_IFBLK anywhere, so I don't know what the situation is there.
> 
> Unfortunately though, the linear mapping is not the only metadata mapping
> a filesystem might have. Many work on directories in seperate mappings
> (ext2, for example, which is where I first looked and will still oops with
> your check).
> 
> Also, others may have other interesting inodes they use for metadata. Do
> any of them go through the pagecache? I dont know. The ext3 journal,
> for example? How does that work?
> 
> Unfortunately I don't know a good way to detect regular data mappings
> easily. Ccing linux-fsdevel. Until that is worked out, you'd need to
> use the safe pagecache invalidate rather than unsafe truncate.

Maybe just testing S_ISREG would be better. Definitely safer than
ISBLK.

Note that for !ISREG files, then you can still attempt the
non-destructive invalidate (after extracting a suitable function
similarly to the truncate one). Most likely the fs is not using
the page right now, so it should give bit more coverage.

I still don't exactly know about, say, ext3 journal. Probably
it doesn't use pagecache anyway. Do any other filesystems do
crazy things with S_ISREG files? They probably deserve to oops
if they do ;)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2009-06-09 11:14 UTC|newest]

Thread overview: 142+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-03 18:46 [PATCH] [0/16] HWPOISON: Intro Andi Kleen
2009-06-03 18:46 ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [1/16] HWPOISON: Add page flag for poisoned pages Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [2/16] HWPOISON: Export some rmap vma locking to outside world Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [3/16] HWPOISON: Add support for poison swap entries v2 Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [4/16] HWPOISON: Add new SIGBUS error codes for hardware poison signals Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [5/16] HWPOISON: Add basic support for poisoned pages in fault handler v3 Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [6/16] HWPOISON: Add various poison checks in mm/memory.c Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-04  4:26   ` Wu Fengguang
2009-06-04  4:26     ` Wu Fengguang
2009-06-04  5:19     ` Andi Kleen
2009-06-04  5:19       ` Andi Kleen
2009-06-04 11:55       ` Wu Fengguang
2009-06-04 11:55         ` Wu Fengguang
2009-06-04 12:52         ` Andi Kleen
2009-06-04 12:52           ` Andi Kleen
2009-06-04 12:50           ` Wu Fengguang
2009-06-04 12:50             ` Wu Fengguang
2009-06-04 13:02             ` Andi Kleen
2009-06-04 13:02               ` Andi Kleen
2009-06-04 13:16               ` Wu Fengguang
2009-06-04 13:16                 ` Wu Fengguang
2009-06-09 10:25   ` Nick Piggin
2009-06-09 10:25     ` Nick Piggin
2009-06-09 12:21     ` Wu Fengguang
2009-06-09 12:21       ` Wu Fengguang
2009-06-09 12:35       ` Nick Piggin
2009-06-09 12:35         ` Nick Piggin
2009-06-03 18:46 ` [PATCH] [7/16] HWPOISON: x86: Add VM_FAULT_HWPOISON handling to x86 page fault handler v2 Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-09  9:54   ` Nick Piggin
2009-06-09  9:54     ` Nick Piggin
2009-06-09 12:34     ` [PATCH] HWPOISON: define VM_FAULT_HWPOISON to 0 when feature is disabled Wu Fengguang
2009-06-09 12:34       ` Wu Fengguang
2009-06-03 18:46 ` [PATCH] [8/16] HWPOISON: Use bitmask/action code for try_to_unmap behaviour Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-09  9:57   ` Nick Piggin
2009-06-09  9:57     ` Nick Piggin
2009-06-10  2:27     ` Wu Fengguang
2009-06-10  2:27       ` Wu Fengguang
2009-06-10  6:07       ` Nick Piggin
2009-06-10  6:07         ` Nick Piggin
2009-06-03 18:46 ` [PATCH] [9/16] HWPOISON: Handle hardware poisoned pages in try_to_unmap Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-04  4:35   ` Wu Fengguang
2009-06-04  4:35     ` Wu Fengguang
2009-06-04  5:21     ` Andi Kleen
2009-06-04  5:21       ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [10/16] HWPOISON: Handle poisoned pages in set_page_dirty() Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-04  0:36   ` Wu Fengguang
2009-06-04  0:36     ` Wu Fengguang
2009-06-04  5:27     ` Andi Kleen
2009-06-04  5:27       ` Andi Kleen
2009-06-09  9:59   ` Nick Piggin
2009-06-09  9:59     ` Nick Piggin
2009-06-09 12:51     ` Wu Fengguang
2009-06-09 12:51       ` Wu Fengguang
2009-06-03 18:46 ` [PATCH] [11/16] HWPOISON: check and isolate corrupted free pages v2 Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-09 10:02   ` Nick Piggin
2009-06-09 10:02     ` Nick Piggin
2009-06-09 13:03     ` Wu Fengguang
2009-06-09 13:03       ` Wu Fengguang
2009-06-09 13:28       ` Nick Piggin
2009-06-09 13:28         ` Nick Piggin
2009-06-09 13:49         ` Wu Fengguang
2009-06-09 13:49           ` Wu Fengguang
2009-06-09 13:55           ` Nick Piggin
2009-06-09 13:55             ` Nick Piggin
2009-06-09 14:56             ` Wu Fengguang
2009-06-09 14:56               ` Wu Fengguang
2009-06-09 15:31               ` Nick Piggin
2009-06-09 15:31                 ` Nick Piggin
2009-06-03 18:46 ` [PATCH] [12/16] Refactor truncate to allow direct truncating of page Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-04  4:32   ` Wu Fengguang
2009-06-04  4:32     ` Wu Fengguang
2009-06-04  5:20     ` Andi Kleen
2009-06-04  5:20       ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [13/16] HWPOISON: The high level memory error handler in the VM v5 Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-04  3:24   ` Wu Fengguang
2009-06-04  3:24     ` Wu Fengguang
2009-06-04  5:13     ` Andi Kleen
2009-06-04  5:13       ` Andi Kleen
2009-06-04  9:07       ` Wu Fengguang
2009-06-04  9:07         ` Wu Fengguang
2009-06-04  9:26         ` Andi Kleen
2009-06-04  9:26           ` Andi Kleen
2009-06-09  9:51   ` Nick Piggin
2009-06-09  9:51     ` Nick Piggin
2009-06-09 11:14     ` Nick Piggin [this message]
2009-06-09 11:14       ` Nick Piggin
2009-06-09 10:09   ` Nick Piggin
2009-06-09 10:09     ` Nick Piggin
2009-06-09 16:05     ` Hugh Dickins
2009-06-09 16:05       ` Hugh Dickins
2009-06-09 16:35       ` Nick Piggin
2009-06-09 16:35         ` Nick Piggin
2009-06-10  8:38       ` Wu Fengguang
2009-06-10  8:38         ` Wu Fengguang
2009-06-10  8:59         ` Nick Piggin
2009-06-10  8:59           ` Nick Piggin
2009-06-10  9:20           ` Wu Fengguang
2009-06-10  9:20             ` Wu Fengguang
2009-06-10 11:03             ` Nick Piggin
2009-06-10 11:03               ` Nick Piggin
2009-06-10 12:16               ` Wu Fengguang
2009-06-10 12:16                 ` Wu Fengguang
2009-06-10 12:36                 ` Nick Piggin
2009-06-10 12:36                   ` Nick Piggin
2009-06-12  9:58       ` Andi Kleen
2009-06-12  9:58         ` Andi Kleen
2009-06-10  3:10     ` [PATCH] HWPOISON: fix tasklist_lock/anon_vma locking order Wu Fengguang
2009-06-10  3:10       ` Wu Fengguang
2009-06-03 18:46 ` [PATCH] [14/16] HWPOISON: FOR TESTING: Enable memory failure code unconditionally Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [15/16] HWPOISON: Add madvise() based injector for hardware poisoned pages v3 Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-03 18:46 ` [PATCH] [16/16] HWPOISON: Add simple debugfs interface to inject hwpoison on arbitary PFNs Andi Kleen
2009-06-03 18:46   ` Andi Kleen
2009-06-09 10:20 ` [PATCH] [0/16] HWPOISON: Intro Nick Piggin
2009-06-09 10:20   ` Nick Piggin
2009-06-10  9:07   ` Wu Fengguang
2009-06-10  9:07     ` Wu Fengguang
2009-06-10  9:18     ` Nick Piggin
2009-06-10  9:18       ` Nick Piggin
2009-06-10  9:45       ` Wu Fengguang
2009-06-10  9:45         ` Wu Fengguang
2009-06-10 11:15         ` Nick Piggin
2009-06-10 11:15           ` Nick Piggin
2009-06-10 12:36           ` Wu Fengguang
2009-06-10 12:36             ` Wu Fengguang
2009-06-10 12:47             ` Nick Piggin
2009-06-10 12:47               ` Nick Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090609111432.GL14820@wotan.suse.de \
    --to=npiggin@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=chris.mason@oracle.com \
    --cc=fengguang.wu@intel.com \
    --cc=hugh.dickins@tiscali.co.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.