[PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Badari Pulavarty]

[-- Attachment #1: Type: text/plain, Size: 150 bytes --]

Hi Andrew,

Here is the 2.6.11-mm3 version of patch for adding "nobh"
support for ext3 writeback mode.

Can you include it in -mm ?

Thanks,
Badari



[-- Attachment #2: ext3-writeback-nobh.2611mm3.patch --]
[-- Type: text/x-patch, Size: 5231 bytes --]

diff -Narup -X dontdiff linux-2.6.11-mm3-org/fs/ext3/inode.c linux-2.6.11/fs/ext3/inode.c
--- linux-2.6.11-mm3-org/fs/ext3/inode.c	2005-03-14 10:52:16.231806824 -0800
+++ linux-2.6.11/fs/ext3/inode.c	2005-03-14 12:19:33.891561512 -0800
@@ -1016,7 +1016,10 @@ retry:
 		ret = PTR_ERR(handle);
 		goto out;
 	}
-	ret = block_prepare_write(page, from, to, ext3_get_block);
+	if (test_opt(inode->i_sb, NOBH))
+		ret = nobh_prepare_write(page, from, to, ext3_get_block);
+	else
+		ret = block_prepare_write(page, from, to, ext3_get_block);
 	if (ret)
 		goto prepare_write_failed;
 
@@ -1100,7 +1103,12 @@ static int ext3_writeback_commit_write(s
 	new_i_size = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
 	if (new_i_size > EXT3_I(inode)->i_disksize)
 		EXT3_I(inode)->i_disksize = new_i_size;
-	ret = generic_commit_write(file, page, from, to);
+
+	if (test_opt(inode->i_sb, NOBH))
+		ret = nobh_commit_write(file, page, from, to);
+	else
+		ret = generic_commit_write(file, page, from, to);
+
 	ret2 = ext3_journal_stop(handle);
 	if (!ret)
 		ret = ret2;
@@ -1385,7 +1393,11 @@ static int ext3_writeback_writepage(stru
 		goto out_fail;
 	}
 
-	ret = block_write_full_page(page, ext3_get_block, wbc);
+	if (test_opt(inode->i_sb, NOBH))
+		ret = nobh_writepage(page, ext3_get_block, wbc);
+	else
+		ret = block_write_full_page(page, ext3_get_block, wbc);
+
 	err = ext3_journal_stop(handle);
 	if (!ret)
 		ret = err;
@@ -1484,6 +1496,8 @@ static int ext3_releasepage(struct page 
 	journal_t *journal = EXT3_JOURNAL(page->mapping->host);
 
 	WARN_ON(PageChecked(page));
+	if (!page_has_buffers(page))	
+		return 0;
 	return journal_try_to_free_buffers(journal, page, wait);
 }
 
@@ -1646,13 +1660,28 @@ static int ext3_block_truncate_page(hand
 	unsigned blocksize, iblock, length, pos;
 	struct inode *inode = mapping->host;
 	struct buffer_head *bh;
-	int err;
+	int err = 0;
 	void *kaddr;
 
 	blocksize = inode->i_sb->s_blocksize;
 	length = blocksize - (offset & (blocksize - 1));
 	iblock = index << (PAGE_CACHE_SHIFT - inode->i_sb->s_blocksize_bits);
 
+	/*
+	 * For "nobh" option,  we can only work if we don't need to
+	 * read-in the page - otherwise we create buffers to do the IO.
+	 */
+	if (!page_has_buffers(page) && test_opt(inode->i_sb, NOBH)) {
+		if (PageUptodate(page)) {
+			kaddr = kmap_atomic(page, KM_USER0);
+			memset(kaddr + offset, 0, length);
+			flush_dcache_page(page);
+			kunmap_atomic(kaddr, KM_USER0);
+			set_page_dirty(page);
+			goto unlock;
+		}
+	}
+
 	if (!page_has_buffers(page))
 		create_empty_buffers(page, blocksize, 0);
 
diff -Narup -X dontdiff linux-2.6.11-mm3-org/fs/ext3/super.c linux-2.6.11/fs/ext3/super.c
--- linux-2.6.11-mm3-org/fs/ext3/super.c	2005-03-01 23:38:38.000000000 -0800
+++ linux-2.6.11/fs/ext3/super.c	2005-03-14 12:18:09.734355352 -0800
@@ -576,7 +576,7 @@ enum {
 	Opt_resgid, Opt_resuid, Opt_sb, Opt_err_cont, Opt_err_panic, Opt_err_ro,
 	Opt_nouid32, Opt_check, Opt_nocheck, Opt_debug, Opt_oldalloc, Opt_orlov,
 	Opt_user_xattr, Opt_nouser_xattr, Opt_acl, Opt_noacl,
-	Opt_reservation, Opt_noreservation, Opt_noload,
+	Opt_reservation, Opt_noreservation, Opt_noload, Opt_nobh,
 	Opt_commit, Opt_journal_update, Opt_journal_inum,
 	Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
 	Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
@@ -611,6 +611,7 @@ static match_table_t tokens = {
 	{Opt_reservation, "reservation"},
 	{Opt_noreservation, "noreservation"},
 	{Opt_noload, "noload"},
+	{Opt_nobh, "nobh"},
 	{Opt_commit, "commit=%u"},
 	{Opt_journal_update, "journal=update"},
 	{Opt_journal_inum, "journal=%u"},
@@ -924,6 +925,9 @@ clear_qf_name:
 			match_int(&args[0], &option);
 			*n_blocks_count = option;
 			break;
+		case Opt_nobh:
+			set_opt(sbi->s_mount_opt, NOBH);
+			break;
 		default:
 			printk (KERN_ERR
 				"EXT3-fs: Unrecognized mount option \"%s\" "
@@ -1563,6 +1567,19 @@ static int ext3_fill_super (struct super
 		break;
 	}
 
+	if (test_opt(sb, NOBH)) {
+		if (sb->s_blocksize_bits != PAGE_CACHE_SHIFT) {
+			printk(KERN_WARNING "EXT3-fs: Ignoring nobh option "
+				"since filesystem blocksize doesn't match "
+				"pagesize\n");
+			clear_opt(sbi->s_mount_opt, NOBH);
+		}
+		if (!(test_opt(sb, DATA_FLAGS) == EXT3_MOUNT_WRITEBACK_DATA)) {
+			printk(KERN_WARNING "EXT3-fs: Ignoring nobh option - "
+				"its supported only with writeback mode\n");
+			clear_opt(sbi->s_mount_opt, NOBH);
+		}
+	}
 	/*
 	 * The journal_load will have done any necessary log recovery,
 	 * so we can safely mount the rest of the filesystem now.
diff -Narup -X dontdiff linux-2.6.11-mm3-org/include/linux/ext3_fs.h linux-2.6.11/include/linux/ext3_fs.h
--- linux-2.6.11-mm3-org/include/linux/ext3_fs.h	2005-03-01 23:38:10.000000000 -0800
+++ linux-2.6.11/include/linux/ext3_fs.h	2005-03-14 12:18:09.762351096 -0800
@@ -357,6 +357,7 @@ struct ext3_inode {
 #define EXT3_MOUNT_POSIX_ACL		0x08000	/* POSIX Access Control Lists */
 #define EXT3_MOUNT_RESERVATION		0x10000	/* Preallocation */
 #define EXT3_MOUNT_BARRIER		0x20000 /* Use block barriers */
+#define EXT3_MOUNT_NOBH			0x40000 /* No bufferheads */
 
 /* Compatibility, for having both ext2_fs.h and ext3_fs.h included at once */
 #ifndef _LINUX_EXT2_FS_H

Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Arjan van de Ven]

On Mon, 2005-03-14 at 11:18 -0800, Badari Pulavarty wrote:
> Hi Andrew,
> 
> Here is the 2.6.11-mm3 version of patch for adding "nobh"
> support for ext3 writeback mode.

can you explain why this is an option ? It's not like the on disk layout
changes or something... is there a reason to ever not want this?

Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Badari Pulavarty]

On Mon, 2005-03-14 at 11:29, Arjan van de Ven wrote:
> On Mon, 2005-03-14 at 11:18 -0800, Badari Pulavarty wrote:
> > Hi Andrew,
> > 
> > Here is the 2.6.11-mm3 version of patch for adding "nobh"
> > support for ext3 writeback mode.
> 
> can you explain why this is an option ? It's not like the on disk layout
> changes or something... is there a reason to ever not want this?

I am slowly trying to reduce the uses of "bufferhead"s in the ext3.
We can get away not attaching a bufferheads for the pages in
ext3 writeback mode easily. But for ordered mode, its doable
but tricky.

There are few cases, I didn't handle in my patch (just to reduce
the code complexity)
	
	- I still create bufferheads for filesystem 
	blocksize != PAGE_SIZE

	- In case of a truncate and the page is not uptodate
	(needs to do IO to read the page) - I attach buffers.
	I had code to eliminate this - but Andrew didn't like it :(


I want to get more run-time before making it a default.

Thanks,
Badari

Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Andrew Morton]

Badari Pulavarty <pbadari@us.ibm.com> wrote:
>
> Here is the 2.6.11-mm3 version of patch for adding "nobh"
>  support for ext3 writeback mode.

Care to update Documentation/filesystems/ext3.txt?

>  Can you include it in -mm ?

Spose so.

Did you have performance and resource consumption numbers to justify it?  I
think I asked that before and promptly forgot the answer, which is a good
reason for taking some care over changelog maintenance...

Re: [Ext2-devel] Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Badari Pulavarty]

On Mon, 2005-03-14 at 18:09, Andrew Morton wrote:
> Badari Pulavarty <pbadari@us.ibm.com> wrote:
> >
> > Here is the 2.6.11-mm3 version of patch for adding "nobh"
> >  support for ext3 writeback mode.
> 
> Care to update Documentation/filesystems/ext3.txt?

Yes. I will do that. I am planning to add "nobh" support to
ext3 ordered mode also, since its the default one. We need
to modify generic interfaces like mpage_writepage(s) to
keep track of bio count and make journal code wait for them etc. -
at that point the "generic" code will no longer be generic.
I am thinking of a way to do it *less* intrusively. 

At that point, we can make "nobh" default option. (which
needs less documentation).

> 
> >  Can you include it in -mm ?
> 
> Spose so.
> 
> Did you have performance and resource consumption numbers to justify it?  I
> think I asked that before and promptly forgot the answer, which is a good
> reason for taking some care over changelog maintenance...

The initial numbers showed 5-7% throughput improvements. I will send out
resource consumption numbers once I complete my regression tests.

Yep. I will update the changelog also.


Thanks,
Badari

Re: [Ext2-devel] Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Arjan van de Ven]

