EXT3: problem with page fault inside a transaction

While reading Andrew's generic_file_buffered_write patches i've remembered one more EXT3 issue.journal_start() in prepare_write() causes different ranking violations if copy_from_user() triggers a page fault. It could cause GFP_FS allocation, re-entering into ext3 code possibly with a different superblock and journal, ranking violation of journalling serialization and mmap_sem and page lock and all other kinds of funny consequences. Our customers complain about this issue.

Andrey Savochkin already discussed the issue in lkml.
( subj EXT3: problem with copy_from_user inside a transaction) as a result we slightly changed prepare_write()/ commit_write() semantic.

To eliminate this possibility we read in not up-to-date buffers in prepare_write(), and do the rest including hole instantiation and inode extension in commit_write(). ENOSPC case must be specificaly handled.
1) we ought to have simular error handling block as we have after prepare_write() because we may have instantiated a few blocks outside i_size.
2) We cant use block_prepare_write() for allocation needs inside commit_write() due to page cache already contains valid data.

Here is the patch moving journal_start() together with space allocation from ext3_prepare_write() to commit_write().
---
Reorganization of ext3_prepare_write/ext3_commit_write to eliminate the possibility of the page fault in between, inside a transaction. It could cause GFP_FS allocation, re-entering into
ext3 code possibly with a different superblock and journal, ranking violation of journalling serialization and mmap_sem and page lock and all other kinds of funny consequences.

---
 fs/buffer.c     |    5 +
 fs/ext3/inode.c |  194 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 mm/filemap.c    |   42 ++++++++----
 3 files changed, 207 insertions(+), 34 deletions(-)

index eeb8ac1..11bf84c 100644
@@ -1768,8 +1768,9 @@ static int __block_prepare_write(struct 
 			if (err)
 				break;
 			if (buffer_new(bh)) {
-				unmap_underlying_metadata(bh->b_bdev,
-							bh->b_blocknr);
+				if (buffer_mapped(bh))
+					unmap_underlying_metadata(bh->b_bdev,
+								  bh->b_blocknr);
 				if (PageUptodate(page)) {
 					set_buffer_uptodate(bh);
 					continue;

1.4.2.1

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

With the stuff Nick and I are looking at, we won't take pagefaults inside prepare_write()/commit_write() any more.

Really? How often?

What on earth are they doing to trigger this? writev() without the 2.6.18 writev() bugfix?

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

Yep. Because the page is locked, it is too much to deal with even without a filesystem in the picture.

SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

I'sorry may be i've missed something, but how cant you prevent this?

Let's look at generic_file_buffered_write:
#### force page fault
fault_in_pages_readable();

### find and lock page
__grab_cache_page()

#### allocate blocks.This may result in low memory condition #### try_to_free_pages->shrink_caches() and etc. a_ops->prepare_write()

### can anyone guarantee that page fault hasn't happened by now ? ### user space buffer swapped out, or became invalid. filemap_copy_from_user()

I have't concrete statistic

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

Yes. Do an atomic copy, which will early exit from the pagefault handler and return a short copy. Then close up the write, drop the page lock, and rerun the fault_in_pages_readable, which will do the full pagefaults for us, then try again.

Regardless of what you do to ext3, the VM just can't handle a fault here anyway.

SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

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

Start here: http://lkml.org/lkml/2006/10/11/12

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