Skip to contentSkip to conversation

direct-to-BIO writeback for writeback-mode ext3

PATCH In review Andrew Morton 2 participants 4 messages Jun 1, 2002

lore
Share & tools
lore
#0 lore raw

PATCH 9/16 direct-to-BIO writeback for writeback-mode ext3

Turn on direct-to-BIO writeback for ext3 in data=writeback mode.

=====================================

fs/ext3/inode.c renamed from fs/ext3/inode.c~ext3-writepages +36 -2
@@ -1389,6 +1389,34 @@ struct address_space_operations ext3_aop
 	releasepage:	ext3_releasepage,	/* BKL not held.  Don't need */
 };
 
+/* For writeback mode, we can use mpage_writepages() */
+
+static int
+ext3_writepages(struct address_space *mapping, int *nr_to_write)
+{
+	int ret;
+	int err;
+
+	ret = write_mapping_buffers(mapping);
+	err = mpage_writepages(mapping, nr_to_write, ext3_get_block);
+	if (!ret)
+		ret = err;
+	return ret;
+}
+
+struct address_space_operations ext3_writeback_aops = {
+	readpage:	ext3_readpage,		/* BKL not held.  Don't need */
+	readpages:	ext3_readpages,		/* BKL not held.  Don't need */
+	writepage:	ext3_writepage,		/* BKL not held.  We take it */
+	writepages:	ext3_writepages,	/* BKL not held.  Don't need */
+	sync_page:	block_sync_page,
+	prepare_write:	ext3_prepare_write,	/* BKL not held.  We take it */
+	commit_write:	ext3_commit_write,	/* BKL not held.  We take it */
+	bmap:		ext3_bmap,		/* BKL held */
+	flushpage:	ext3_flushpage,		/* BKL not held.  Don't need */
+	releasepage:	ext3_releasepage,	/* BKL not held.  Don't need */
+};
+
 /*
  * ext3_block_truncate_page() zeroes out a mapping from file offset `from'
  * up to the end of the block which corresponds to `from'.
@@ -2159,7 +2187,10 @@ void ext3_read_inode(struct inode * inod
 	else if (S_ISREG(inode->i_mode)) {
 		inode->i_op = &ext3_file_inode_operations;
 		inode->i_fop = &ext3_file_operations;
-		inode->i_mapping->a_ops = &ext3_aops;
+		if (ext3_should_writeback_data(inode))
+			inode->i_mapping->a_ops = &ext3_writeback_aops;
+		else
+			inode->i_mapping->a_ops = &ext3_aops;
 	} else if (S_ISDIR(inode->i_mode)) {
 		inode->i_op = &ext3_dir_inode_operations;
 		inode->i_fop = &ext3_dir_operations;
@@ -2168,7 +2199,10 @@ void ext3_read_inode(struct inode * inod
 			inode->i_op = &ext3_fast_symlink_inode_operations;
 		else {
 			inode->i_op = &page_symlink_inode_operations;
-			inode->i_mapping->a_ops = &ext3_aops;
+			if (ext3_should_writeback_data(inode))
+				inode->i_mapping->a_ops = &ext3_writeback_aops;
+			else
+				inode->i_mapping->a_ops = &ext3_aops;
 		}
 	} else 
 		init_special_inode(inode, inode->i_mode,
include/linux/ext3_jbd.h renamed from include/linux/ext3_jbd.h~ext3-writepages +5 -0
@@ -299,5 +299,10 @@ static inline int ext3_should_order_data
 	return (test_opt(inode->i_sb, DATA_FLAGS) == EXT3_MOUNT_ORDERED_DATA);
 }
 
+static inline int ext3_should_writeback_data(struct inode *inode)
+{
+	return !ext3_should_journal_data(inode) &&
+			!ext3_should_order_data(inode);
+}
 
 #endif	/* _LINUX_EXT3_JBD_H */
include/linux/ext3_fs.h renamed from include/linux/ext3_fs.h~ext3-writepages +1 -0
@@ -695,6 +695,7 @@ extern struct file_operations ext3_file_
 
 /* inode.c */
 extern struct address_space_operations ext3_aops;
+extern struct address_space_operations ext3_writeback_aops;
 
 /* namei.c */
 extern struct inode_operations ext3_dir_inode_operations;
fs/ext3/namei.c renamed from fs/ext3/namei.c~ext3-writepages +8 -2
@@ -510,7 +510,10 @@ static int ext3_create (struct inode * d
 	if (!IS_ERR(inode)) {
 		inode->i_op = &ext3_file_inode_operations;
 		inode->i_fop = &ext3_file_operations;
-		inode->i_mapping->a_ops = &ext3_aops;
+		if (ext3_should_writeback_data(inode))
+			inode->i_mapping->a_ops = &ext3_writeback_aops;
+		else
+			inode->i_mapping->a_ops = &ext3_aops;
 		ext3_mark_inode_dirty(handle, inode);
 		err = ext3_add_nondir(handle, dentry, inode);
 	}
@@ -985,7 +988,10 @@ static int ext3_symlink (struct inode * 
 
 	if (l > sizeof (EXT3_I(inode)->i_data)) {
 		inode->i_op = &page_symlink_inode_operations;
-		inode->i_mapping->a_ops = &ext3_aops;
+		if (ext3_should_writeback_data(inode))
+			inode->i_mapping->a_ops = &ext3_writeback_aops;
+		else
+			inode->i_mapping->a_ops = &ext3_aops;
 		/*
 		 * page_symlink() calls into ext3_prepare/commit_write.
 		 * We have a transaction open.  All is sweetness.  It also sets

-

#2 lore raw
In reply to Andrew Morton
Quoted 8 lines · Andreas Dilger
Quoted 1 line · Andrew Morton

Turn on direct-to-BIO writeback for ext3 in data=writeback mode.

A minor note on this (especially minor since I believe data=journal doesn't even work in 2.5), but you should probably also change the address ops in ext3/ioctl.c if you enable/disable per-inode data journaling.

hrm. Actually, changing journalling mode against a file while modifications are happening against it is almost certain to explode if the timing is right. ISTR that we have seen bug reports against this on ext3-users. This is just waaaay too hard to do.

But we can fix it by doing the opposite: create three separate a_ops instances, one for each journalling mode. Assign it at new_inode/read_inode time.

This way, we don't have to do the `ext3_should_journal_data()' tests all over the place and we just don't care if someone diddles the journalling mode while the file is otherwise in use.

Another one for my todo list..

-

#3 lore raw
In reply to Andrew Morton
Quoted 9 lines · Andrew Morton
Quoted 7 lines · Andreas Dilger
Quoted 1 line · Andrew Morton

Turn on direct-to-BIO writeback for ext3 in data=writeback mode.

A minor note on this (especially minor since I believe data=journal doesn't even work in 2.5), but you should probably also change the address ops in ext3/ioctl.c if you enable/disable per-inode data journaling.

Quoted 4 lines

hrm. Actually, changing journalling mode against a file while modifications are happening against it is almost certain to explode if the timing is right. ISTR that we have seen bug reports against this on ext3-users. This is just waaaay too hard to do.

Actually, if you look at the code in ioctl.c for changing the journaling mode of a file, it basically stops _all_ I/O to the filesystem and waits for it to complete before changing the journal data flag, so it should also be possible to change the aops pointer at the same time. The "stop all I/O" is one of the reasons why enabling data journaling on files is only allowed for root/privileged users.

Quoted 3 lines

But we can fix it by doing the opposite: create three separate a_ops instances, one for each journalling mode. Assign it at new_inode/read_inode time.

Sure, as long as this doesn't increase the amount of code duplication.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/