On Tue, 2005-03-15 at 08:09 -0800, Badari Pulavarty wrote:
> On Mon, 2005-03-14 at 18:09, Andrew Morton wrote:
> > Badari Pulavarty <pbadari@us.ibm.com> wrote:
> > >
> > > Here is the 2.6.11-mm3 version of patch for adding "nobh"
> > >  support for ext3 writeback mode.
> > 
> > Care to update Documentation/filesystems/ext3.txt?
> 
> Yes. I will do that. I am planning to add "nobh" support to
> ext3 ordered mode also, since its the default one. We need
> to modify generic interfaces like mpage_writepage(s) to
> keep track of bio count and make journal code wait for them etc. -
> at that point the "generic" code will no longer be generic.
> I am thinking of a way to do it *less* intrusively. 
> 
> At that point, we can make "nobh" default option. (which
> needs less documentation).

I still don't get why you want a mount option. Sure during development
it can be nice.. but do you still want it in the production trees??
(I understand that for small blocksizes you need to fallback code, fine,
no problem, but why do you want to make it an *option* instead of
automatic)

Re: [Ext2-devel] Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Badari Pulavarty]

On Tue, 2005-03-15 at 08:26, Arjan van de Ven wrote:
> On Tue, 2005-03-15 at 08:09 -0800, Badari Pulavarty wrote:
> > On Mon, 2005-03-14 at 18:09, Andrew Morton wrote:
> > > Badari Pulavarty <pbadari@us.ibm.com> wrote:
> > > >
> > > > Here is the 2.6.11-mm3 version of patch for adding "nobh"
> > > >  support for ext3 writeback mode.
> > > 
> > > Care to update Documentation/filesystems/ext3.txt?
> > 
> > Yes. I will do that. I am planning to add "nobh" support to
> > ext3 ordered mode also, since its the default one. We need
> > to modify generic interfaces like mpage_writepage(s) to
> > keep track of bio count and make journal code wait for them etc. -
> > at that point the "generic" code will no longer be generic.
> > I am thinking of a way to do it *less* intrusively. 
> > 
> > At that point, we can make "nobh" default option. (which
> > needs less documentation).
> 
> I still don't get why you want a mount option. Sure during development
> it can be nice.. but do you still want it in the production trees??

