xfs
[Top] [All Lists]

Re: [PATCH] mm: Fix XFS oops due to dirty pages without buffers on s390

To: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
Subject: Re: [PATCH] mm: Fix XFS oops due to dirty pages without buffers on s390
From: Hugh Dickins <hughd@xxxxxxxxxx>
Date: Wed, 10 Oct 2012 14:57:48 -0700 (PDT)
Cc: Jan Kara <jack@xxxxxxx>, linux-mm@xxxxxxxxx, LKML <linux-kernel@xxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, Mel Gorman <mgorman@xxxxxxx>, linux-s390@xxxxxxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:x-x-sender:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; bh=JrxcqF41Ui+vl2VguhBfdocbtq67/lmorwAsewTT9nQ=; b=VMLWmjzaJZG/mciVRNoSFOocJWw74cdLL2+isSf6PqCPfLdlCq68yTRwn8KCnwgWnK 961/BP2pjFDvthAqbNjpguwVJMN9x49WUn5ulXWaMnUECJ4QNFED5r0iYSE8cc/Mbhaw /BqZH6hm89fd2T08lJkhLw7qBTlSTwI4aBVvHW24JKRT+HLzY0GZVgGBgmMV/S/+e1A8 cZn7ZaWJ+ZmbBPwtGCbhofYAyjhFV3XrVVGm1WJBXHV5Hk438lJC2WoCouAO1Z3gu1ew h2K65x/W+BjBANRNZHyTJtAfStSDN/00yLu6utidHuOG46ZvcLVZ0Pnz3RG92RfT/ITS iwUw==
In-reply-to: <alpine.LSU.2.00.1210091600450.30446@xxxxxxxxxxxx>
References: <1349108796-32161-1-git-send-email-jack@xxxxxxx> <alpine.LSU.2.00.1210082029190.2237@xxxxxxxxxxxx> <20121009101822.79bdcb65@mschwide> <alpine.LSU.2.00.1210091600450.30446@xxxxxxxxxxxx>
User-agent: Alpine 2.00 (LSU 1167 2008-08-23)
On Tue, 9 Oct 2012, Hugh Dickins wrote:
> On Tue, 9 Oct 2012, Martin Schwidefsky wrote:
> > On Mon, 8 Oct 2012 21:24:40 -0700 (PDT)
> > Hugh Dickins <hughd@xxxxxxxxxx> wrote:
> > 
> > > A separate worry came to mind as I thought about your patch: where
> > > in page migration is s390's dirty storage key migrated from old page
> > > to new?  And if there is a problem there, that too should be fixed
> > > by what I propose in the previous paragraph.
> > 
> > That is covered by the SetPageUptodate() in migrate_page_copy().
> 
> I don't think so: that makes sure that the newpage is not marked
> dirty in storage key just because of the copy_highpage to it; but
> I see nothing to mark the newpage dirty in storage key when the
> old page was dirty there.

I went to prepare a patch to fix this, and ended up finding no such
problem to fix - which fits with how no such problem has been reported.

Most of it is handled by page migration's unmap_and_move() having to
unmap the old page first: so the old page will pass through the final
page_remove_rmap(), which will transfer storage key to page_dirty in
those cases which it deals with (with the old code, any file or swap
page; with the new code, any unaccounted file or swap page, now that
we realize the accounted files don't even need this); and page_dirty
is already properly migrated to the new page.

But that does leave one case behind: an anonymous page not yet in
swapcache, migrated via a swap-like migration entry.  But this case
is not a problem because PageDirty doesn't actually affect anything
for an anonymous page not in swapcache.  There are various places
where we set it, and its life-history is hard to make sense of, but
in fact it's meaningless in 2.6, where page reclaim adds anon to swap
(and sets PageDirty) whether the page was marked dirty before or not
(which makes sense when we use the ZERO_PAGE for anon read faults).

2.4 did behave differently: it was liable to free anon pages not
marked dirty, and I think most of our anon SetPageDirtys are just a
relic of those days - I do have a patch from 18 months ago to remove
them (adding PG_dirty to the flags which should not be set when a
page is freed), but there are usually more urgent things to attend
to than rebase and retest that.

Hugh

<Prev in Thread] Current Thread [Next in Thread>