[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.00.1012162239270.23229@sister.anvils>
Date: Thu, 16 Dec 2010 22:52:50 -0800 (PST)
From: Hugh Dickins <hughd@...gle.com>
To: Wu Fengguang <fengguang.wu@...el.com>
cc: Andrew Morton <akpm@...ux-foundation.org>, Jan Kara <jack@...e.cz>,
Christoph Hellwig <hch@....de>,
Trond Myklebust <Trond.Myklebust@...app.com>,
Dave Chinner <david@...morbit.com>,
"Theodore Ts'o" <tytso@....edu>,
Chris Mason <chris.mason@...cle.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Mel Gorman <mel@....ul.ie>, Rik van Riel <riel@...hat.com>,
KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Greg Thelen <gthelen@...gle.com>,
Minchan Kim <minchan.kim@...il.com>,
linux-mm <linux-mm@...ck.org>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 22/35] writeback: trace global dirty page states
On Fri, 17 Dec 2010, Wu Fengguang wrote:
> On Mon, Dec 13, 2010 at 10:47:08PM +0800, Wu, Fengguang wrote:
>
> > + TP_fast_assign(
> > + strlcpy(__entry->bdi,
> > + dev_name(mapping->backing_dev_info->dev), 32);
> > + __entry->ino = mapping->host->i_ino;
>
> I got an oops against the above line on shmem. Can be fixed by the
> below patch, but still not 100% confident..
>
> Thanks,
> Fengguang
> ---
> Subject: writeback fix dereferencing NULL shmem mapping->host
> Date: Thu Dec 16 22:22:00 CST 2010
>
> The oops happens when doing "cp /proc/vmstat /dev/shm". It seems to be
> triggered on accessing host->i_ino, since the offset of i_ino is exactly
> 0x50. However I'm afraid the problem is not fully understand
>
> 1) it's not normal that tmpfs will have mapping->host == NULL
>
> 2) I tried removing the dereference as the below diff, however it
> didn't stop the oops. This is very weird.
>
> TRACE_EVENT balance_dirty_state:
>
> TP_fast_assign(
> strlcpy(__entry->bdi,
> dev_name(mapping->backing_dev_info->dev), 32);
I believe this line above is actually the problem: you can imagine that
tmpfs leaves backing_dev_info->dev NULL, and dev_name() appears to
access dev->init_name at 64-bit offset 0x50 down struct device.
> - __entry->ino = mapping->host->i_ino;
> __entry->nr_dirty = nr_dirty;
> __entry->nr_writeback = nr_writeback;
> __entry->nr_unstable = nr_unstable;
...
>
> CC: Hugh Dickins <hugh.dickins@...cali.co.uk>
I prefer hughd@...gle.com, but the tiscali address survived unexpectedly.
> Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
> ---
> mm/page-writeback.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> --- linux-next.orig/mm/page-writeback.c 2010-12-17 09:30:11.000000000 +0800
> +++ linux-next/mm/page-writeback.c 2010-12-17 09:31:05.000000000 +0800
> @@ -907,6 +907,9 @@ void balance_dirty_pages_ratelimited_nr(
> {
> struct backing_dev_info *bdi = mapping->backing_dev_info;
>
> + if (!mapping_cap_writeback_dirty(mapping))
> + return;
> +
> current->nr_dirtied += nr_pages_dirtied;
>
> if (unlikely(!current->nr_dirtied_pause))
That would not really be the right patch to fix your oops, but it
or something like would be a very sensible patch in its own right:
looking back through old patches I never got around to sending in,
I can see I had a very similar one two years ago, to save wasting
time on dirty page accounting here when it's inappropriate.
Though mine was testing !mapping_cap_account_dirty(mapping).
Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists