From: Jan Glauber <jglauber@redhat.com>
Subject: [RHEL5] page_mkclean causes data corruption on s390
Date: Thu, 05 Apr 2007 19:47:32 +0000
Bugzilla: 235375
Message-Id: <1175802452.5365.40.camel@localhost.localdomain>
Changelog: [s390] page_mkclean causes data corruption on s390


A serious bug from upstream is also in RHEL5 due to the backport of the 
"Fix nasty and subtle race in shared mmap'ed page writeback" from 2.6.20. 

The short story is that by having the dirty bit set for a page that is not
yet uptodate random filesystem data corruption could happen. The bug was not
trivial to detect, if the file is not critical the corruption may not be
noticed by the user.

With the patch various reproducers for the bug ran successfully without 
data corruption.

Patch was sent upstream yesterday:
http://marc.info/?l=linux-kernel&m=117570464411485&w=2

Jan
--

The git commit c2fda5fed81eea077363b285b66eafce20dfd45a which
added the page_test_and_clear_dirty call to page_mkclean and the
git commit 7658cc289288b8ae7dd2c2224549a048431222b3 which fixes
the "nasty and subtle race in shared mmap'ed page writeback"
problem in clear_page_dirty_for_io cause data corruption on s390.

The effect of the two changes is that for every call to
clear_page_dirty_for_io a page_test_and_clear_dirty is done. If
the per page dirty bit is set set_page_dirty is called. Strangly
clear_page_dirty_for_io is called for not-uptodate pages, e.g.
over this call-chain:

[<000000000007c0f2>] clear_page_dirty_for_io+0x12a/0x130
[<000000000007c494>] generic_writepages+0x258/0x3e0 
[<000000000007c692>] do_writepages+0x76/0x7c 
[<00000000000c7a26>] __writeback_single_inode+0xba/0x3e4
[<00000000000c831a>] sync_sb_inodes+0x23e/0x398 
[<00000000000c8802>] writeback_inodes+0x12e/0x140 
[<000000000007b9ee>] wb_kupdate+0xd2/0x178 
[<000000000007cca2>] pdflush+0x162/0x23c 

The bad news now is that page_test_and_clear_dirty might claim
that a not-uptodate page is dirty since SetPageUptodate which
resets the per page dirty bit has not yet been called. The page
writeback that follows clobbers the data on disk.

The simplest solution to this problem is to move the call to
page_test_and_clear_dirty under the "if (page_mapped(page))".
If a file backed page is mapped it is uptodate.

Index: linux-rhel5/mm/rmap.c
===================================================================
--- linux-rhel5.orig/mm/rmap.c	2007-03-15 17:12:01.000000000 +0100
+++ linux-rhel5/mm/rmap.c	2007-04-05 15:16:13.000000000 +0200
@@ -495,9 +495,9 @@ int page_mkclean(struct page *page)
 		struct address_space *mapping = page_mapping(page);
 		if (mapping)
 			ret = page_mkclean_file(mapping, page);
+		if (page_test_and_clear_dirty(page))
+			ret = 1;
 	}
-	if (page_test_and_clear_dirty(page))
-		ret = 1;
 
 	return ret;
 }