Once I get "nobh" working for both ordered and writeback mode - 
I will take out the option. Only reason why, you may
want "bh"s are for faster lookups. "bh" stores the get_block()
information, getting rid of it means - we need to do few more
get_block() calls when we need the disk mapping. 

We have seen small amount of "reads" when we are doing write-only 
tests with "nobh" option. I am not at a point, where I can quantify
the performance hit due to not caching the disk mapping info. 

Thanks,
Badari

Re: [Ext2-devel] Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Matthew Wilcox]

On Tue, Mar 15, 2005 at 05:26:36PM +0100, Arjan van de Ven wrote:
> I still don't get why you want a mount option. Sure during development
> it can be nice.. but do you still want it in the production trees??
> (I understand that for small blocksizes you need to fallback code, fine,
> no problem, but why do you want to make it an *option* instead of
> automatic)

Can we get rid of the "nobh" ext2 mount option too so other people don't
get mislead into thinking this is a good idea?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [Ext2-devel] Re: [PATCH] 2.6.11-mm3 patch for ext3 writeback "nobh" option

[Andrew Morton]

Matthew Wilcox <matthew@wil.cx> wrote:
>
> Can we get rid of the "nobh" ext2 mount option too so other people don't
>  get mislead into thinking this is a good idea?

What's wrong with ext2 -o nobh??