[PATCH] Add "-o bh" option to ext3

[PATCH] Add "-o bh" option to ext3

[Badari Pulavarty]

Its not really need for now, but as we try to make "nobh"
as default option, it would be nice to have a "-obh" fallback
option - if things go wrong.

Thanks,
Badari

This patch adds "-o bh" option to force use of buffer_heads.
This option is needed when we make "nobh" as default -
and if we run into problems.

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
Index: linux-2.6.16-rc5/fs/ext3/super.c
===================================================================
--- linux-2.6.16-rc5.orig/fs/ext3/super.c	2006-02-26 21:09:35.000000000
-0800
+++ linux-2.6.16-rc5/fs/ext3/super.c	2006-03-10 10:31:26.000000000 -0800
@@ -628,7 +628,7 @@ enum {
 	Opt_resgid, Opt_resuid, Opt_sb, Opt_err_cont, Opt_err_panic,
Opt_err_ro,
 	Opt_nouid32, 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_nobh,
+	Opt_reservation, Opt_noreservation, Opt_noload, Opt_nobh, Opt_bh,
 	Opt_commit, Opt_journal_update, Opt_journal_inum, Opt_journal_dev,
 	Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
 	Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
@@ -664,6 +664,7 @@ static match_table_t tokens = {
 	{Opt_noreservation, "noreservation"},
 	{Opt_noload, "noload"},
 	{Opt_nobh, "nobh"},
+	{Opt_bh, "bh"},
 	{Opt_commit, "commit=%u"},
 	{Opt_journal_update, "journal=update"},
 	{Opt_journal_inum, "journal=%u"},
@@ -1011,6 +1012,9 @@ clear_qf_name:
 		case Opt_nobh:
 			set_opt(sbi->s_mount_opt, NOBH);
 			break;
+		case Opt_bh:
+			clear_opt(sbi->s_mount_opt, NOBH);
+			break;
 		default:
 			printk (KERN_ERR
 				"EXT3-fs: Unrecognized mount option \"%s\" "

Re: [PATCH] Add "-o bh" option to ext3

[Pavel Machek]

On Pá 10-03-06 10:49:50, Badari Pulavarty wrote:
> Its not really need for now, but as we try to make "nobh"
> as default option, it would be nice to have a "-obh" fallback
> option - if things go wrong.

Docs patch is missing...

...and no, it is not even clear to me what bh vs. nobh does...

							Pavel
-- 
13:            fs = new FileStream( Args[ 0 ], FileMode.Open,

Re: [PATCH] Add "-o bh" option to ext3

[Badari Pulavarty]

On Mon, 2006-03-13 at 10:52 +0100, Pavel Machek wrote:
> On Pá 10-03-06 10:49:50, Badari Pulavarty wrote:
> > Its not really need for now, but as we try to make "nobh"
> > as default option, it would be nice to have a "-obh" fallback
> > option - if things go wrong.
> 
> Docs patch is missing...
> 
> ...and no, it is not even clear to me what bh vs. nobh does...
> 
> 							Pavel

Hope this helps.

Thanks,
Badari

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
Index: linux-2.6.16-rc6/Documentation/filesystems/ext3.txt
===================================================================
--- linux-2.6.16-rc6.orig/Documentation/filesystems/ext3.txt	2006-03-11 14:12:55.000000000 -0800
+++ linux-2.6.16-rc6/Documentation/filesystems/ext3.txt	2006-03-13 16:38:36.000000000 -0800
@@ -113,6 +113,14 @@ noquota
 grpquota
 usrquota
 
+bh		(*)	ext3 associates buffer heads to data pages to
+nobh			(a) cache disk block mapping information
+			(b) link pages into transaction to provide
+			    ordering guarantees.
+			"bh" option forces use of buffer heads.
+			"nobh" option tries to avoid associating buffer
+			heads (supported only for "writeback" mode).
+
 
 Specification
 =============

Re: [PATCH] Add "-o bh" option to ext3

[Pavel Machek]

Hi!

> > > Its not really need for now, but as we try to make "nobh"
> > > as default option, it would be nice to have a "-obh" fallback
> > > option - if things go wrong.
> > 
> > Docs patch is missing...
> > 
> > ...and no, it is not even clear to me what bh vs. nobh does...
> 
> Hope this helps.

Not really, I still am not sure what it does. Is it like "nobh is more
effective code, and should have exactly zero impact to the user, but
as it is new, we make it optional"?

								Pavel

> @@ -113,6 +113,14 @@ noquota
>  grpquota
>  usrquota
>  
> +bh		(*)	ext3 associates buffer heads to data pages to
> +nobh			(a) cache disk block mapping information

missing full stop?

> +			(b) link pages into transaction to provide
> +			    ordering guarantees.
> +			"bh" option forces use of buffer heads.
> +			"nobh" option tries to avoid associating buffer
> +			heads (supported only for "writeback" mode).
> +

-- 
15:        try

Re: [PATCH] Add "-o bh" option to ext3

[Badari Pulavarty]

On Tue, 2006-03-14 at 10:47 +0100, Pavel Machek wrote:
> Hi!
> 
> > > > Its not really need for now, but as we try to make "nobh"
> > > > as default option, it would be nice to have a "-obh" fallback
> > > > option - if things go wrong.
> > > 
> > > Docs patch is missing...
> > > 
> > > ...and no, it is not even clear to me what bh vs. nobh does...
> > 
> > Hope this helps.
> 
> Not really, I still am not sure what it does. Is it like "nobh is more
> effective code, and should have exactly zero impact to the user, but
> as it is new, we make it optional"?

I wish, its that easy to say :)

Historically (2.4 and earlier), buffer_head is the primary structure for
doing IO. We also used it as the interface between VFS, helper functions
and filesystem-specific code to pass physical disk block# information.
We also used them to link buffers/pages/data to JBD transactions to
provide ordering guarantees (for various journal modes).

Now (2.6), we no longer use buffer_head as a primary IO descriptor, 
but we still use it for other reasons. In general, buffer_heads are 
evil - eats up low mem, lots of them floating around, bigger code path,
bigger memory foot print, causes TLB/SLB misses, causes fragmentation
etc..

"nobh" option tries to attaching buffer_head to pages to cache disk
block mapping information. Where ever its needed, it uses temporary
(on stack) buffer_head to pass it to lower-level filesystem-specific
code and uses the disk block# mapping info from it - to create bios.
(BTW, since its also used for transaction ordering - we can't support
"nobh" option for all journaling modes).

Now, "zero impact to user ?" - don't know for sure. Since buffer_head
nicely cache disk block mapping information - we save on calls to
filesystem->get_block() when we need this. With "nobh" option, 
we need to do this every time. Especially on filesystems with 
blocksize < pagesize (1k, 2k) - we may need to multiple calls to 
->get_block() to get all the disk block#s for a single page (4k). 
These calls,  *in theory* could end up doing a disk read. All the
benefits of not having buffer_heads may be worth taking this 
overhead ?  Don't know for sure - thats why this is an "option" 
for now :(

Clear as mud ? :)

Thanks,
Badari