public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  6:39 ` Andrew Morton
@ 2006-08-10  9:29   ` Alex Tomas
  2006-08-10  9:48     ` Andrew Morton
                       ` (2 more replies)
  2006-08-10 17:17   ` Theodore Tso
  1 sibling, 3 replies; 26+ messages in thread
From: Alex Tomas @ 2006-08-10  9:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: cmm, linux-fsdevel, ext2-devel, linux-kernel

>>>>> Andrew Morton (AM) writes:

 >> From a quick scan:

 AM> - The code is very poorly commented.  I'd want to spend a lot of time
 AM>   reviewing this implementation, but not in its present state.  

what sort of comments are you expecting?

 AM> - Far, far too many inlines

probably, I'll review the code in this regard

 AM> - overly-terse variable naming

same

 AM> - There are several places which appear to be putting block numbers into
 AM>   an `int'.

same

 AM> - Needs kmalloc()->kzalloc() conversion

OK

 AM> - replace all brelse() calls with put_bh().  Because brelse() is
 AM>   old-fashioned, has a weird name and neelessly permits a NULL arg.

 AM>   In fact it would be beter to convert JBD and ext3 to put_bh before
 AM>   copying it all over.

OK

 AM> - The open-coded __clear_bit(BH_New, ...) in ext4_ext_get_blocks is a bit
 AM>   nasty.  We can live with nasty, but are we sure that it isn't buggy??

I believe it isn't buggy -- it applies to non-shared var.
it also showed minor improvement on SMP.

 AM> - It has about 7,000 instances of

 AM> 	if ((lhs = expression)) {

 AM>   whereas the preferred coding style is

 AM> 	lhs = expression;
 AM> 	if (lhs) {

OK

 AM> - The existing comments could benefit from some rework by a native English
 AM>   speaker.

could someone assist here, please?

thanks, Alex

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  9:29   ` [Ext2-devel] " Alex Tomas
@ 2006-08-10  9:48     ` Andrew Morton
  2006-08-10 10:08       ` Alex Tomas
  2006-08-10 15:55       ` Zach Brown
  2006-08-10 17:49     ` Randy.Dunlap
  2006-08-11 20:57     ` Randy.Dunlap
  2 siblings, 2 replies; 26+ messages in thread
From: Andrew Morton @ 2006-08-10  9:48 UTC (permalink / raw)
  To: Alex Tomas; +Cc: cmm, linux-fsdevel, ext2-devel, linux-kernel

On Thu, 10 Aug 2006 13:29:56 +0400
Alex Tomas <alex@clusterfs.com> wrote:

>  AM> - The code is very poorly commented.  I'd want to spend a lot of time
>  AM>   reviewing this implementation, but not in its present state.  
> 
> what sort of comments are you expecting?

Ones which tell me what the code is attempting to do.  Ones which tell me
the things which I need to know and which I cannot determine from the
implementation within a reasonable period of time.  Ones which tell me
about the hidden design decisions, the known shortcomings, the
things-still-to-do.

It's a bit of an artform, really.  I guess one needs to put oneself in the
position of the reader, then work out what the reader wants to know.

Good examples don't immediately leap to mind, I'm afraid.  Maybe some of
fs/buffer.c?  That's important and pretty tricky code in there, so it goes
to some lengths.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  9:48     ` Andrew Morton
@ 2006-08-10 10:08       ` Alex Tomas
  2006-08-10 15:55       ` Zach Brown
  1 sibling, 0 replies; 26+ messages in thread
From: Alex Tomas @ 2006-08-10 10:08 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alex Tomas, cmm, linux-fsdevel, ext2-devel, linux-kernel


just to make things clear ... I thought the code isn't that bad commented. I may be wrong, of course. but could you have a look at few routines
(ext3_ext_create_new_leaf() or ext3_ext_get_blocks() fo example)
and tell me what's wrong with existing comments (besides monkey english)
and how it should look like?

thanks, Alex

>>>>> Andrew Morton (AM) writes:

 AM> On Thu, 10 Aug 2006 13:29:56 +0400
 AM> Alex Tomas <alex@clusterfs.com> wrote:

 AM> - The code is very poorly commented.  I'd want to spend a lot of time
 AM> reviewing this implementation, but not in its present state.  
 >> 
 >> what sort of comments are you expecting?

 AM> Ones which tell me what the code is attempting to do.  Ones which tell me
 AM> the things which I need to know and which I cannot determine from the
 AM> implementation within a reasonable period of time.  Ones which tell me
 AM> about the hidden design decisions, the known shortcomings, the
 AM> things-still-to-do.

 AM> It's a bit of an artform, really.  I guess one needs to put oneself in the
 AM> position of the reader, then work out what the reader wants to know.

 AM> Good examples don't immediately leap to mind, I'm afraid.  Maybe some of
 AM> fs/buffer.c?  That's important and pretty tricky code in there, so it goes
 AM> to some lengths.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  9:48     ` Andrew Morton
  2006-08-10 10:08       ` Alex Tomas
@ 2006-08-10 15:55       ` Zach Brown
  1 sibling, 0 replies; 26+ messages in thread
From: Zach Brown @ 2006-08-10 15:55 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alex Tomas, cmm, linux-fsdevel, ext2-devel, linux-kernel


> Good examples don't immediately leap to mind, I'm afraid.  Maybe some of
> fs/buffer.c?  That's important and pretty tricky code in there, so it goes
> to some lengths.

fs/direct-io.c?  It has some fantastic commentary.  (Just please don't
also take inspiration from its bug/line ratio :)).

- z

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  6:39 ` Andrew Morton
  2006-08-10  9:29   ` [Ext2-devel] " Alex Tomas
@ 2006-08-10 17:17   ` Theodore Tso
  2006-08-10 18:00     ` Andrew Morton
  1 sibling, 1 reply; 26+ messages in thread
From: Theodore Tso @ 2006-08-10 17:17 UTC (permalink / raw)
  To: Andrew Morton; +Cc: cmm, linux-fsdevel, ext2-devel, linux-kernel

On Wed, Aug 09, 2006 at 11:39:40PM -0700, Andrew Morton wrote:
> - replace all brelse() calls with put_bh().  Because brelse() is
>   old-fashioned, has a weird name and neelessly permits a NULL arg.
> 
>   In fact it would be beter to convert JBD and ext3 to put_bh before
>   copying it all over.

Wouldn't it be better to preserve in the source code history the
brelse->put_bh conversion?  We can pour a huge number of changes in
ext4 before we submit, but I would have thought it would be easier for
everyone to see what is going on if we submit with just the minimal
changes, and then have patches that address concerns like this one at
a time.

						- Ted

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  9:29   ` [Ext2-devel] " Alex Tomas
  2006-08-10  9:48     ` Andrew Morton
@ 2006-08-10 17:49     ` Randy.Dunlap
  2006-08-10 19:05       ` Alex Tomas
  2006-08-11 20:57     ` Randy.Dunlap
  2 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-08-10 17:49 UTC (permalink / raw)
  To: Alex Tomas; +Cc: Andrew Morton, cmm, linux-fsdevel, ext2-devel, linux-kernel

On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:

> >>>>> Andrew Morton (AM) writes:
> 
>  >> From a quick scan:
> 
>  AM> - The code is very poorly commented.  I'd want to spend a lot of time
>  AM>   reviewing this implementation, but not in its present state.  
> 
> what sort of comments are you expecting?

Helpful ones.  Not obvious stuff.  Intents.
Tricks used (if they are the right thing to do).

How, what, why.  But not nitty-gritty details of how.
"Why" is often more important.

>  AM> - The existing comments could benefit from some rework by a native English
>  AM>   speaker.
> 
> could someone assist here, please?

Yes.  How would you like it?  Just comments via email or (quilt) patches?
Which files/patches?

---
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10 17:17   ` Theodore Tso
@ 2006-08-10 18:00     ` Andrew Morton
  2006-08-11 22:13       ` Mingming Cao
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Morton @ 2006-08-10 18:00 UTC (permalink / raw)
  To: Theodore Tso; +Cc: cmm, linux-fsdevel, ext2-devel, linux-kernel

On Thu, 10 Aug 2006 13:17:55 -0400
Theodore Tso <tytso@mit.edu> wrote:

> On Wed, Aug 09, 2006 at 11:39:40PM -0700, Andrew Morton wrote:
> > - replace all brelse() calls with put_bh().  Because brelse() is
> >   old-fashioned, has a weird name and neelessly permits a NULL arg.
> > 
> >   In fact it would be beter to convert JBD and ext3 to put_bh before
> >   copying it all over.
> 
> Wouldn't it be better to preserve in the source code history the
> brelse->put_bh conversion?  We can pour a huge number of changes in
> ext4 before we submit, but I would have thought it would be easier for
> everyone to see what is going on if we submit with just the minimal
> changes, and then have patches that address concerns like this one at
> a time.
> 

I'd suggest that this be one of the cleanups which be done within ext3
before taking the ext4 copy.

That's assuming we want to do the spring-cleaning - we might of course
decide not to.  But it'd be a good time to do so.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10 17:49     ` Randy.Dunlap
@ 2006-08-10 19:05       ` Alex Tomas
  0 siblings, 0 replies; 26+ messages in thread
From: Alex Tomas @ 2006-08-10 19:05 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Alex Tomas, Andrew Morton, cmm, linux-fsdevel, ext2-devel,
	linux-kernel

>>>>> Randy Dunlap (RD) writes:

 RD> On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:
 >> >>>>> Andrew Morton (AM) writes:
 >> 
 >> >> From a quick scan:
 >> 
 AM> - The code is very poorly commented.  I'd want to spend a lot of time
 AM> reviewing this implementation, but not in its present state.  
 >> 
 >> what sort of comments are you expecting?

 RD> Helpful ones.  Not obvious stuff.  Intents.
 RD> Tricks used (if they are the right thing to do).

 RD> How, what, why.  But not nitty-gritty details of how.
 RD> "Why" is often more important.

well, it's a simple b+tree and i'm not sure there are ticks in.
I'll try to re-read them again WRT what you wrote.


 AM> - The existing comments could benefit from some rework by a native English
 AM> speaker.
 >> 
 >> could someone assist here, please?

 RD> Yes.  How would you like it?  Just comments via email or (quilt) patches?
 RD> Which files/patches?

please, have a look at ext4-extents.patch first

thanks, Alex

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10  9:29   ` [Ext2-devel] " Alex Tomas
  2006-08-10  9:48     ` Andrew Morton
  2006-08-10 17:49     ` Randy.Dunlap
@ 2006-08-11 20:57     ` Randy.Dunlap
  2006-08-11 21:05       ` Alex Tomas
                         ` (3 more replies)
  2 siblings, 4 replies; 26+ messages in thread
From: Randy.Dunlap @ 2006-08-11 20:57 UTC (permalink / raw)
  To: Alex Tomas; +Cc: Andrew Morton, cmm, linux-fsdevel, ext2-devel, linux-kernel

On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:

>  AM> - The existing comments could benefit from some rework by a
>  AM> native English speaker.
> 
> could someone assist here, please?

See if this helps.
Patch applies on top of all ext4 patches from
http://ext2.sourceforge.net/48bitext3/patches/latest/.

---
From: Randy Dunlap <rdunlap@xenotime.net>

Clean up comments in ext4-extents patch.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 fs/ext4/extents.c               |  226 ++++++++++++++++++++++------------------
 include/linux/ext4_fs_extents.h |   54 ++++-----
 include/linux/ext4_jbd2.h       |    4 
 3 files changed, 157 insertions(+), 127 deletions(-)

--- linux-2618-rc4-ext4.orig/include/linux/ext4_jbd2.h
+++ linux-2618-rc4-ext4/include/linux/ext4_jbd2.h
@@ -28,8 +28,8 @@
  * indirection blocks, the group and superblock summaries, and the data
  * block to complete the transaction.
  *
- * For extents-enabled fs we may have to allocate and modify upto
- * 5 levels of tree + root which is stored in inode. */
+ * For extents-enabled fs we may have to allocate and modify up to
+ * 5 levels of tree + root which are stored in the inode. */
 
 #define EXT4_SINGLEDATA_TRANS_BLOCKS(sb)				\
 	(EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_EXTENTS)	\
--- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_extents.h
+++ linux-2618-rc4-ext4/include/linux/ext4_fs_extents.h
@@ -22,29 +22,29 @@
 #include <linux/ext4_fs.h>
 
 /*
- * with AGRESSIVE_TEST defined capacity of index/leaf blocks
- * become very little, so index split, in-depth growing and
- * other hard changes happens much more often
- * this is for debug purposes only
+ * With AGRESSIVE_TEST defined, the capacity of index/leaf blocks
+ * becomes very small, so index split, in-depth growing and
+ * other hard changes happen much more often.
+ * This is for debug purposes only.
  */
 #define AGRESSIVE_TEST_
 
 /*
- * with EXTENTS_STATS defined number of blocks and extents
- * are collected in truncate path. they'll be showed at
- * umount time
+ * With EXTENTS_STATS defined, the number of blocks and extents
+ * are collected in the truncate path. They'll be shown at
+ * umount time.
  */
 #define EXTENTS_STATS__
 
 /*
- * if CHECK_BINSEARCH defined, then results of binary search
- * will be checked by linear search
+ * If CHECK_BINSEARCH is defined, then the results of the binary search
+ * will also be checked by linear search.
  */
 #define CHECK_BINSEARCH__
 
 /*
- * if EXT_DEBUG is defined you can use 'extdebug' mount option
- * to get lots of info what's going on
+ * If EXT_DEBUG is defined you can use the 'extdebug' mount option
+ * to get lots of info about what's going on.
  */
 #define EXT_DEBUG__
 #ifdef EXT_DEBUG
@@ -54,58 +54,58 @@
 #endif
 
 /*
- * if EXT_STATS is defined then stats numbers are collected
- * these number will be displayed at umount time
+ * If EXT_STATS is defined then stats numbers are collected.
+ * These number will be displayed at umount time.
  */
 #define EXT_STATS_
 
 
 /*
- * ext4_inode has i_block array (60 bytes total)
- * first 12 bytes store ext4_extent_header
- * the remain stores array of ext4_extent
+ * ext4_inode has i_block array (60 bytes total).
+ * The first 12 bytes store ext4_extent_header;
+ * the remainder stores an array of ext4_extent.
  */
 
 /*
- * this is extent on-disk structure
- * it's used at the bottom of the tree
+ * This is the extent on-disk structure.
+ * It's used at the bottom of the tree.
  */
 struct ext4_extent {
 	__le32	ee_block;	/* first logical block extent covers */
 	__le16	ee_len;		/* number of blocks covered by extent */
 	__le16	ee_start_hi;	/* high 16 bits of physical block */
-	__le32	ee_start;	/* low 32 bigs of physical block */
+	__le32	ee_start;	/* low 32 bits of physical block */
 };
 
 /*
- * this is index on-disk structure
- * it's used at all the levels, but the bottom
+ * This is index on-disk structure.
+ * It's used at all the levels except the bottom.
  */
 struct ext4_extent_idx {
 	__le32	ei_block;	/* index covers logical blocks from 'block' */
 	__le32	ei_leaf;	/* pointer to the physical block of the next *
-				 * level. leaf or next index could bet here */
+				 * level. leaf or next index could be there */
 	__le16	ei_leaf_hi;	/* high 16 bits of physical block */
 	__u16	ei_unused;
 };
 
 /*
- * each block (leaves and indexes), even inode-stored has header
+ * Each block (leaves and indexes), even inode-stored has header.
  */
 struct ext4_extent_header {
 	__le16	eh_magic;	/* probably will support different formats */
 	__le16	eh_entries;	/* number of valid entries */
 	__le16	eh_max;		/* capacity of store in entries */
-	__le16	eh_depth;	/* has tree real underlaying blocks? */
+	__le16	eh_depth;	/* has tree real underlying blocks? */
 	__le32	eh_generation;	/* generation of the tree */
 };
 
 #define EXT4_EXT_MAGIC		cpu_to_le16(0xf30a)
 
 /*
- * array of ext4_ext_path contains path to some extent
- * creation/lookup routines use it for traversal/splitting/etc
- * truncate uses it to simulate recursive walking
+ * Array of ext4_ext_path contains path to some extent.
+ * Creation/lookup routines use it for traversal/splitting/etc.
+ * Truncate uses it to simulate recursive walking.
  */
 struct ext4_ext_path {
 	ext4_fsblk_t			p_block;
--- linux-2618-rc4-ext4.orig/fs/ext4/extents.c
+++ linux-2618-rc4-ext4/fs/ext4/extents.c
@@ -44,7 +44,10 @@
 #include <asm/uaccess.h>
 
 
-/* this macro combines low and hi parts of phys. blocknr into ext4_fsblk_t */
+/*
+ * ext_pblock:
+ * combine low and high parts of physical block number into ext4_fsblk_t
+ */
 static inline ext4_fsblk_t ext_pblock(struct ext4_extent *ex)
 {
 	ext4_fsblk_t block;
@@ -55,7 +58,10 @@ static inline ext4_fsblk_t ext_pblock(st
 	return block;
 }
 
-/* this macro combines low and hi parts of phys. blocknr into ext4_fsblk_t */
+/*
+ * idx_pblock:
+ * combine low and high parts of a leaf physical block number into ext4_fsblk_t
+ */
 static inline ext4_fsblk_t idx_pblock(struct ext4_extent_idx *ix)
 {
 	ext4_fsblk_t block;
@@ -66,7 +72,11 @@ static inline ext4_fsblk_t idx_pblock(st
 	return block;
 }
 
-/* the routine stores large phys. blocknr into extent breaking it into parts */
+/*
+ * ext4_ext_store_pblock:
+ * stores a large physical block number into an extent struct,
+ * breaking it into parts
+ */
 static inline void ext4_ext_store_pblock(struct ext4_extent *ex, ext4_fsblk_t pb)
 {
 	ex->ee_start = cpu_to_le32((unsigned long) (pb & 0xffffffff));
@@ -74,7 +84,11 @@ static inline void ext4_ext_store_pblock
 		ex->ee_start_hi = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
 }
 
-/* the routine stores large phys. blocknr into index breaking it into parts */
+/*
+ * ext4_idx_store_pblock:
+ * stores a large physical block number into an index struct,
+ * breaking it into parts
+ */
 static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix, ext4_fsblk_t pb)
 {
 	ix->ei_leaf = cpu_to_le32((unsigned long) (pb & 0xffffffff));
@@ -179,8 +193,8 @@ static ext4_fsblk_t ext4_ext_find_goal(s
 		if ((ex = path[depth].p_ext))
 			return ext_pblock(ex)+(block-le32_to_cpu(ex->ee_block));
 
-		/* it looks index is empty
-		 * try to find starting from index itself */
+		/* it looks like index is empty;
+		 * try to find starting block from index itself */
 		if (path[depth].p_bh)
 			return path[depth].p_bh->b_blocknr;
 	}
@@ -317,7 +331,8 @@ static void ext4_ext_drop_refs(struct ex
 }
 
 /*
- * binary search for closest index by given block
+ * ext4_ext_binsearch_idx:
+ * binary search for the closest index of the given block
  */
 static void
 ext4_ext_binsearch_idx(struct inode *inode, struct ext4_ext_path *path, int block)
@@ -375,7 +390,8 @@ ext4_ext_binsearch_idx(struct inode *ino
 }
 
 /*
- * binary search for closest extent by given block
+ * ext4_ext_binsearch:
+ * binary search for closest extent of the given block
  */
 static void
 ext4_ext_binsearch(struct inode *inode, struct ext4_ext_path *path, int block)
@@ -388,8 +404,8 @@ ext4_ext_binsearch(struct inode *inode, 
 
 	if (eh->eh_entries == 0) {
 		/*
-		 * this leaf is empty yet:
-		 *  we get such a leaf in split/add case
+		 * this leaf is empty:
+		 * we get such a leaf in split/add case
 		 */
 		return;
 	}
@@ -520,8 +536,9 @@ err:
 }
 
 /*
- * insert new index [logical;ptr] into the block at cupr
- * it check where to insert: before curp or after curp
+ * ext4_ext_insert_index:
+ * insert new index [@logical;@ptr] into the block at @curp;
+ * check where to insert: before @curp or after @curp
  */
 static int ext4_ext_insert_index(handle_t *handle, struct inode *inode,
 				struct ext4_ext_path *curp,
@@ -574,13 +591,14 @@ static int ext4_ext_insert_index(handle_
 }
 
 /*
- * routine inserts new subtree into the path, using free index entry
- * at depth 'at:
- *  - allocates all needed blocks (new leaf and all intermediate index blocks)
- *  - makes decision where to split
- *  - moves remaining extens and index entries (right to the split point)
- *    into the newly allocated blocks
- *  - initialize subtree
+ * ext4_ext_split:
+ * inserts new subtree into the path, using free index entry
+ * at depth @at:
+ * - allocates all needed blocks (new leaf and all intermediate index blocks)
+ * - makes decision where to split
+ * - moves remaining extents and index entries (right to the split point)
+ *   into the newly allocated blocks
+ * - initializes subtree
  */
 static int ext4_ext_split(handle_t *handle, struct inode *inode,
 				struct ext4_ext_path *path,
@@ -598,14 +616,14 @@ static int ext4_ext_split(handle_t *hand
 	int err = 0;
 
 	/* make decision: where to split? */
-	/* FIXME: now desicion is simplest: at current extent */
+	/* FIXME: now decision is simplest: at current extent */
 
-	/* if current leaf will be splitted, then we should use
+	/* if current leaf will be split, then we should use
 	 * border from split point */
 	BUG_ON(path[depth].p_ext > EXT_MAX_EXTENT(path[depth].p_hdr));
 	if (path[depth].p_ext != EXT_MAX_EXTENT(path[depth].p_hdr)) {
 		border = path[depth].p_ext[1].ee_block;
-		ext_debug("leaf will be splitted."
+		ext_debug("leaf will be split."
 				" next leaf starts at %d\n",
 			          le32_to_cpu(border));
 	} else {
@@ -616,16 +634,16 @@ static int ext4_ext_split(handle_t *hand
 	}
 
 	/*
-	 * if error occurs, then we break processing
-	 * and turn filesystem read-only. so, index won't
+	 * If error occurs, then we break processing
+	 * and mark filesystem read-only. index won't
 	 * be inserted and tree will be in consistent
-	 * state. next mount will repair buffers too
+	 * state. Next mount will repair buffers too.
 	 */
 
 	/*
-	 * get array to track all allocated blocks
-	 * we need this to handle errors and free blocks
-	 * upon them
+	 * Get array to track all allocated blocks.
+	 * We need this to handle errors and free blocks
+	 * upon them.
 	 */
 	ablocks = kmalloc(sizeof(ext4_fsblk_t) * depth, GFP_NOFS);
 	if (!ablocks)
@@ -661,7 +679,7 @@ static int ext4_ext_split(handle_t *hand
 	neh->eh_depth = 0;
 	ex = EXT_FIRST_EXTENT(neh);
 
-	/* move remain of path[depth] to the new leaf */
+	/* move remainder of path[depth] to the new leaf */
 	BUG_ON(path[depth].p_hdr->eh_entries != path[depth].p_hdr->eh_max);
 	/* start copy from next extent */
 	/* TODO: we could do it by single memmove */
@@ -813,11 +831,12 @@ cleanup:
 }
 
 /*
- * routine implements tree growing procedure:
- *  - allocates new block
- *  - moves top-level data (index block or leaf) into the new block
- *  - initialize new top-level, creating index that points to the
- *    just created block
+ * ext4_ext_grow_indepth:
+ * implements tree growing procedure:
+ * - allocates new block
+ * - moves top-level data (index block or leaf) into the new block
+ * - initializes new top-level, creating index that points to the
+ *   just created block
  */
 static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
 					struct ext4_ext_path *path,
@@ -892,8 +911,9 @@ out:
 }
 
 /*
- * routine finds empty index and adds new leaf. if no free index found
- * then it requests in-depth growing
+ * ext4_ext_create_new_leaf:
+ * finds empty index and adds new leaf.
+ * if no free index is found, then it requests in-depth growing.
  */
 static int ext4_ext_create_new_leaf(handle_t *handle, struct inode *inode,
 					struct ext4_ext_path *path,
@@ -912,8 +932,8 @@ repeat:
 		curp--;
 	}
 
-	/* we use already allocated block for index block
-	 * so, subsequent data blocks should be contigoues */
+	/* we use already allocated block for index block,
+	 * so subsequent data blocks should be contiguous */
 	if (EXT_HAS_FREE_INDEX(curp)) {
 		/* if we found index with free entry, then use that
 		 * entry: create all needed subtree and add new leaf */
@@ -943,12 +963,12 @@ repeat:
 		}
 
 		/*
-		 * only first (depth 0 -> 1) produces free space
-		 * in all other cases we have to split growed tree
+		 * only first (depth 0 -> 1) produces free space;
+		 * in all other cases we have to split the grown tree
 		 */
 		depth = ext_depth(inode);
 		if (path[depth].p_hdr->eh_entries == path[depth].p_hdr->eh_max) {
-			/* now we need split */
+			/* now we need to split */
 			goto repeat;
 		}
 	}
@@ -958,10 +978,11 @@ out:
 }
 
 /*
- * returns allocated block in subsequent extent or EXT_MAX_BLOCK
- * NOTE: it consider block number from index entry as
- * allocated block. thus, index entries have to be consistent
- * with leafs
+ * ext4_ext_next_allocated_block:
+ * returns allocated block in subsequent extent or EXT_MAX_BLOCK.
+ * NOTE: it considers block number from index entry as
+ * allocated block. Thus, index entries have to be consistent
+ * with leaves.
  */
 static unsigned long
 ext4_ext_next_allocated_block(struct ext4_ext_path *path)
@@ -993,6 +1014,7 @@ ext4_ext_next_allocated_block(struct ext
 }
 
 /*
+ * ext4_ext_next_leaf_block:
  * returns first allocated block from next leaf or EXT_MAX_BLOCK
  */
 static unsigned ext4_ext_next_leaf_block(struct inode *inode,
@@ -1021,8 +1043,9 @@ static unsigned ext4_ext_next_leaf_block
 }
 
 /*
- * if leaf gets modified and modified extent is first in the leaf
- * then we have to correct all indexes above
+ * ext4_ext_correct_indexes:
+ * if leaf gets modified and modified extent is first in the leaf,
+ * then we have to correct all indexes above.
  * TODO: do we need to correct tree in all cases?
  */
 int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
@@ -1050,7 +1073,7 @@ int ext4_ext_correct_indexes(handle_t *h
 	}
 
 	/*
-	 * TODO: we need correction if border is smaller then current one
+	 * TODO: we need correction if border is smaller than current one
 	 */
 	k = depth - 1;
 	border = path[depth].p_ext->ee_block;
@@ -1085,7 +1108,7 @@ ext4_can_extents_be_merged(struct inode 
 	/*
 	 * To allow future support for preallocated extents to be added
 	 * as an RO_COMPAT feature, refuse to merge to extents if
-	 * can result in the top bit of ee_len being set
+	 * this can result in the top bit of ee_len being set.
 	 */
 	if (le16_to_cpu(ex1->ee_len) + le16_to_cpu(ex2->ee_len) > EXT_MAX_LEN)
 		return 0;
@@ -1100,9 +1123,10 @@ ext4_can_extents_be_merged(struct inode 
 }
 
 /*
- * this routine tries to merge requsted extent into the existing
- * extent or inserts requested extent as new one into the tree,
- * creating new leaf in no-space case
+ * ext4_ext_insert_extent:
+ * tries to merge requsted extent into the existing extent or
+ * inserts requested extent as new one into the tree,
+ * creating new leaf in the no-space case.
  */
 int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
 				struct ext4_ext_path *path,
@@ -1163,8 +1187,8 @@ repeat:
 	}
 
 	/*
-	 * there is no free space in found leaf
-	 * we're gonna add new leaf in the tree
+	 * There is no free space in the found leaf.
+	 * We're gonna add a new leaf in the tree.
 	 */
 	err = ext4_ext_create_new_leaf(handle, inode, path, newext);
 	if (err)
@@ -1377,7 +1401,8 @@ ext4_ext_put_in_cache(struct inode *inod
 }
 
 /*
- * this routine calculate boundaries of the gap requested block fits into
+ * ext4_ext_put_gap_in_cache:
+ * calculate boundaries of the gap that the requested block fits into
  * and cache this gap
  */
 static inline void
@@ -1452,9 +1477,10 @@ ext4_ext_in_cache(struct inode *inode, u
 }
 
 /*
- * routine removes index from the index block
- * it's used in truncate case only. thus all requests are for
- * last index in the block only
+ * ext4_ext_rm_idx:
+ * removes index from the index block.
+ * It's used in truncate case only, thus all requests are for
+ * last index in the block only.
  */
 int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
 			struct ext4_ext_path *path)
@@ -1480,11 +1506,12 @@ int ext4_ext_rm_idx(handle_t *handle, st
 }
 
 /*
- * This routine returns max. credits extent tree can consume.
+ * ext4_ext_calc_credits_for_insert:
+ * This routine returns max. credits that the extent tree can consume.
  * It should be OK for low-performance paths like ->writepage()
- * To allow many writing process to fit a single transaction,
- * caller should calculate credits under truncate_mutex and
- * pass actual path.
+ * To allow many writing processes to fit into a single transaction,
+ * the caller should calculate credits under truncate_mutex and
+ * pass the actual path.
  */
 int inline ext4_ext_calc_credits_for_insert(struct inode *inode,
 						struct ext4_ext_path *path)
@@ -1500,9 +1527,9 @@ int inline ext4_ext_calc_credits_for_ins
 	}
 
 	/*
-	 * given 32bit logical block (4294967296 blocks), max. tree
+	 * given 32-bit logical block (4294967296 blocks), max. tree
 	 * can be 4 levels in depth -- 4 * 340^4 == 53453440000.
-	 * let's also add one more level for imbalance.
+	 * Let's also add one more level for imbalance.
 	 */
 	depth = 5;
 
@@ -1510,13 +1537,13 @@ int inline ext4_ext_calc_credits_for_ins
 	needed = 2;
 
 	/*
-	 * tree can be full, so it'd need to grow in depth:
+	 * tree can be full, so it would need to grow in depth:
 	 * allocation + old root + new root
 	 */
 	needed += 2 + 1 + 1;
 
 	/*
-	 * Index split can happen, we'd need:
+	 * Index split can happen, we would need:
 	 *    allocate intermediate indexes (bitmap + group)
 	 *  + change two blocks at each level, but root (already included)
 	 */
@@ -1634,7 +1661,7 @@ ext4_ext_rm_leaf(handle_t *handle, struc
 			BUG_ON(b != ex_ee_block + ex_ee_len - 1);
 		}
 
-		/* at present, extent can't cross block group */
+		/* at present, extent can't cross block group: */
 		/* leaf + bitmap + group desc + sb + inode */
 		credits = 5;
 		if (ex == EXT_FIRST_EXTENT(eh)) {
@@ -1660,7 +1687,7 @@ ext4_ext_rm_leaf(handle_t *handle, struc
 			goto out;
 
 		if (num == 0) {
-			/* this extent is removed entirely mark slot unused */
+			/* this extent is removed; mark slot entirely unused */
 			ext4_ext_store_pblock(ex, 0);
 			eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)-1);
 		}
@@ -1692,7 +1719,8 @@ out:
 }
 
 /*
- * returns 1 if current index have to be freed (even partial)
+ * ext4_ext_more_to_rm:
+ * returns 1 if current index has to be freed (even partial)
  */
 static int inline
 ext4_ext_more_to_rm(struct ext4_ext_path *path)
@@ -1703,7 +1731,7 @@ ext4_ext_more_to_rm(struct ext4_ext_path
 		return 0;
 
 	/*
-	 * if truncate on deeper level happened it it wasn't partial
+	 * if truncate on deeper level happened, it wasn't partial,
 	 * so we have to consider current index for truncation
 	 */
 	if (le16_to_cpu(path->p_hdr->eh_entries) == path->p_block)
@@ -1729,8 +1757,8 @@ int ext4_ext_remove_space(struct inode *
 	ext4_ext_invalidate_cache(inode);
 
 	/*
-	 * we start scanning from right side freeing all the blocks
-	 * after i_size and walking into the deep
+	 * We start scanning from right side, freeing all the blocks
+	 * after i_size and walking into the tree depth-wise.
 	 */
 	path = kmalloc(sizeof(struct ext4_ext_path) * (depth + 1), GFP_KERNEL);
 	if (path == NULL) {
@@ -1749,7 +1777,7 @@ int ext4_ext_remove_space(struct inode *
 		if (i == depth) {
 			/* this is leaf block */
 			err = ext4_ext_rm_leaf(handle, inode, path, start);
-			/* root level have p_bh == NULL, brelse() eats this */
+			/* root level has p_bh == NULL, brelse() eats this */
 			brelse(path[i].p_bh);
 			path[i].p_bh = NULL;
 			i--;
@@ -1772,14 +1800,14 @@ int ext4_ext_remove_space(struct inode *
 		BUG_ON(path[i].p_hdr->eh_magic != EXT4_EXT_MAGIC);
 
 		if (!path[i].p_idx) {
-			/* this level hasn't touched yet */
+			/* this level hasn't been touched yet */
 			path[i].p_idx = EXT_LAST_INDEX(path[i].p_hdr);
 			path[i].p_block = le16_to_cpu(path[i].p_hdr->eh_entries)+1;
 			ext_debug("init index ptr: hdr 0x%p, num %d\n",
 				  path[i].p_hdr,
 				  le16_to_cpu(path[i].p_hdr->eh_entries));
 		} else {
-			/* we've already was here, see at next index */
+			/* we were already here, see at next index */
 			path[i].p_idx--;
 		}
 
@@ -1799,19 +1827,19 @@ int ext4_ext_remove_space(struct inode *
 				break;
 			}
 
-			/* put actual number of indexes to know is this
-			 * number got changed at the next iteration */
+			/* save actual number of indexes since this
+			 * number is changed at the next iteration */
 			path[i].p_block = le16_to_cpu(path[i].p_hdr->eh_entries);
 			i++;
 		} else {
-			/* we finish processing this index, go up */
+			/* we finished processing this index, go up */
 			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
-				/* index is empty, remove it
+				/* index is empty, remove it;
 				 * handle must be already prepared by the
 				 * truncatei_leaf() */
 				err = ext4_ext_rm_idx(handle, inode, path + i);
 			}
-			/* root level have p_bh == NULL, brelse() eats this */
+			/* root level has p_bh == NULL, brelse() eats this */
 			brelse(path[i].p_bh);
 			path[i].p_bh = NULL;
 			i--;
@@ -1822,8 +1850,8 @@ int ext4_ext_remove_space(struct inode *
 	/* TODO: flexible tree reduction should be here */
 	if (path->p_hdr->eh_entries == 0) {
 		/*
-		 * truncate to zero freed all the tree
-		 * so, we need to correct eh_depth
+		 * truncate to zero freed all the tree,
+		 * so we need to correct eh_depth
 		 */
 		err = ext4_ext_get_access(handle, inode, path);
 		if (err == 0) {
@@ -1911,7 +1939,7 @@ int ext4_ext_get_blocks(handle_t *handle
 		if (goal == EXT4_EXT_CACHE_GAP) {
 			if (!create) {
 				/* block isn't allocated yet and
-				 * user don't want to allocate it */
+				 * user doesn't want to allocate it */
 				goto out2;
 			}
 			/* we should allocate requested block */
@@ -1920,7 +1948,7 @@ int ext4_ext_get_blocks(handle_t *handle
 		        newblock = iblock
 		                   - le32_to_cpu(newex.ee_block)
 			           + ext_pblock(&newex);
-			/* number of remain blocks in the extent */
+			/* number of remaining blocks in the extent */
 			allocated = le16_to_cpu(newex.ee_len) -
 					(iblock - le32_to_cpu(newex.ee_block));
 			goto out;
@@ -1940,8 +1968,8 @@ int ext4_ext_get_blocks(handle_t *handle
 	depth = ext_depth(inode);
 
 	/*
-	 * consistent leaf must not be empty
-	 * this situations is possible, though, _during_ tree modification
+	 * consistent leaf must not be empty;
+	 * this situation is possible, though, _during_ tree modification;
 	 * this is why assert can't be put in ext4_ext_find_extent()
 	 */
 	BUG_ON(path[depth].p_ext == NULL && depth != 0);
@@ -1959,10 +1987,10 @@ int ext4_ext_get_blocks(handle_t *handle
 		 */
 		if (ee_len > EXT_MAX_LEN)
 			goto out2;
-		/* if found exent covers block, simple return it */
+		/* if found extent covers block, simply return it */
 	        if (iblock >= ee_block && iblock < ee_block + ee_len) {
 			newblock = iblock - ee_block + ee_start;
-			/* number of remain blocks in the extent */
+			/* number of remaining blocks in the extent */
 			allocated = ee_len - (iblock - ee_block);
 			ext_debug("%d fit into %lu:%d -> "E3FSBLK"\n", (int) iblock,
 					ee_block, ee_len, newblock);
@@ -1973,17 +2001,18 @@ int ext4_ext_get_blocks(handle_t *handle
 	}
 
 	/*
-	 * requested block isn't allocated yet
+	 * requested block isn't allocated yet;
 	 * we couldn't try to create block if create flag is zero
 	 */
 	if (!create) {
-		/* put just found gap into cache to speedup subsequest reqs */
+		/* put just found gap into cache to speed up
+		 * subsequent requests */
 		ext4_ext_put_gap_in_cache(inode, path, iblock);
 		goto out2;
 	}
 	/*
          * Okay, we need to do block allocation.  Lazily initialize the block
-         * allocation info here if necessary
+         * allocation info here if necessary.
         */
 	if (S_ISREG(inode->i_mode) && (!EXT4_I(inode)->i_block_alloc_info))
 		ext4_init_block_alloc_info(inode);
@@ -2061,9 +2090,9 @@ void ext4_ext_truncate(struct inode * in
 	ext4_ext_invalidate_cache(inode);
 
 	/*
-	 * TODO: optimization is possible here
-	 * probably we need not scaning at all,
-	 * because page truncation is enough
+	 * TODO: optimization is possible here.
+	 * Probably we need not scan at all,
+	 * because page truncation is enough.
 	 */
 	if (ext4_orphan_add(handle, inode))
 		goto out_stop;
@@ -2077,13 +2106,13 @@ void ext4_ext_truncate(struct inode * in
 	err = ext4_ext_remove_space(inode, last_block);
 
 	/* In a multi-transaction truncate, we only make the final
-	 * transaction synchronous */
+	 * transaction synchronous. */
 	if (IS_SYNC(inode))
 		handle->h_sync = 1;
 
 out_stop:
 	/*
-	 * If this was a simple ftruncate(), and the file will remain alive
+	 * If this was a simple ftruncate() and the file will remain alive,
 	 * then we need to clear up the orphan record which we created above.
 	 * However, if this was a real unlink then we were called by
 	 * ext4_delete_inode(), and we allow that function to clean up the
@@ -2097,7 +2126,8 @@ out_stop:
 }
 
 /*
- * this routine calculate max number of blocks we could modify
+ * ext4_ext_writepage_trans_blocks:
+ * calculate max number of blocks we could modify
  * in order to allocate new block for an inode
  */
 int ext4_ext_writepage_trans_blocks(struct inode *inode, int num)
@@ -2106,7 +2136,7 @@ int ext4_ext_writepage_trans_blocks(stru
 
 	needed = ext4_ext_calc_credits_for_insert(inode, NULL);
 
-	/* caller want to allocate num blocks, but note it includes sb */
+	/* caller wants to allocate num blocks, but note it includes sb */
 	needed = needed * num - (num - 1);
 
 #ifdef CONFIG_QUOTA

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-11 20:57     ` Randy.Dunlap
@ 2006-08-11 21:05       ` Alex Tomas
  2006-08-11 21:49       ` Mingming Cao
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Alex Tomas @ 2006-08-11 21:05 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Alex Tomas, Andrew Morton, cmm, linux-fsdevel, ext2-devel,
	linux-kernel

thanks a lot Randy

> On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:
> See if this helps.
> Patch applies on top of all ext4 patches from
> http://ext2.sourceforge.net/48bitext3/patches/latest/.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-11 20:57     ` Randy.Dunlap
  2006-08-11 21:05       ` Alex Tomas
@ 2006-08-11 21:49       ` Mingming Cao
  2006-08-11 23:00       ` Andrew Morton
  2006-08-15 15:40       ` Pavel Machek
  3 siblings, 0 replies; 26+ messages in thread
From: Mingming Cao @ 2006-08-11 21:49 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Alex Tomas, Andrew Morton, linux-fsdevel, ext2-devel,
	linux-kernel

On Fri, 2006-08-11 at 13:57 -0700, Randy.Dunlap wrote:
> On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:
> 
> >  AM> - The existing comments could benefit from some rework by a
> >  AM> native English speaker.
> > 
> > could someone assist here, please?
> 
> See if this helps.
> Patch applies on top of all ext4 patches from
> http://ext2.sourceforge.net/48bitext3/patches/latest/.
> 
> ---
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Clean up comments in ext4-extents patch.
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>

Thanks, Randy, I added this to the queue.

> ---
>  fs/ext4/extents.c               |  226 ++++++++++++++++++++++------------------
>  include/linux/ext4_fs_extents.h |   54 ++++-----
>  include/linux/ext4_jbd2.h       |    4 
>  3 files changed, 157 insertions(+), 127 deletions(-)
> 
> --- linux-2618-rc4-ext4.orig/include/linux/ext4_jbd2.h
> +++ linux-2618-rc4-ext4/include/linux/ext4_jbd2.h
> @@ -28,8 +28,8 @@
>   * indirection blocks, the group and superblock summaries, and the data
>   * block to complete the transaction.
>   *
> - * For extents-enabled fs we may have to allocate and modify upto
> - * 5 levels of tree + root which is stored in inode. */
> + * For extents-enabled fs we may have to allocate and modify up to
> + * 5 levels of tree + root which are stored in the inode. */
>  
>  #define EXT4_SINGLEDATA_TRANS_BLOCKS(sb)				\
>  	(EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_EXTENTS)	\
> --- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_extents.h
> +++ linux-2618-rc4-ext4/include/linux/ext4_fs_extents.h
> @@ -22,29 +22,29 @@
>  #include <linux/ext4_fs.h>
>  
>  /*
> - * with AGRESSIVE_TEST defined capacity of index/leaf blocks
> - * become very little, so index split, in-depth growing and
> - * other hard changes happens much more often
> - * this is for debug purposes only
> + * With AGRESSIVE_TEST defined, the capacity of index/leaf blocks
> + * becomes very small, so index split, in-depth growing and
> + * other hard changes happen much more often.
> + * This is for debug purposes only.
>   */
>  #define AGRESSIVE_TEST_
>  
>  /*
> - * with EXTENTS_STATS defined number of blocks and extents
> - * are collected in truncate path. they'll be showed at
> - * umount time
> + * With EXTENTS_STATS defined, the number of blocks and extents
> + * are collected in the truncate path. They'll be shown at
> + * umount time.
>   */
>  #define EXTENTS_STATS__
>  
>  /*
> - * if CHECK_BINSEARCH defined, then results of binary search
> - * will be checked by linear search
> + * If CHECK_BINSEARCH is defined, then the results of the binary search
> + * will also be checked by linear search.
>   */
>  #define CHECK_BINSEARCH__
>  
>  /*
> - * if EXT_DEBUG is defined you can use 'extdebug' mount option
> - * to get lots of info what's going on
> + * If EXT_DEBUG is defined you can use the 'extdebug' mount option
> + * to get lots of info about what's going on.
>   */
>  #define EXT_DEBUG__
>  #ifdef EXT_DEBUG
> @@ -54,58 +54,58 @@
>  #endif
>  
>  /*
> - * if EXT_STATS is defined then stats numbers are collected
> - * these number will be displayed at umount time
> + * If EXT_STATS is defined then stats numbers are collected.
> + * These number will be displayed at umount time.
>   */
>  #define EXT_STATS_
>  
> 
>  /*
> - * ext4_inode has i_block array (60 bytes total)
> - * first 12 bytes store ext4_extent_header
> - * the remain stores array of ext4_extent
> + * ext4_inode has i_block array (60 bytes total).
> + * The first 12 bytes store ext4_extent_header;
> + * the remainder stores an array of ext4_extent.
>   */
>  
>  /*
> - * this is extent on-disk structure
> - * it's used at the bottom of the tree
> + * This is the extent on-disk structure.
> + * It's used at the bottom of the tree.
>   */
>  struct ext4_extent {
>  	__le32	ee_block;	/* first logical block extent covers */
>  	__le16	ee_len;		/* number of blocks covered by extent */
>  	__le16	ee_start_hi;	/* high 16 bits of physical block */
> -	__le32	ee_start;	/* low 32 bigs of physical block */
> +	__le32	ee_start;	/* low 32 bits of physical block */
>  };
>  
>  /*
> - * this is index on-disk structure
> - * it's used at all the levels, but the bottom
> + * This is index on-disk structure.
> + * It's used at all the levels except the bottom.
>   */
>  struct ext4_extent_idx {
>  	__le32	ei_block;	/* index covers logical blocks from 'block' */
>  	__le32	ei_leaf;	/* pointer to the physical block of the next *
> -				 * level. leaf or next index could bet here */
> +				 * level. leaf or next index could be there */
>  	__le16	ei_leaf_hi;	/* high 16 bits of physical block */
>  	__u16	ei_unused;
>  };
>  
>  /*
> - * each block (leaves and indexes), even inode-stored has header
> + * Each block (leaves and indexes), even inode-stored has header.
>   */
>  struct ext4_extent_header {
>  	__le16	eh_magic;	/* probably will support different formats */
>  	__le16	eh_entries;	/* number of valid entries */
>  	__le16	eh_max;		/* capacity of store in entries */
> -	__le16	eh_depth;	/* has tree real underlaying blocks? */
> +	__le16	eh_depth;	/* has tree real underlying blocks? */
>  	__le32	eh_generation;	/* generation of the tree */
>  };
>  
>  #define EXT4_EXT_MAGIC		cpu_to_le16(0xf30a)
>  
>  /*
> - * array of ext4_ext_path contains path to some extent
> - * creation/lookup routines use it for traversal/splitting/etc
> - * truncate uses it to simulate recursive walking
> + * Array of ext4_ext_path contains path to some extent.
> + * Creation/lookup routines use it for traversal/splitting/etc.
> + * Truncate uses it to simulate recursive walking.
>   */
>  struct ext4_ext_path {
>  	ext4_fsblk_t			p_block;
> --- linux-2618-rc4-ext4.orig/fs/ext4/extents.c
> +++ linux-2618-rc4-ext4/fs/ext4/extents.c
> @@ -44,7 +44,10 @@
>  #include <asm/uaccess.h>
>  
> 
> -/* this macro combines low and hi parts of phys. blocknr into ext4_fsblk_t */
> +/*
> + * ext_pblock:
> + * combine low and high parts of physical block number into ext4_fsblk_t
> + */
>  static inline ext4_fsblk_t ext_pblock(struct ext4_extent *ex)
>  {
>  	ext4_fsblk_t block;
> @@ -55,7 +58,10 @@ static inline ext4_fsblk_t ext_pblock(st
>  	return block;
>  }
>  
> -/* this macro combines low and hi parts of phys. blocknr into ext4_fsblk_t */
> +/*
> + * idx_pblock:
> + * combine low and high parts of a leaf physical block number into ext4_fsblk_t
> + */
>  static inline ext4_fsblk_t idx_pblock(struct ext4_extent_idx *ix)
>  {
>  	ext4_fsblk_t block;
> @@ -66,7 +72,11 @@ static inline ext4_fsblk_t idx_pblock(st
>  	return block;
>  }
>  
> -/* the routine stores large phys. blocknr into extent breaking it into parts */
> +/*
> + * ext4_ext_store_pblock:
> + * stores a large physical block number into an extent struct,
> + * breaking it into parts
> + */
>  static inline void ext4_ext_store_pblock(struct ext4_extent *ex, ext4_fsblk_t pb)
>  {
>  	ex->ee_start = cpu_to_le32((unsigned long) (pb & 0xffffffff));
> @@ -74,7 +84,11 @@ static inline void ext4_ext_store_pblock
>  		ex->ee_start_hi = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
>  }
>  
> -/* the routine stores large phys. blocknr into index breaking it into parts */
> +/*
> + * ext4_idx_store_pblock:
> + * stores a large physical block number into an index struct,
> + * breaking it into parts
> + */
>  static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix, ext4_fsblk_t pb)
>  {
>  	ix->ei_leaf = cpu_to_le32((unsigned long) (pb & 0xffffffff));
> @@ -179,8 +193,8 @@ static ext4_fsblk_t ext4_ext_find_goal(s
>  		if ((ex = path[depth].p_ext))
>  			return ext_pblock(ex)+(block-le32_to_cpu(ex->ee_block));
>  
> -		/* it looks index is empty
> -		 * try to find starting from index itself */
> +		/* it looks like index is empty;
> +		 * try to find starting block from index itself */
>  		if (path[depth].p_bh)
>  			return path[depth].p_bh->b_blocknr;
>  	}
> @@ -317,7 +331,8 @@ static void ext4_ext_drop_refs(struct ex
>  }
>  
>  /*
> - * binary search for closest index by given block
> + * ext4_ext_binsearch_idx:
> + * binary search for the closest index of the given block
>   */
>  static void
>  ext4_ext_binsearch_idx(struct inode *inode, struct ext4_ext_path *path, int block)
> @@ -375,7 +390,8 @@ ext4_ext_binsearch_idx(struct inode *ino
>  }
>  
>  /*
> - * binary search for closest extent by given block
> + * ext4_ext_binsearch:
> + * binary search for closest extent of the given block
>   */
>  static void
>  ext4_ext_binsearch(struct inode *inode, struct ext4_ext_path *path, int block)
> @@ -388,8 +404,8 @@ ext4_ext_binsearch(struct inode *inode, 
>  
>  	if (eh->eh_entries == 0) {
>  		/*
> -		 * this leaf is empty yet:
> -		 *  we get such a leaf in split/add case
> +		 * this leaf is empty:
> +		 * we get such a leaf in split/add case
>  		 */
>  		return;
>  	}
> @@ -520,8 +536,9 @@ err:
>  }
>  
>  /*
> - * insert new index [logical;ptr] into the block at cupr
> - * it check where to insert: before curp or after curp
> + * ext4_ext_insert_index:
> + * insert new index [@logical;@ptr] into the block at @curp;
> + * check where to insert: before @curp or after @curp
>   */
>  static int ext4_ext_insert_index(handle_t *handle, struct inode *inode,
>  				struct ext4_ext_path *curp,
> @@ -574,13 +591,14 @@ static int ext4_ext_insert_index(handle_
>  }
>  
>  /*
> - * routine inserts new subtree into the path, using free index entry
> - * at depth 'at:
> - *  - allocates all needed blocks (new leaf and all intermediate index blocks)
> - *  - makes decision where to split
> - *  - moves remaining extens and index entries (right to the split point)
> - *    into the newly allocated blocks
> - *  - initialize subtree
> + * ext4_ext_split:
> + * inserts new subtree into the path, using free index entry
> + * at depth @at:
> + * - allocates all needed blocks (new leaf and all intermediate index blocks)
> + * - makes decision where to split
> + * - moves remaining extents and index entries (right to the split point)
> + *   into the newly allocated blocks
> + * - initializes subtree
>   */
>  static int ext4_ext_split(handle_t *handle, struct inode *inode,
>  				struct ext4_ext_path *path,
> @@ -598,14 +616,14 @@ static int ext4_ext_split(handle_t *hand
>  	int err = 0;
>  
>  	/* make decision: where to split? */
> -	/* FIXME: now desicion is simplest: at current extent */
> +	/* FIXME: now decision is simplest: at current extent */
>  
> -	/* if current leaf will be splitted, then we should use
> +	/* if current leaf will be split, then we should use
>  	 * border from split point */
>  	BUG_ON(path[depth].p_ext > EXT_MAX_EXTENT(path[depth].p_hdr));
>  	if (path[depth].p_ext != EXT_MAX_EXTENT(path[depth].p_hdr)) {
>  		border = path[depth].p_ext[1].ee_block;
> -		ext_debug("leaf will be splitted."
> +		ext_debug("leaf will be split."
>  				" next leaf starts at %d\n",
>  			          le32_to_cpu(border));
>  	} else {
> @@ -616,16 +634,16 @@ static int ext4_ext_split(handle_t *hand
>  	}
>  
>  	/*
> -	 * if error occurs, then we break processing
> -	 * and turn filesystem read-only. so, index won't
> +	 * If error occurs, then we break processing
> +	 * and mark filesystem read-only. index won't
>  	 * be inserted and tree will be in consistent
> -	 * state. next mount will repair buffers too
> +	 * state. Next mount will repair buffers too.
>  	 */
>  
>  	/*
> -	 * get array to track all allocated blocks
> -	 * we need this to handle errors and free blocks
> -	 * upon them
> +	 * Get array to track all allocated blocks.
> +	 * We need this to handle errors and free blocks
> +	 * upon them.
>  	 */
>  	ablocks = kmalloc(sizeof(ext4_fsblk_t) * depth, GFP_NOFS);
>  	if (!ablocks)
> @@ -661,7 +679,7 @@ static int ext4_ext_split(handle_t *hand
>  	neh->eh_depth = 0;
>  	ex = EXT_FIRST_EXTENT(neh);
>  
> -	/* move remain of path[depth] to the new leaf */
> +	/* move remainder of path[depth] to the new leaf */
>  	BUG_ON(path[depth].p_hdr->eh_entries != path[depth].p_hdr->eh_max);
>  	/* start copy from next extent */
>  	/* TODO: we could do it by single memmove */
> @@ -813,11 +831,12 @@ cleanup:
>  }
>  
>  /*
> - * routine implements tree growing procedure:
> - *  - allocates new block
> - *  - moves top-level data (index block or leaf) into the new block
> - *  - initialize new top-level, creating index that points to the
> - *    just created block
> + * ext4_ext_grow_indepth:
> + * implements tree growing procedure:
> + * - allocates new block
> + * - moves top-level data (index block or leaf) into the new block
> + * - initializes new top-level, creating index that points to the
> + *   just created block
>   */
>  static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
>  					struct ext4_ext_path *path,
> @@ -892,8 +911,9 @@ out:
>  }
>  
>  /*
> - * routine finds empty index and adds new leaf. if no free index found
> - * then it requests in-depth growing
> + * ext4_ext_create_new_leaf:
> + * finds empty index and adds new leaf.
> + * if no free index is found, then it requests in-depth growing.
>   */
>  static int ext4_ext_create_new_leaf(handle_t *handle, struct inode *inode,
>  					struct ext4_ext_path *path,
> @@ -912,8 +932,8 @@ repeat:
>  		curp--;
>  	}
>  
> -	/* we use already allocated block for index block
> -	 * so, subsequent data blocks should be contigoues */
> +	/* we use already allocated block for index block,
> +	 * so subsequent data blocks should be contiguous */
>  	if (EXT_HAS_FREE_INDEX(curp)) {
>  		/* if we found index with free entry, then use that
>  		 * entry: create all needed subtree and add new leaf */
> @@ -943,12 +963,12 @@ repeat:
>  		}
>  
>  		/*
> -		 * only first (depth 0 -> 1) produces free space
> -		 * in all other cases we have to split growed tree
> +		 * only first (depth 0 -> 1) produces free space;
> +		 * in all other cases we have to split the grown tree
>  		 */
>  		depth = ext_depth(inode);
>  		if (path[depth].p_hdr->eh_entries == path[depth].p_hdr->eh_max) {
> -			/* now we need split */
> +			/* now we need to split */
>  			goto repeat;
>  		}
>  	}
> @@ -958,10 +978,11 @@ out:
>  }
>  
>  /*
> - * returns allocated block in subsequent extent or EXT_MAX_BLOCK
> - * NOTE: it consider block number from index entry as
> - * allocated block. thus, index entries have to be consistent
> - * with leafs
> + * ext4_ext_next_allocated_block:
> + * returns allocated block in subsequent extent or EXT_MAX_BLOCK.
> + * NOTE: it considers block number from index entry as
> + * allocated block. Thus, index entries have to be consistent
> + * with leaves.
>   */
>  static unsigned long
>  ext4_ext_next_allocated_block(struct ext4_ext_path *path)
> @@ -993,6 +1014,7 @@ ext4_ext_next_allocated_block(struct ext
>  }
>  
>  /*
> + * ext4_ext_next_leaf_block:
>   * returns first allocated block from next leaf or EXT_MAX_BLOCK
>   */
>  static unsigned ext4_ext_next_leaf_block(struct inode *inode,
> @@ -1021,8 +1043,9 @@ static unsigned ext4_ext_next_leaf_block
>  }
>  
>  /*
> - * if leaf gets modified and modified extent is first in the leaf
> - * then we have to correct all indexes above
> + * ext4_ext_correct_indexes:
> + * if leaf gets modified and modified extent is first in the leaf,
> + * then we have to correct all indexes above.
>   * TODO: do we need to correct tree in all cases?
>   */
>  int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
> @@ -1050,7 +1073,7 @@ int ext4_ext_correct_indexes(handle_t *h
>  	}
>  
>  	/*
> -	 * TODO: we need correction if border is smaller then current one
> +	 * TODO: we need correction if border is smaller than current one
>  	 */
>  	k = depth - 1;
>  	border = path[depth].p_ext->ee_block;
> @@ -1085,7 +1108,7 @@ ext4_can_extents_be_merged(struct inode 
>  	/*
>  	 * To allow future support for preallocated extents to be added
>  	 * as an RO_COMPAT feature, refuse to merge to extents if
> -	 * can result in the top bit of ee_len being set
> +	 * this can result in the top bit of ee_len being set.
>  	 */
>  	if (le16_to_cpu(ex1->ee_len) + le16_to_cpu(ex2->ee_len) > EXT_MAX_LEN)
>  		return 0;
> @@ -1100,9 +1123,10 @@ ext4_can_extents_be_merged(struct inode 
>  }
>  
>  /*
> - * this routine tries to merge requsted extent into the existing
> - * extent or inserts requested extent as new one into the tree,
> - * creating new leaf in no-space case
> + * ext4_ext_insert_extent:
> + * tries to merge requsted extent into the existing extent or
> + * inserts requested extent as new one into the tree,
> + * creating new leaf in the no-space case.
>   */
>  int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
>  				struct ext4_ext_path *path,
> @@ -1163,8 +1187,8 @@ repeat:
>  	}
>  
>  	/*
> -	 * there is no free space in found leaf
> -	 * we're gonna add new leaf in the tree
> +	 * There is no free space in the found leaf.
> +	 * We're gonna add a new leaf in the tree.
>  	 */
>  	err = ext4_ext_create_new_leaf(handle, inode, path, newext);
>  	if (err)
> @@ -1377,7 +1401,8 @@ ext4_ext_put_in_cache(struct inode *inod
>  }
>  
>  /*
> - * this routine calculate boundaries of the gap requested block fits into
> + * ext4_ext_put_gap_in_cache:
> + * calculate boundaries of the gap that the requested block fits into
>   * and cache this gap
>   */
>  static inline void
> @@ -1452,9 +1477,10 @@ ext4_ext_in_cache(struct inode *inode, u
>  }
>  
>  /*
> - * routine removes index from the index block
> - * it's used in truncate case only. thus all requests are for
> - * last index in the block only
> + * ext4_ext_rm_idx:
> + * removes index from the index block.
> + * It's used in truncate case only, thus all requests are for
> + * last index in the block only.
>   */
>  int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
>  			struct ext4_ext_path *path)
> @@ -1480,11 +1506,12 @@ int ext4_ext_rm_idx(handle_t *handle, st
>  }
>  
>  /*
> - * This routine returns max. credits extent tree can consume.
> + * ext4_ext_calc_credits_for_insert:
> + * This routine returns max. credits that the extent tree can consume.
>   * It should be OK for low-performance paths like ->writepage()
> - * To allow many writing process to fit a single transaction,
> - * caller should calculate credits under truncate_mutex and
> - * pass actual path.
> + * To allow many writing processes to fit into a single transaction,
> + * the caller should calculate credits under truncate_mutex and
> + * pass the actual path.
>   */
>  int inline ext4_ext_calc_credits_for_insert(struct inode *inode,
>  						struct ext4_ext_path *path)
> @@ -1500,9 +1527,9 @@ int inline ext4_ext_calc_credits_for_ins
>  	}
>  
>  	/*
> -	 * given 32bit logical block (4294967296 blocks), max. tree
> +	 * given 32-bit logical block (4294967296 blocks), max. tree
>  	 * can be 4 levels in depth -- 4 * 340^4 == 53453440000.
> -	 * let's also add one more level for imbalance.
> +	 * Let's also add one more level for imbalance.
>  	 */
>  	depth = 5;
>  
> @@ -1510,13 +1537,13 @@ int inline ext4_ext_calc_credits_for_ins
>  	needed = 2;
>  
>  	/*
> -	 * tree can be full, so it'd need to grow in depth:
> +	 * tree can be full, so it would need to grow in depth:
>  	 * allocation + old root + new root
>  	 */
>  	needed += 2 + 1 + 1;
>  
>  	/*
> -	 * Index split can happen, we'd need:
> +	 * Index split can happen, we would need:
>  	 *    allocate intermediate indexes (bitmap + group)
>  	 *  + change two blocks at each level, but root (already included)
>  	 */
> @@ -1634,7 +1661,7 @@ ext4_ext_rm_leaf(handle_t *handle, struc
>  			BUG_ON(b != ex_ee_block + ex_ee_len - 1);
>  		}
>  
> -		/* at present, extent can't cross block group */
> +		/* at present, extent can't cross block group: */
>  		/* leaf + bitmap + group desc + sb + inode */
>  		credits = 5;
>  		if (ex == EXT_FIRST_EXTENT(eh)) {
> @@ -1660,7 +1687,7 @@ ext4_ext_rm_leaf(handle_t *handle, struc
>  			goto out;
>  
>  		if (num == 0) {
> -			/* this extent is removed entirely mark slot unused */
> +			/* this extent is removed; mark slot entirely unused */
>  			ext4_ext_store_pblock(ex, 0);
>  			eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)-1);
>  		}
> @@ -1692,7 +1719,8 @@ out:
>  }
>  
>  /*
> - * returns 1 if current index have to be freed (even partial)
> + * ext4_ext_more_to_rm:
> + * returns 1 if current index has to be freed (even partial)
>   */
>  static int inline
>  ext4_ext_more_to_rm(struct ext4_ext_path *path)
> @@ -1703,7 +1731,7 @@ ext4_ext_more_to_rm(struct ext4_ext_path
>  		return 0;
>  
>  	/*
> -	 * if truncate on deeper level happened it it wasn't partial
> +	 * if truncate on deeper level happened, it wasn't partial,
>  	 * so we have to consider current index for truncation
>  	 */
>  	if (le16_to_cpu(path->p_hdr->eh_entries) == path->p_block)
> @@ -1729,8 +1757,8 @@ int ext4_ext_remove_space(struct inode *
>  	ext4_ext_invalidate_cache(inode);
>  
>  	/*
> -	 * we start scanning from right side freeing all the blocks
> -	 * after i_size and walking into the deep
> +	 * We start scanning from right side, freeing all the blocks
> +	 * after i_size and walking into the tree depth-wise.
>  	 */
>  	path = kmalloc(sizeof(struct ext4_ext_path) * (depth + 1), GFP_KERNEL);
>  	if (path == NULL) {
> @@ -1749,7 +1777,7 @@ int ext4_ext_remove_space(struct inode *
>  		if (i == depth) {
>  			/* this is leaf block */
>  			err = ext4_ext_rm_leaf(handle, inode, path, start);
> -			/* root level have p_bh == NULL, brelse() eats this */
> +			/* root level has p_bh == NULL, brelse() eats this */
>  			brelse(path[i].p_bh);
>  			path[i].p_bh = NULL;
>  			i--;
> @@ -1772,14 +1800,14 @@ int ext4_ext_remove_space(struct inode *
>  		BUG_ON(path[i].p_hdr->eh_magic != EXT4_EXT_MAGIC);
>  
>  		if (!path[i].p_idx) {
> -			/* this level hasn't touched yet */
> +			/* this level hasn't been touched yet */
>  			path[i].p_idx = EXT_LAST_INDEX(path[i].p_hdr);
>  			path[i].p_block = le16_to_cpu(path[i].p_hdr->eh_entries)+1;
>  			ext_debug("init index ptr: hdr 0x%p, num %d\n",
>  				  path[i].p_hdr,
>  				  le16_to_cpu(path[i].p_hdr->eh_entries));
>  		} else {
> -			/* we've already was here, see at next index */
> +			/* we were already here, see at next index */
>  			path[i].p_idx--;
>  		}
>  
> @@ -1799,19 +1827,19 @@ int ext4_ext_remove_space(struct inode *
>  				break;
>  			}
>  
> -			/* put actual number of indexes to know is this
> -			 * number got changed at the next iteration */
> +			/* save actual number of indexes since this
> +			 * number is changed at the next iteration */
>  			path[i].p_block = le16_to_cpu(path[i].p_hdr->eh_entries);
>  			i++;
>  		} else {
> -			/* we finish processing this index, go up */
> +			/* we finished processing this index, go up */
>  			if (path[i].p_hdr->eh_entries == 0 && i > 0) {
> -				/* index is empty, remove it
> +				/* index is empty, remove it;
>  				 * handle must be already prepared by the
>  				 * truncatei_leaf() */
>  				err = ext4_ext_rm_idx(handle, inode, path + i);
>  			}
> -			/* root level have p_bh == NULL, brelse() eats this */
> +			/* root level has p_bh == NULL, brelse() eats this */
>  			brelse(path[i].p_bh);
>  			path[i].p_bh = NULL;
>  			i--;
> @@ -1822,8 +1850,8 @@ int ext4_ext_remove_space(struct inode *
>  	/* TODO: flexible tree reduction should be here */
>  	if (path->p_hdr->eh_entries == 0) {
>  		/*
> -		 * truncate to zero freed all the tree
> -		 * so, we need to correct eh_depth
> +		 * truncate to zero freed all the tree,
> +		 * so we need to correct eh_depth
>  		 */
>  		err = ext4_ext_get_access(handle, inode, path);
>  		if (err == 0) {
> @@ -1911,7 +1939,7 @@ int ext4_ext_get_blocks(handle_t *handle
>  		if (goal == EXT4_EXT_CACHE_GAP) {
>  			if (!create) {
>  				/* block isn't allocated yet and
> -				 * user don't want to allocate it */
> +				 * user doesn't want to allocate it */
>  				goto out2;
>  			}
>  			/* we should allocate requested block */
> @@ -1920,7 +1948,7 @@ int ext4_ext_get_blocks(handle_t *handle
>  		        newblock = iblock
>  		                   - le32_to_cpu(newex.ee_block)
>  			           + ext_pblock(&newex);
> -			/* number of remain blocks in the extent */
> +			/* number of remaining blocks in the extent */
>  			allocated = le16_to_cpu(newex.ee_len) -
>  					(iblock - le32_to_cpu(newex.ee_block));
>  			goto out;
> @@ -1940,8 +1968,8 @@ int ext4_ext_get_blocks(handle_t *handle
>  	depth = ext_depth(inode);
>  
>  	/*
> -	 * consistent leaf must not be empty
> -	 * this situations is possible, though, _during_ tree modification
> +	 * consistent leaf must not be empty;
> +	 * this situation is possible, though, _during_ tree modification;
>  	 * this is why assert can't be put in ext4_ext_find_extent()
>  	 */
>  	BUG_ON(path[depth].p_ext == NULL && depth != 0);
> @@ -1959,10 +1987,10 @@ int ext4_ext_get_blocks(handle_t *handle
>  		 */
>  		if (ee_len > EXT_MAX_LEN)
>  			goto out2;
> -		/* if found exent covers block, simple return it */
> +		/* if found extent covers block, simply return it */
>  	        if (iblock >= ee_block && iblock < ee_block + ee_len) {
>  			newblock = iblock - ee_block + ee_start;
> -			/* number of remain blocks in the extent */
> +			/* number of remaining blocks in the extent */
>  			allocated = ee_len - (iblock - ee_block);
>  			ext_debug("%d fit into %lu:%d -> "E3FSBLK"\n", (int) iblock,
>  					ee_block, ee_len, newblock);
> @@ -1973,17 +2001,18 @@ int ext4_ext_get_blocks(handle_t *handle
>  	}
>  
>  	/*
> -	 * requested block isn't allocated yet
> +	 * requested block isn't allocated yet;
>  	 * we couldn't try to create block if create flag is zero
>  	 */
>  	if (!create) {
> -		/* put just found gap into cache to speedup subsequest reqs */
> +		/* put just found gap into cache to speed up
> +		 * subsequent requests */
>  		ext4_ext_put_gap_in_cache(inode, path, iblock);
>  		goto out2;
>  	}
>  	/*
>           * Okay, we need to do block allocation.  Lazily initialize the block
> -         * allocation info here if necessary
> +         * allocation info here if necessary.
>          */
>  	if (S_ISREG(inode->i_mode) && (!EXT4_I(inode)->i_block_alloc_info))
>  		ext4_init_block_alloc_info(inode);
> @@ -2061,9 +2090,9 @@ void ext4_ext_truncate(struct inode * in
>  	ext4_ext_invalidate_cache(inode);
>  
>  	/*
> -	 * TODO: optimization is possible here
> -	 * probably we need not scaning at all,
> -	 * because page truncation is enough
> +	 * TODO: optimization is possible here.
> +	 * Probably we need not scan at all,
> +	 * because page truncation is enough.
>  	 */
>  	if (ext4_orphan_add(handle, inode))
>  		goto out_stop;
> @@ -2077,13 +2106,13 @@ void ext4_ext_truncate(struct inode * in
>  	err = ext4_ext_remove_space(inode, last_block);
>  
>  	/* In a multi-transaction truncate, we only make the final
> -	 * transaction synchronous */
> +	 * transaction synchronous. */
>  	if (IS_SYNC(inode))
>  		handle->h_sync = 1;
>  
>  out_stop:
>  	/*
> -	 * If this was a simple ftruncate(), and the file will remain alive
> +	 * If this was a simple ftruncate() and the file will remain alive,
>  	 * then we need to clear up the orphan record which we created above.
>  	 * However, if this was a real unlink then we were called by
>  	 * ext4_delete_inode(), and we allow that function to clean up the
> @@ -2097,7 +2126,8 @@ out_stop:
>  }
>  
>  /*
> - * this routine calculate max number of blocks we could modify
> + * ext4_ext_writepage_trans_blocks:
> + * calculate max number of blocks we could modify
>   * in order to allocate new block for an inode
>   */
>  int ext4_ext_writepage_trans_blocks(struct inode *inode, int num)
> @@ -2106,7 +2136,7 @@ int ext4_ext_writepage_trans_blocks(stru
>  
>  	needed = ext4_ext_calc_credits_for_insert(inode, NULL);
>  
> -	/* caller want to allocate num blocks, but note it includes sb */
> +	/* caller wants to allocate num blocks, but note it includes sb */
>  	needed = needed * num - (num - 1);
>  
>  #ifdef CONFIG_QUOTA
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-10 18:00     ` Andrew Morton
@ 2006-08-11 22:13       ` Mingming Cao
  2006-08-11 23:16         ` Andrew Morton
  0 siblings, 1 reply; 26+ messages in thread
From: Mingming Cao @ 2006-08-11 22:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Theodore Tso, linux-fsdevel, ext2-devel, linux-kernel

On Thu, 2006-08-10 at 11:00 -0700, Andrew Morton wrote:
> On Thu, 10 Aug 2006 13:17:55 -0400
> Theodore Tso <tytso@mit.edu> wrote:
> 
> > On Wed, Aug 09, 2006 at 11:39:40PM -0700, Andrew Morton wrote:
> > > - replace all brelse() calls with put_bh().  Because brelse() is
> > >   old-fashioned, has a weird name and neelessly permits a NULL arg.
> > > 
> > >   In fact it would be beter to convert JBD and ext3 to put_bh before
> > >   copying it all over.
> > 
> > Wouldn't it be better to preserve in the source code history the
> > brelse->put_bh conversion?  We can pour a huge number of changes in
> > ext4 before we submit, but I would have thought it would be easier for
> > everyone to see what is going on if we submit with just the minimal
> > changes, and then have patches that address concerns like this one at
> > a time.
> > 
> 
> I'd suggest that this be one of the cleanups which be done within ext3
> before taking the ext4 copy.


Looked at this today -- currently brelse() and __brelse() will check the
b_count before calling put_bh().  I think it's okay to replace put_bh()
without checking the b_count, as we always call put_bh() with get_bh
()....but want to confirm with you.

Mingming


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-11 20:57     ` Randy.Dunlap
  2006-08-11 21:05       ` Alex Tomas
  2006-08-11 21:49       ` Mingming Cao
@ 2006-08-11 23:00       ` Andrew Morton
  2006-08-12  6:02         ` Randy.Dunlap
  2006-08-15 15:40       ` Pavel Machek
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Morton @ 2006-08-11 23:00 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: Alex Tomas, cmm, linux-fsdevel, ext2-devel, linux-kernel

On Fri, 11 Aug 2006 13:57:37 -0700
"Randy.Dunlap" <rdunlap@xenotime.net> wrote:

> On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:
> 
> >  AM> - The existing comments could benefit from some rework by a
> >  AM> native English speaker.
> > 
> > could someone assist here, please?
> 
> See if this helps.

Thanks, Randy.  The Kconfig help text could do with some help too, if
you're feeling keen..  

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-11 22:13       ` Mingming Cao
@ 2006-08-11 23:16         ` Andrew Morton
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Morton @ 2006-08-11 23:16 UTC (permalink / raw)
  To: cmm; +Cc: Theodore Tso, linux-fsdevel, ext2-devel, linux-kernel

On Fri, 11 Aug 2006 15:13:09 -0700
Mingming Cao <cmm@us.ibm.com> wrote:

> On Thu, 2006-08-10 at 11:00 -0700, Andrew Morton wrote:
> > On Thu, 10 Aug 2006 13:17:55 -0400
> > Theodore Tso <tytso@mit.edu> wrote:
> > 
> > > On Wed, Aug 09, 2006 at 11:39:40PM -0700, Andrew Morton wrote:
> > > > - replace all brelse() calls with put_bh().  Because brelse() is
> > > >   old-fashioned, has a weird name and neelessly permits a NULL arg.
> > > > 
> > > >   In fact it would be beter to convert JBD and ext3 to put_bh before
> > > >   copying it all over.
> > > 
> > > Wouldn't it be better to preserve in the source code history the
> > > brelse->put_bh conversion?  We can pour a huge number of changes in
> > > ext4 before we submit, but I would have thought it would be easier for
> > > everyone to see what is going on if we submit with just the minimal
> > > changes, and then have patches that address concerns like this one at
> > > a time.
> > > 
> > 
> > I'd suggest that this be one of the cleanups which be done within ext3
> > before taking the ext4 copy.
> 
> 
> Looked at this today -- currently brelse() and __brelse() will check the
> b_count before calling put_bh().  I think it's okay to replace put_bh()
> without checking the b_count, as we always call put_bh() with get_bh
> ()....but want to confirm with you.
> 

I haven't seen that warning come out in a couple of years.

I guess that during development it would be useful to trap underflows in
put_bh().

Like this?


 fs/buffer.c                 |    8 ++++++++
 include/linux/buffer_head.h |    6 +-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff -puN include/linux/buffer_head.h~put_bh-debug include/linux/buffer_head.h
--- a/include/linux/buffer_head.h~put_bh-debug
+++ a/include/linux/buffer_head.h
@@ -232,11 +232,7 @@ static inline void get_bh(struct buffer_
         atomic_inc(&bh->b_count);
 }
 
-static inline void put_bh(struct buffer_head *bh)
-{
-        smp_mb__before_atomic_dec();
-        atomic_dec(&bh->b_count);
-}
+void put_bh(struct buffer_head *bh);
 
 static inline void brelse(struct buffer_head *bh)
 {
diff -puN fs/buffer.c~put_bh-debug fs/buffer.c
--- a/fs/buffer.c~put_bh-debug
+++ a/fs/buffer.c
@@ -47,6 +47,14 @@ static void invalidate_bh_lrus(void);
 
 #define BH_ENTRY(list) list_entry((list), struct buffer_head, b_assoc_buffers)
 
+void put_bh(struct buffer_head *bh)
+{
+	WARN_ON(atomic_read(&bh->b_count <= 0);
+        smp_mb__before_atomic_dec();
+        atomic_dec(&bh->b_count);
+}
+EXPORT_SYMBOL(put_bh);
+
 inline void
 init_buffer(struct buffer_head *bh, bh_end_io_t *handler, void *private)
 {
_


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-11 23:00       ` Andrew Morton
@ 2006-08-12  6:02         ` Randy.Dunlap
  2006-08-12 17:43           ` Darrick J. Wong
  0 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-08-12  6:02 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-fsdevel, cmm, ext2-devel, Alex Tomas, linux-kernel

On Fri, 11 Aug 2006 16:00:02 -0700 Andrew Morton wrote:

> On Fri, 11 Aug 2006 13:57:37 -0700
> "Randy.Dunlap" <rdunlap@xenotime.net> wrote:
> 
> > On Thu, 10 Aug 2006 13:29:56 +0400 Alex Tomas wrote:
> > 
> > >  AM> - The existing comments could benefit from some rework by a
> > >  AM> native English speaker.
> > > 
> > > could someone assist here, please?
> > 
> > See if this helps.
> 
> Thanks, Randy.  The Kconfig help text could do with some help too,
> if you're feeling keen..  

Uh, yes.  Well, I don't really care for the "ext3dev" name, but
I tried to ignore that "feature" and fix it up anyway.
Feel free to ignore any parts that you don't want.


---
From: Randy Dunlap <rdunlap@xenotime.net>

Clean up help text and module names in ext4 & jbd2 Kconfig entries.
Add "depends on EXPERIMENTAL".

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 fs/Kconfig |   59 ++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 30 insertions(+), 29 deletions(-)

--- linux-2618-rc4-ext4.orig/fs/Kconfig
+++ linux-2618-rc4-ext4/fs/Kconfig
@@ -139,28 +139,29 @@ config EXT3_FS_SECURITY
 	  extended attributes for file security labels, say N.
 
 config EXT3DEV_FS
-	tristate "Developmenting extended fs support"
+	tristate "Ext3dev/ext4 extended fs support development"
+	depends on EXPERIMENTAL
 	select JBD2
 	help
-	  Ext3dev is a precede filesystem toward next generation
-	  of extended fs, based on ext3 filesystem code. It will be
-	  renamed ext4 fs later once this ext3dev is mature and stabled.
+	  Ext3dev is a predecessor filesystem of the next generation
+	  extended fs ext4, based on ext3 filesystem code. It will be
+	  renamed ext4 fs later, once ext3dev is mature and stabled.
 
 	  Unlike the change from ext2 filesystem to ext3 filesystem,
 	  the on-disk format of ext3dev is not the same as ext3 any more:
-	  it is based on extent maps and it support 48 bit physical block
+	  it is based on extent maps and it supports 48-bit physical block
 	  numbers. These combined on-disk format changes will allow
-	  ext3dev/ext4 to handle more than 16TB filesystem volume --
-	  a hard limit that ext3 can not overcome without changing
+	  ext3dev/ext4 to handle more than 16 TB filesystem volumes --
+	  a hard limit that ext3 cannot overcome without changing the
 	  on-disk format.
 
-	  Other than extent maps and 48 bit block number, ext3dev also is
+	  Other than extent maps and 48-bit block number, ext3dev also is
 	  likely to have other new features such as persistent preallocation,
-	  high resolution time stamps and larger file support etc.  These
+	  high resolution time stamps, and larger file support etc.  These
 	  features will be added to ext3dev gradually.
 
-	  To compile this file system support as a module, choose M here: the
-	  module will be called ext2.  Be aware however that the file system
+	  To compile this file system support as a module, choose M here. The
+	  module will be called ext3dev.  Be aware, however, that the filesystem
 	  of your root partition (the one containing the directory /) cannot
 	  be compiled as a module, and so this could be dangerous.
 
@@ -177,17 +178,17 @@ config EXT3DEV_FS_XATTR
 
 	  If unsure, say N.
 
-	  You need this for POSIX ACL support on ext3.
+	  You need this for POSIX ACL support on ext3dev/ext4.
 
 config EXT3DEV_FS_POSIX_ACL
 	bool "Ext3dev POSIX Access Control Lists"
 	depends on EXT3DEV_FS_XATTR
 	select FS_POSIX_ACL
 	help
-	  Posix Access Control Lists (ACLs) support permissions for users and
+	  POSIX Access Control Lists (ACLs) support permissions for users and
 	  groups beyond the owner/group/world scheme.
 
-	  To learn more about Access Control Lists, visit the Posix ACLs for
+	  To learn more about Access Control Lists, visit the POSIX ACLs for
 	  Linux website <http://acl.bestbits.at/>.
 
 	  If you don't know what Access Control Lists are, say N
@@ -199,7 +200,7 @@ config EXT3DEV_FS_SECURITY
 	  Security labels support alternative access control models
 	  implemented by security modules like SELinux.  This option
 	  enables an extended attribute handler for file security
-	  labels in the ext3 filesystem.
+	  labels in the ext3dev/ext4 filesystem.
 
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
@@ -240,31 +241,31 @@ config JBD2
 	tristate
 	help
 	  This is a generic journaling layer for block devices that support
-	  both 32 bit and 64 bit block numbers.  It is currently used by
-	  the ext3dev/ext4 file system, but it could also be used to add
+	  both 32-bit and 64-bit block numbers.  It is currently used by
+	  the ext3dev/ext4 filesystem, but it could also be used to add
 	  journal support to other file systems or block devices such
-	   as RAID or LVM.
+	  as RAID or LVM.
 
-	  If you are using the ext4, you need to say Y here. If you are not
-	  using ext4 then you will probably want to say N.
+	  If you are using ext3dev/ext4, you need to say Y here. If you are not
+	  using ext3dev/ext4 then you will probably want to say N.
 
-	  To compile this device as a module, choose M here: the module will be
-	  called jbd.  If you are compiling ext4 into the kernel,
+	  To compile this device as a module, choose M here. The module will be
+	  called jbd2.  If you are compiling ext3dev/ext4 into the kernel,
 	  you cannot compile this code as a module.
 
 config JBD2_DEBUG
-	bool "JBD2 (ext4) debugging support"
+	bool "JBD2 (ext3dev/ext4) debugging support"
 	depends on JBD2
 	help
-	  If you are using the ext4 journaled file system (or potentially any
-	  other file system/device using JBD2), this option allows you to
-	  enable debugging output while the system is running, in order to
-	  help track down any problems you are having.  By default the
-	  debugging output will be turned off.
+	  If you are using the ext3dev/ext4 journaled file system (or
+	  potentially any other filesystem/device using JBD2), this option
+	  allows you to enable debugging output while the system is running,
+	  in order to help track down any problems you are having.
+	  By default the debugging output will be turned off.
 
 	  If you select Y here, then you will be able to turn on debugging
 	  with "echo N > /proc/sys/fs/jbd2-debug", where N is a number between
-	  1 and 5, the higher the number, the more debugging output is
+	  1 and 5. The higher the number, the more debugging output is
 	  generated.  To turn debugging off again, do
 	  "echo 0 > /proc/sys/fs/jbd2-debug".
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-12  6:02         ` Randy.Dunlap
@ 2006-08-12 17:43           ` Darrick J. Wong
  2006-08-12 18:20             ` Randy.Dunlap
  0 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2006-08-12 17:43 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Andrew Morton, linux-fsdevel, cmm, ext2-devel, Alex Tomas,
	linux-kernel

Randy.Dunlap wrote:

> Uh, yes.  Well, I don't really care for the "ext3dev" name, but
> I tried to ignore that "feature" and fix it up anyway.
> Feel free to ignore any parts that you don't want.

Three nits to pick:

> +	  renamed ext4 fs later, once ext3dev is mature and stabled.

I think you want "stabilized", not "stabled".

(Until someone writes horsefs, that is. ;))

> +	  Other than extent maps and 48-bit block number, ext3dev also is

"...48-bit block numbers..."

> +	  By default the debugging output will be turned off.

"By default, the..."

--D

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-12 17:43           ` Darrick J. Wong
@ 2006-08-12 18:20             ` Randy.Dunlap
  2006-08-14 16:26               ` Mingming Cao
  0 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-08-12 18:20 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Andrew Morton, linux-fsdevel, cmm, ext2-devel, Alex Tomas,
	linux-kernel

On Sat, 12 Aug 2006 10:43:04 -0700 Darrick J. Wong wrote:

> Randy.Dunlap wrote:
> 
> > Uh, yes.  Well, I don't really care for the "ext3dev" name, but
> > I tried to ignore that "feature" and fix it up anyway.
> > Feel free to ignore any parts that you don't want.
> 
> Three nits to pick:
> 
> > +	  renamed ext4 fs later, once ext3dev is mature and
> > stabled.
> 
> I think you want "stabilized", not "stabled".
> 
> (Until someone writes horsefs, that is. ;))
> 
> > +	  Other than extent maps and 48-bit block number,
> > ext3dev also is
> 
> "...48-bit block numbers..."
> 
> > +	  By default the debugging output will be turned off.
> 
> "By default, the..."

Thanks, all fixed, although I think that the comma on the last
one is optional.  New patch is below, although what I would
really prefer to see is this:

- Drop the "ext3dev" name.  Use "ext4dev" temporarily, then
  switch to "ext4".

---
From: Randy Dunlap <rdunlap@xenotime.net>

Clean up help text and module names in ext4 & jbd2 Kconfig entries.
Add "depends on EXPERIMENTAL".

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 fs/Kconfig |   59 ++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 30 insertions(+), 29 deletions(-)

--- linux-2618-rc4-ext4.orig/fs/Kconfig
+++ linux-2618-rc4-ext4/fs/Kconfig
@@ -139,28 +139,29 @@ config EXT3_FS_SECURITY
 	  extended attributes for file security labels, say N.
 
 config EXT3DEV_FS
-	tristate "Developmenting extended fs support"
+	tristate "Ext3dev/ext4 extended fs support development (EXPERIMENTAL)"
+	depends on EXPERIMENTAL
 	select JBD2
 	help
-	  Ext3dev is a precede filesystem toward next generation
-	  of extended fs, based on ext3 filesystem code. It will be
-	  renamed ext4 fs later once this ext3dev is mature and stabled.
+	  Ext3dev is a predecessor filesystem of the next generation
+	  extended fs ext4, based on ext3 filesystem code. It will be
+	  renamed ext4 fs later, once ext3dev is mature and stabilized.
 
 	  Unlike the change from ext2 filesystem to ext3 filesystem,
 	  the on-disk format of ext3dev is not the same as ext3 any more:
-	  it is based on extent maps and it support 48 bit physical block
+	  it is based on extent maps and it supports 48-bit physical block
 	  numbers. These combined on-disk format changes will allow
-	  ext3dev/ext4 to handle more than 16TB filesystem volume --
-	  a hard limit that ext3 can not overcome without changing
+	  ext3dev/ext4 to handle more than 16 TB filesystem volumes --
+	  a hard limit that ext3 cannot overcome without changing the
 	  on-disk format.
 
-	  Other than extent maps and 48 bit block number, ext3dev also is
+	  Other than extent maps and 48-bit block numbers, ext3dev also is
 	  likely to have other new features such as persistent preallocation,
-	  high resolution time stamps and larger file support etc.  These
+	  high resolution time stamps, and larger file support etc.  These
 	  features will be added to ext3dev gradually.
 
-	  To compile this file system support as a module, choose M here: the
-	  module will be called ext2.  Be aware however that the file system
+	  To compile this file system support as a module, choose M here. The
+	  module will be called ext3dev.  Be aware, however, that the filesystem
 	  of your root partition (the one containing the directory /) cannot
 	  be compiled as a module, and so this could be dangerous.
 
@@ -177,17 +178,17 @@ config EXT3DEV_FS_XATTR
 
 	  If unsure, say N.
 
-	  You need this for POSIX ACL support on ext3.
+	  You need this for POSIX ACL support on ext3dev/ext4.
 
 config EXT3DEV_FS_POSIX_ACL
 	bool "Ext3dev POSIX Access Control Lists"
 	depends on EXT3DEV_FS_XATTR
 	select FS_POSIX_ACL
 	help
-	  Posix Access Control Lists (ACLs) support permissions for users and
+	  POSIX Access Control Lists (ACLs) support permissions for users and
 	  groups beyond the owner/group/world scheme.
 
-	  To learn more about Access Control Lists, visit the Posix ACLs for
+	  To learn more about Access Control Lists, visit the POSIX ACLs for
 	  Linux website <http://acl.bestbits.at/>.
 
 	  If you don't know what Access Control Lists are, say N
@@ -199,7 +200,7 @@ config EXT3DEV_FS_SECURITY
 	  Security labels support alternative access control models
 	  implemented by security modules like SELinux.  This option
 	  enables an extended attribute handler for file security
-	  labels in the ext3 filesystem.
+	  labels in the ext3dev/ext4 filesystem.
 
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
@@ -240,31 +241,31 @@ config JBD2
 	tristate
 	help
 	  This is a generic journaling layer for block devices that support
-	  both 32 bit and 64 bit block numbers.  It is currently used by
-	  the ext3dev/ext4 file system, but it could also be used to add
+	  both 32-bit and 64-bit block numbers.  It is currently used by
+	  the ext3dev/ext4 filesystem, but it could also be used to add
 	  journal support to other file systems or block devices such
-	   as RAID or LVM.
+	  as RAID or LVM.
 
-	  If you are using the ext4, you need to say Y here. If you are not
-	  using ext4 then you will probably want to say N.
+	  If you are using ext3dev/ext4, you need to say Y here. If you are not
+	  using ext3dev/ext4 then you will probably want to say N.
 
-	  To compile this device as a module, choose M here: the module will be
-	  called jbd.  If you are compiling ext4 into the kernel,
+	  To compile this device as a module, choose M here. The module will be
+	  called jbd2.  If you are compiling ext3dev/ext4 into the kernel,
 	  you cannot compile this code as a module.
 
 config JBD2_DEBUG
-	bool "JBD2 (ext4) debugging support"
+	bool "JBD2 (ext3dev/ext4) debugging support"
 	depends on JBD2
 	help
-	  If you are using the ext4 journaled file system (or potentially any
-	  other file system/device using JBD2), this option allows you to
-	  enable debugging output while the system is running, in order to
-	  help track down any problems you are having.  By default the
-	  debugging output will be turned off.
+	  If you are using the ext3dev/ext4 journaled file system (or
+	  potentially any other filesystem/device using JBD2), this option
+	  allows you to enable debugging output while the system is running,
+	  in order to help track down any problems you are having.
+	  By default, the debugging output will be turned off.
 
 	  If you select Y here, then you will be able to turn on debugging
 	  with "echo N > /proc/sys/fs/jbd2-debug", where N is a number between
-	  1 and 5, the higher the number, the more debugging output is
+	  1 and 5. The higher the number, the more debugging output is
 	  generated.  To turn debugging off again, do
 	  "echo 0 > /proc/sys/fs/jbd2-debug".
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-12 18:20             ` Randy.Dunlap
@ 2006-08-14 16:26               ` Mingming Cao
  2006-08-14 17:22                 ` Randy.Dunlap
  0 siblings, 1 reply; 26+ messages in thread
From: Mingming Cao @ 2006-08-14 16:26 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Darrick J. Wong, Andrew Morton, linux-fsdevel, ext2-devel,
	Alex Tomas, linux-kernel

On Sat, 2006-08-12 at 11:20 -0700, Randy.Dunlap wrote:
> On Sat, 12 Aug 2006 10:43:04 -0700 Darrick J. Wong wrote:
> 
> > Randy.Dunlap wrote:
> > 
> > > Uh, yes.  Well, I don't really care for the "ext3dev" name, but
> > > I tried to ignore that "feature" and fix it up anyway.
> > > Feel free to ignore any parts that you don't want.
> > 
> > Three nits to pick:
> > 
> > > +	  renamed ext4 fs later, once ext3dev is mature and
> > > stabled.
> > 
> > I think you want "stabilized", not "stabled".
> > 
> > (Until someone writes horsefs, that is. ;))
> > 
> > > +	  Other than extent maps and 48-bit block number,
> > > ext3dev also is
> > 
> > "...48-bit block numbers..."
> > 
> > > +	  By default the debugging output will be turned off.
> > 
> > "By default, the..."
> 
> Thanks, all fixed, although I think that the comma on the last
> one is optional. 

Thanks, Randy and Darrick.

>  New patch is below, although what I would
> really prefer to see is this:
> 
> - Drop the "ext3dev" name.  Use "ext4dev" temporarily, then
>   switch to "ext4".
> 

I think ext4dev is a better name too. Would you like to make that
changes as well?

Thanks,

Mingming
> ---
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Clean up help text and module names in ext4 & jbd2 Kconfig entries.
> Add "depends on EXPERIMENTAL".
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> ---
>  fs/Kconfig |   59 ++++++++++++++++++++++++++++++-----------------------------
>  1 files changed, 30 insertions(+), 29 deletions(-)
> 
> --- linux-2618-rc4-ext4.orig/fs/Kconfig
> +++ linux-2618-rc4-ext4/fs/Kconfig
> @@ -139,28 +139,29 @@ config EXT3_FS_SECURITY
>  	  extended attributes for file security labels, say N.
>  
>  config EXT3DEV_FS
> -	tristate "Developmenting extended fs support"
> +	tristate "Ext3dev/ext4 extended fs support development (EXPERIMENTAL)"
> +	depends on EXPERIMENTAL
>  	select JBD2
>  	help
> -	  Ext3dev is a precede filesystem toward next generation
> -	  of extended fs, based on ext3 filesystem code. It will be
> -	  renamed ext4 fs later once this ext3dev is mature and stabled.
> +	  Ext3dev is a predecessor filesystem of the next generation
> +	  extended fs ext4, based on ext3 filesystem code. It will be
> +	  renamed ext4 fs later, once ext3dev is mature and stabilized.
>  
>  	  Unlike the change from ext2 filesystem to ext3 filesystem,
>  	  the on-disk format of ext3dev is not the same as ext3 any more:
> -	  it is based on extent maps and it support 48 bit physical block
> +	  it is based on extent maps and it supports 48-bit physical block
>  	  numbers. These combined on-disk format changes will allow
> -	  ext3dev/ext4 to handle more than 16TB filesystem volume --
> -	  a hard limit that ext3 can not overcome without changing
> +	  ext3dev/ext4 to handle more than 16 TB filesystem volumes --
> +	  a hard limit that ext3 cannot overcome without changing the
>  	  on-disk format.
>  
> -	  Other than extent maps and 48 bit block number, ext3dev also is
> +	  Other than extent maps and 48-bit block numbers, ext3dev also is
>  	  likely to have other new features such as persistent preallocation,
> -	  high resolution time stamps and larger file support etc.  These
> +	  high resolution time stamps, and larger file support etc.  These
>  	  features will be added to ext3dev gradually.
>  
> -	  To compile this file system support as a module, choose M here: the
> -	  module will be called ext2.  Be aware however that the file system
> +	  To compile this file system support as a module, choose M here. The
> +	  module will be called ext3dev.  Be aware, however, that the filesystem
>  	  of your root partition (the one containing the directory /) cannot
>  	  be compiled as a module, and so this could be dangerous.
>  
> @@ -177,17 +178,17 @@ config EXT3DEV_FS_XATTR
>  
>  	  If unsure, say N.
>  
> -	  You need this for POSIX ACL support on ext3.
> +	  You need this for POSIX ACL support on ext3dev/ext4.
>  
>  config EXT3DEV_FS_POSIX_ACL
>  	bool "Ext3dev POSIX Access Control Lists"
>  	depends on EXT3DEV_FS_XATTR
>  	select FS_POSIX_ACL
>  	help
> -	  Posix Access Control Lists (ACLs) support permissions for users and
> +	  POSIX Access Control Lists (ACLs) support permissions for users and
>  	  groups beyond the owner/group/world scheme.
>  
> -	  To learn more about Access Control Lists, visit the Posix ACLs for
> +	  To learn more about Access Control Lists, visit the POSIX ACLs for
>  	  Linux website <http://acl.bestbits.at/>.
>  
>  	  If you don't know what Access Control Lists are, say N
> @@ -199,7 +200,7 @@ config EXT3DEV_FS_SECURITY
>  	  Security labels support alternative access control models
>  	  implemented by security modules like SELinux.  This option
>  	  enables an extended attribute handler for file security
> -	  labels in the ext3 filesystem.
> +	  labels in the ext3dev/ext4 filesystem.
>  
>  	  If you are not using a security module that requires using
>  	  extended attributes for file security labels, say N.
> @@ -240,31 +241,31 @@ config JBD2
>  	tristate
>  	help
>  	  This is a generic journaling layer for block devices that support
> -	  both 32 bit and 64 bit block numbers.  It is currently used by
> -	  the ext3dev/ext4 file system, but it could also be used to add
> +	  both 32-bit and 64-bit block numbers.  It is currently used by
> +	  the ext3dev/ext4 filesystem, but it could also be used to add
>  	  journal support to other file systems or block devices such
> -	   as RAID or LVM.
> +	  as RAID or LVM.
>  
> -	  If you are using the ext4, you need to say Y here. If you are not
> -	  using ext4 then you will probably want to say N.
> +	  If you are using ext3dev/ext4, you need to say Y here. If you are not
> +	  using ext3dev/ext4 then you will probably want to say N.
>  
> -	  To compile this device as a module, choose M here: the module will be
> -	  called jbd.  If you are compiling ext4 into the kernel,
> +	  To compile this device as a module, choose M here. The module will be
> +	  called jbd2.  If you are compiling ext3dev/ext4 into the kernel,
>  	  you cannot compile this code as a module.
>  
>  config JBD2_DEBUG
> -	bool "JBD2 (ext4) debugging support"
> +	bool "JBD2 (ext3dev/ext4) debugging support"
>  	depends on JBD2
>  	help
> -	  If you are using the ext4 journaled file system (or potentially any
> -	  other file system/device using JBD2), this option allows you to
> -	  enable debugging output while the system is running, in order to
> -	  help track down any problems you are having.  By default the
> -	  debugging output will be turned off.
> +	  If you are using the ext3dev/ext4 journaled file system (or
> +	  potentially any other filesystem/device using JBD2), this option
> +	  allows you to enable debugging output while the system is running,
> +	  in order to help track down any problems you are having.
> +	  By default, the debugging output will be turned off.
>  
>  	  If you select Y here, then you will be able to turn on debugging
>  	  with "echo N > /proc/sys/fs/jbd2-debug", where N is a number between
> -	  1 and 5, the higher the number, the more debugging output is
> +	  1 and 5. The higher the number, the more debugging output is
>  	  generated.  To turn debugging off again, do
>  	  "echo 0 > /proc/sys/fs/jbd2-debug".
>  


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-14 16:26               ` Mingming Cao
@ 2006-08-14 17:22                 ` Randy.Dunlap
  2006-08-14 17:52                   ` Jeff Garzik
  0 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-08-14 17:22 UTC (permalink / raw)
  To: cmm
  Cc: Darrick J. Wong, Andrew Morton, linux-fsdevel, ext2-devel,
	Alex Tomas, linux-kernel

On Mon, 14 Aug 2006 09:26:31 -0700 Mingming Cao wrote:

> On Sat, 2006-08-12 at 11:20 -0700, Randy.Dunlap wrote:
> > 
> >  New patch is below, although what I would
> > really prefer to see is this:
> > 
> > - Drop the "ext3dev" name.  Use "ext4dev" temporarily, then
> >   switch to "ext4".
> 
> I think ext4dev is a better name too. Would you like to make that
> changes as well?

---
From: Randy Dunlap <rdunlap@xenotime.net>

Rename ext3dev to ext4dev.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 fs/Kconfig                |   58 +++++++++++++++++++++++-----------------------
 fs/Makefile               |    2 -
 fs/ext4/Makefile          |   10 +++----
 fs/ext4/acl.h             |    6 ++--
 fs/ext4/file.c            |    2 -
 fs/ext4/inode.c           |    2 -
 fs/ext4/namei.c           |    6 ++--
 fs/ext4/super.c           |   18 +++++++-------
 fs/ext4/symlink.c         |    4 +--
 fs/ext4/xattr.c           |    8 +++---
 fs/ext4/xattr.h           |    8 +++---
 include/linux/ext4_fs_i.h |    4 +--
 12 files changed, 64 insertions(+), 64 deletions(-)

--- linux-2618-rc4-ext4.orig/fs/Kconfig
+++ linux-2618-rc4-ext4/fs/Kconfig
@@ -138,38 +138,38 @@ config EXT3_FS_SECURITY
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
 
-config EXT3DEV_FS
-	tristate "Ext3dev/ext4 extended fs support development (EXPERIMENTAL)"
+config EXT4DEV_FS
+	tristate "Ext4dev extended fs support development (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
 	select JBD2
 	help
-	  Ext3dev is a predecessor filesystem of the next generation
+	  Ext4dev is a predecessor filesystem of the next generation
 	  extended fs ext4, based on ext3 filesystem code. It will be
-	  renamed ext4 fs later, once ext3dev is mature and stabilized.
+	  renamed ext4 fs later, once ext4dev is mature and stabilized.
 
 	  Unlike the change from ext2 filesystem to ext3 filesystem,
-	  the on-disk format of ext3dev is not the same as ext3 any more:
+	  the on-disk format of ext4dev is not the same as ext3 any more:
 	  it is based on extent maps and it supports 48-bit physical block
 	  numbers. These combined on-disk format changes will allow
-	  ext3dev/ext4 to handle more than 16 TB filesystem volumes --
+	  ext4dev to handle more than 16 TB filesystem volumes --
 	  a hard limit that ext3 cannot overcome without changing the
 	  on-disk format.
 
-	  Other than extent maps and 48-bit block numbers, ext3dev also is
+	  Other than extent maps and 48-bit block numbers, ext4dev also is
 	  likely to have other new features such as persistent preallocation,
 	  high resolution time stamps, and larger file support etc.  These
-	  features will be added to ext3dev gradually.
+	  features will be added to ext4dev gradually.
 
 	  To compile this file system support as a module, choose M here. The
-	  module will be called ext3dev.  Be aware, however, that the filesystem
+	  module will be called ext4dev.  Be aware, however, that the filesystem
 	  of your root partition (the one containing the directory /) cannot
 	  be compiled as a module, and so this could be dangerous.
 
 	  If unsure, say N.
 
-config EXT3DEV_FS_XATTR
-	bool "Ext3dev extended attributes"
-	depends on EXT3DEV_FS
+config EXT4DEV_FS_XATTR
+	bool "Ext4dev extended attributes"
+	depends on EXT4DEV_FS
 	default y
 	help
 	  Extended attributes are name:value pairs associated with inodes by
@@ -178,11 +178,11 @@ config EXT3DEV_FS_XATTR
 
 	  If unsure, say N.
 
-	  You need this for POSIX ACL support on ext3dev/ext4.
+	  You need this for POSIX ACL support on ext4dev.
 
-config EXT3DEV_FS_POSIX_ACL
-	bool "Ext3dev POSIX Access Control Lists"
-	depends on EXT3DEV_FS_XATTR
+config EXT4DEV_FS_POSIX_ACL
+	bool "Ext4dev POSIX Access Control Lists"
+	depends on EXT4DEV_FS_XATTR
 	select FS_POSIX_ACL
 	help
 	  POSIX Access Control Lists (ACLs) support permissions for users and
@@ -193,14 +193,14 @@ config EXT3DEV_FS_POSIX_ACL
 
 	  If you don't know what Access Control Lists are, say N
 
-config EXT3DEV_FS_SECURITY
-	bool "Ext3dev Security Labels"
-	depends on EXT3DEV_FS_XATTR
+config EXT4DEV_FS_SECURITY
+	bool "Ext4dev Security Labels"
+	depends on EXT4DEV_FS_XATTR
 	help
 	  Security labels support alternative access control models
 	  implemented by security modules like SELinux.  This option
 	  enables an extended attribute handler for file security
-	  labels in the ext3dev/ext4 filesystem.
+	  labels in the ext4dev filesystem.
 
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
@@ -242,22 +242,22 @@ config JBD2
 	help
 	  This is a generic journaling layer for block devices that support
 	  both 32-bit and 64-bit block numbers.  It is currently used by
-	  the ext3dev/ext4 filesystem, but it could also be used to add
+	  the ext4dev filesystem, but it could also be used to add
 	  journal support to other file systems or block devices such
 	  as RAID or LVM.
 
-	  If you are using ext3dev/ext4, you need to say Y here. If you are not
-	  using ext3dev/ext4 then you will probably want to say N.
+	  If you are using ext4dev, you need to say Y here. If you are not
+	  using ext4dev then you will probably want to say N.
 
 	  To compile this device as a module, choose M here. The module will be
-	  called jbd2.  If you are compiling ext3dev/ext4 into the kernel,
+	  called jbd2.  If you are compiling ext4dev into the kernel,
 	  you cannot compile this code as a module.
 
 config JBD2_DEBUG
-	bool "JBD2 (ext3dev/ext4) debugging support"
+	bool "JBD2 (ext4dev) debugging support"
 	depends on JBD2
 	help
-	  If you are using the ext3dev/ext4 journaled file system (or
+	  If you are using the ext4dev journaled file system (or
 	  potentially any other filesystem/device using JBD2), this option
 	  allows you to enable debugging output while the system is running,
 	  in order to help track down any problems you are having.
@@ -272,9 +272,9 @@ config JBD2_DEBUG
 config FS_MBCACHE
 # Meta block cache for Extended Attributes (ext2/ext3/ext4)
 	tristate
-	depends on EXT2_FS_XATTR || EXT3_FS_XATTR || EXT3DEV_FS_XATTR
-	default y if EXT2_FS=y || EXT3_FS=y || EXT3DEV_FS=y
-	default m if EXT2_FS=m || EXT3_FS=m || EXT3DEV_FS=m
+	depends on EXT2_FS_XATTR || EXT3_FS_XATTR || EXT4DEV_FS_XATTR
+	default y if EXT2_FS=y || EXT3_FS=y || EXT4DEV_FS=y
+	default m if EXT2_FS=m || EXT3_FS=m || EXT4DEV_FS=m
 
 config REISERFS_FS
 	tristate "Reiserfs support"
--- linux-2618-rc4-ext4.orig/fs/Makefile
+++ linux-2618-rc4-ext4/fs/Makefile
@@ -54,7 +54,7 @@ obj-$(CONFIG_PROFILING)		+= dcookies.o
 # Do not add any filesystems before this line
 obj-$(CONFIG_REISERFS_FS)	+= reiserfs/
 obj-$(CONFIG_EXT3_FS)		+= ext3/ # Before ext2 so root fs can be ext3
-obj-$(CONFIG_EXT3DEV_FS)	+= ext4/ # Before ext2 so root fs can be ext3dev
+obj-$(CONFIG_EXT4DEV_FS)	+= ext4/ # Before ext2 so root fs can be ext3dev
 obj-$(CONFIG_JBD)		+= jbd/
 obj-$(CONFIG_JBD2)		+= jbd2/
 obj-$(CONFIG_EXT2_FS)		+= ext2/
--- linux-2618-rc4-ext4.orig/fs/ext4/Makefile
+++ linux-2618-rc4-ext4/fs/ext4/Makefile
@@ -2,11 +2,11 @@
 # Makefile for the linux ext4-filesystem routines.
 #
 
-obj-$(CONFIG_EXT3DEV_FS) += ext3dev.o
+obj-$(CONFIG_EXT4DEV_FS) += ext4dev.o
 
-ext3dev-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
+ext4dev-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
 	   ioctl.o namei.o super.o symlink.o hash.o resize.o extents.o
 
-ext3dev-$(CONFIG_EXT3DEV_FS_XATTR)	+= xattr.o xattr_user.o xattr_trusted.o
-ext3dev-$(CONFIG_EXT3DEV_FS_POSIX_ACL)	+= acl.o
-ext3dev-$(CONFIG_EXT3DEV_FS_SECURITY)	+= xattr_security.o
+ext4dev-$(CONFIG_EXT4DEV_FS_XATTR)	+= xattr.o xattr_user.o xattr_trusted.o
+ext4dev-$(CONFIG_EXT4DEV_FS_POSIX_ACL)	+= acl.o
+ext4dev-$(CONFIG_EXT4DEV_FS_SECURITY)	+= xattr_security.o
--- linux-2618-rc4-ext4.orig/fs/ext4/acl.h
+++ linux-2618-rc4-ext4/fs/ext4/acl.h
@@ -51,7 +51,7 @@ static inline int ext4_acl_count(size_t 
 	}
 }
 
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 
 /* Value for inode->u.ext4_i.i_acl and inode->u.ext4_i.i_default_acl
    if the ACL has not been cached */
@@ -62,7 +62,7 @@ extern int ext4_permission (struct inode
 extern int ext4_acl_chmod (struct inode *);
 extern int ext4_init_acl (handle_t *, struct inode *, struct inode *);
 
-#else  /* CONFIG_EXT3DEV_FS_POSIX_ACL */
+#else  /* CONFIG_EXT4DEV_FS_POSIX_ACL */
 #include <linux/sched.h>
 #define ext4_permission NULL
 
@@ -77,5 +77,5 @@ ext4_init_acl(handle_t *handle, struct i
 {
 	return 0;
 }
-#endif  /* CONFIG_EXT3DEV_FS_POSIX_ACL */
+#endif  /* CONFIG_EXT4DEV_FS_POSIX_ACL */
 
--- linux-2618-rc4-ext4.orig/fs/ext4/xattr.h
+++ linux-2618-rc4-ext4/fs/ext4/xattr.h
@@ -56,7 +56,7 @@ struct ext4_xattr_entry {
 #define EXT4_XATTR_SIZE(size) \
 	(((size) + EXT4_XATTR_ROUND) & ~EXT4_XATTR_ROUND)
 
-# ifdef CONFIG_EXT3DEV_FS_XATTR
+# ifdef CONFIG_EXT4DEV_FS_XATTR
 
 extern struct xattr_handler ext4_xattr_user_handler;
 extern struct xattr_handler ext4_xattr_trusted_handler;
@@ -79,7 +79,7 @@ extern void exit_ext4_xattr(void);
 
 extern struct xattr_handler *ext4_xattr_handlers[];
 
-# else  /* CONFIG_EXT3DEV_FS_XATTR */
+# else  /* CONFIG_EXT4DEV_FS_XATTR */
 
 static inline int
 ext4_xattr_get(struct inode *inode, int name_index, const char *name,
@@ -131,9 +131,9 @@ exit_ext4_xattr(void)
 
 #define ext4_xattr_handlers	NULL
 
-# endif  /* CONFIG_EXT3DEV_FS_XATTR */
+# endif  /* CONFIG_EXT4DEV_FS_XATTR */
 
-#ifdef CONFIG_EXT3DEV_FS_SECURITY
+#ifdef CONFIG_EXT4DEV_FS_SECURITY
 extern int ext4_init_security(handle_t *handle, struct inode *inode,
 				struct inode *dir);
 #else
--- linux-2618-rc4-ext4.orig/fs/ext4/file.c
+++ linux-2618-rc4-ext4/fs/ext4/file.c
@@ -126,7 +126,7 @@ const struct file_operations ext4_file_o
 struct inode_operations ext4_file_inode_operations = {
 	.truncate	= ext4_truncate,
 	.setattr	= ext4_setattr,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
--- linux-2618-rc4-ext4.orig/fs/ext4/inode.c
+++ linux-2618-rc4-ext4/fs/ext4/inode.c
@@ -2589,7 +2589,7 @@ void ext4_read_inode(struct inode * inod
 	struct buffer_head *bh;
 	int block;
 
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	ei->i_acl = EXT4_ACL_NOT_CACHED;
 	ei->i_default_acl = EXT4_ACL_NOT_CACHED;
 #endif
--- linux-2618-rc4-ext4.orig/fs/ext4/namei.c
+++ linux-2618-rc4-ext4/fs/ext4/namei.c
@@ -1689,7 +1689,7 @@ retry:
 	err = PTR_ERR(inode);
 	if (!IS_ERR(inode)) {
 		init_special_inode(inode, inode->i_mode, rdev);
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 		inode->i_op = &ext4_special_inode_operations;
 #endif
 		err = ext4_add_nondir(handle, dentry, inode);
@@ -2364,7 +2364,7 @@ struct inode_operations ext4_dir_inode_o
 	.mknod		= ext4_mknod,
 	.rename		= ext4_rename,
 	.setattr	= ext4_setattr,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
@@ -2375,7 +2375,7 @@ struct inode_operations ext4_dir_inode_o
 
 struct inode_operations ext4_special_inode_operations = {
 	.setattr	= ext4_setattr,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
--- linux-2618-rc4-ext4.orig/fs/ext4/super.c
+++ linux-2618-rc4-ext4/fs/ext4/super.c
@@ -448,7 +448,7 @@ static struct inode *ext4_alloc_inode(st
 	ei = kmem_cache_alloc(ext4_inode_cachep, SLAB_NOFS);
 	if (!ei)
 		return NULL;
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	ei->i_acl = EXT4_ACL_NOT_CACHED;
 	ei->i_default_acl = EXT4_ACL_NOT_CACHED;
 #endif
@@ -470,7 +470,7 @@ static void init_once(void * foo, kmem_c
 	if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) ==
 	    SLAB_CTOR_CONSTRUCTOR) {
 		INIT_LIST_HEAD(&ei->i_orphan);
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 		init_rwsem(&ei->xattr_sem);
 #endif
 		mutex_init(&ei->truncate_mutex);
@@ -499,7 +499,7 @@ static void destroy_inodecache(void)
 static void ext4_clear_inode(struct inode *inode)
 {
 	struct ext4_block_alloc_info *rsv = EXT4_I(inode)->i_block_alloc_info;
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	if (EXT4_I(inode)->i_acl &&
 			EXT4_I(inode)->i_acl != EXT4_ACL_NOT_CACHED) {
 		posix_acl_release(EXT4_I(inode)->i_acl);
@@ -793,7 +793,7 @@ static int parse_options (char *options,
 		case Opt_orlov:
 			clear_opt (sbi->s_mount_opt, OLDALLOC);
 			break;
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 		case Opt_user_xattr:
 			set_opt (sbi->s_mount_opt, XATTR_USER);
 			break;
@@ -806,7 +806,7 @@ static int parse_options (char *options,
 			printk("EXT4 (no)user_xattr options not supported\n");
 			break;
 #endif
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 		case Opt_acl:
 			set_opt(sbi->s_mount_opt, POSIX_ACL);
 			break;
@@ -2683,9 +2683,9 @@ static int ext4_get_sb(struct file_syste
 	return get_sb_bdev(fs_type, flags, dev_name, data, ext4_fill_super, mnt);
 }
 
-static struct file_system_type ext3dev_fs_type = {
+static struct file_system_type ext4dev_fs_type = {
 	.owner		= THIS_MODULE,
-	.name		= "ext3dev",
+	.name		= "ext4dev",
 	.get_sb		= ext4_get_sb,
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
@@ -2699,7 +2699,7 @@ static int __init init_ext4_fs(void)
 	err = init_inodecache();
 	if (err)
 		goto out1;
-        err = register_filesystem(&ext3dev_fs_type);
+        err = register_filesystem(&ext4dev_fs_type);
 	if (err)
 		goto out;
 	return 0;
@@ -2712,7 +2712,7 @@ out1:
 
 static void __exit exit_ext4_fs(void)
 {
-	unregister_filesystem(&ext3dev_fs_type);
+	unregister_filesystem(&ext4dev_fs_type);
 	destroy_inodecache();
 	exit_ext4_xattr();
 }
--- linux-2618-rc4-ext4.orig/fs/ext4/symlink.c
+++ linux-2618-rc4-ext4/fs/ext4/symlink.c
@@ -34,7 +34,7 @@ struct inode_operations ext4_symlink_ino
 	.readlink	= generic_readlink,
 	.follow_link	= page_follow_link_light,
 	.put_link	= page_put_link,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
@@ -45,7 +45,7 @@ struct inode_operations ext4_symlink_ino
 struct inode_operations ext4_fast_symlink_inode_operations = {
 	.readlink	= generic_readlink,
 	.follow_link	= ext4_follow_link,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
--- linux-2618-rc4-ext4.orig/fs/ext4/xattr.c
+++ linux-2618-rc4-ext4/fs/ext4/xattr.c
@@ -104,12 +104,12 @@ static struct mb_cache *ext4_xattr_cache
 
 static struct xattr_handler *ext4_xattr_handler_map[] = {
 	[EXT4_XATTR_INDEX_USER]		     = &ext4_xattr_user_handler,
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	[EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  = &ext4_xattr_acl_access_handler,
 	[EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &ext4_xattr_acl_default_handler,
 #endif
 	[EXT4_XATTR_INDEX_TRUSTED]	     = &ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT3DEV_FS_SECURITY
+#ifdef CONFIG_EXT4DEV_FS_SECURITY
 	[EXT4_XATTR_INDEX_SECURITY]	     = &ext4_xattr_security_handler,
 #endif
 };
@@ -117,11 +117,11 @@ static struct xattr_handler *ext4_xattr_
 struct xattr_handler *ext4_xattr_handlers[] = {
 	&ext4_xattr_user_handler,
 	&ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	&ext4_xattr_acl_access_handler,
 	&ext4_xattr_acl_default_handler,
 #endif
-#ifdef CONFIG_EXT3DEV_FS_SECURITY
+#ifdef CONFIG_EXT4DEV_FS_SECURITY
 	&ext4_xattr_security_handler,
 #endif
 	NULL
--- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_i.h
+++ linux-2618-rc4-ext4/include/linux/ext4_fs_i.h
@@ -103,7 +103,7 @@ struct ext4_inode_info {
 	struct ext4_block_alloc_info *i_block_alloc_info;
 
 	__u32	i_dir_start_lookup;
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	/*
 	 * Extended attributes can be read independently of the main file
 	 * data. Taking i_mutex even when reading would cause contention
@@ -113,7 +113,7 @@ struct ext4_inode_info {
 	 */
 	struct rw_semaphore xattr_sem;
 #endif
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	struct posix_acl	*i_acl;
 	struct posix_acl	*i_default_acl;
 #endif

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-14 17:22                 ` Randy.Dunlap
@ 2006-08-14 17:52                   ` Jeff Garzik
  2006-08-14 18:05                     ` Randy.Dunlap
  0 siblings, 1 reply; 26+ messages in thread
From: Jeff Garzik @ 2006-08-14 17:52 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: cmm, Darrick J. Wong, Andrew Morton, linux-fsdevel, ext2-devel,
	Alex Tomas, linux-kernel

Randy.Dunlap wrote:
> @@ -2683,9 +2683,9 @@ static int ext4_get_sb(struct file_syste
>  	return get_sb_bdev(fs_type, flags, dev_name, data, ext4_fill_super, mnt);
>  }
>  
> -static struct file_system_type ext3dev_fs_type = {
> +static struct file_system_type ext4dev_fs_type = {
>  	.owner		= THIS_MODULE,
> -	.name		= "ext3dev",
> +	.name		= "ext4dev",
>  	.get_sb		= ext4_get_sb,
>  	.kill_sb	= kill_block_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
> @@ -2699,7 +2699,7 @@ static int __init init_ext4_fs(void)
>  	err = init_inodecache();
>  	if (err)
>  		goto out1;
> -        err = register_filesystem(&ext3dev_fs_type);
> +        err = register_filesystem(&ext4dev_fs_type);
>  	if (err)
>  		goto out;
>  	return 0;
> @@ -2712,7 +2712,7 @@ out1:
>  
>  static void __exit exit_ext4_fs(void)
>  {
> -	unregister_filesystem(&ext3dev_fs_type);
> +	unregister_filesystem(&ext4dev_fs_type);
>  	destroy_inodecache();
>  	exit_ext4_xattr();


IMO these non-CONFIG bits should just be ext4_

	Jeff



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-14 17:52                   ` Jeff Garzik
@ 2006-08-14 18:05                     ` Randy.Dunlap
  2006-08-14 18:13                       ` Jeff Garzik
  0 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-08-14 18:05 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: cmm, Darrick J. Wong, Andrew Morton, linux-fsdevel, ext2-devel,
	Alex Tomas, linux-kernel

On Mon, 14 Aug 2006 13:52:42 -0400 Jeff Garzik wrote:

> IMO these non-CONFIG bits should just be ext4_

Agreed.  Replacement patch below.

---
From: Randy Dunlap <rdunlap@xenotime.net>

Rename ext3dev to ext4dev.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 fs/Kconfig                |   58 +++++++++++++++++++++++-----------------------
 fs/Makefile               |    2 -
 fs/ext4/Makefile          |   10 +++----
 fs/ext4/acl.h             |    6 ++--
 fs/ext4/file.c            |    2 -
 fs/ext4/inode.c           |    2 -
 fs/ext4/namei.c           |    6 ++--
 fs/ext4/super.c           |   18 +++++++-------
 fs/ext4/symlink.c         |    4 +--
 fs/ext4/xattr.c           |    8 +++---
 fs/ext4/xattr.h           |    8 +++---
 include/linux/ext4_fs_i.h |    4 +--
 12 files changed, 64 insertions(+), 64 deletions(-)

--- linux-2618-rc4-ext4.orig/fs/Kconfig
+++ linux-2618-rc4-ext4/fs/Kconfig
@@ -138,38 +138,38 @@ config EXT3_FS_SECURITY
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
 
-config EXT3DEV_FS
-	tristate "Ext3dev/ext4 extended fs support development (EXPERIMENTAL)"
+config EXT4DEV_FS
+	tristate "Ext4dev extended fs support development (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
 	select JBD2
 	help
-	  Ext3dev is a predecessor filesystem of the next generation
+	  Ext4dev is a predecessor filesystem of the next generation
 	  extended fs ext4, based on ext3 filesystem code. It will be
-	  renamed ext4 fs later, once ext3dev is mature and stabilized.
+	  renamed ext4 fs later, once ext4dev is mature and stabilized.
 
 	  Unlike the change from ext2 filesystem to ext3 filesystem,
-	  the on-disk format of ext3dev is not the same as ext3 any more:
+	  the on-disk format of ext4dev is not the same as ext3 any more:
 	  it is based on extent maps and it supports 48-bit physical block
 	  numbers. These combined on-disk format changes will allow
-	  ext3dev/ext4 to handle more than 16 TB filesystem volumes --
+	  ext4dev to handle more than 16 TB filesystem volumes --
 	  a hard limit that ext3 cannot overcome without changing the
 	  on-disk format.
 
-	  Other than extent maps and 48-bit block numbers, ext3dev also is
+	  Other than extent maps and 48-bit block numbers, ext4dev also is
 	  likely to have other new features such as persistent preallocation,
 	  high resolution time stamps, and larger file support etc.  These
-	  features will be added to ext3dev gradually.
+	  features will be added to ext4dev gradually.
 
 	  To compile this file system support as a module, choose M here. The
-	  module will be called ext3dev.  Be aware, however, that the filesystem
+	  module will be called ext4dev.  Be aware, however, that the filesystem
 	  of your root partition (the one containing the directory /) cannot
 	  be compiled as a module, and so this could be dangerous.
 
 	  If unsure, say N.
 
-config EXT3DEV_FS_XATTR
-	bool "Ext3dev extended attributes"
-	depends on EXT3DEV_FS
+config EXT4DEV_FS_XATTR
+	bool "Ext4dev extended attributes"
+	depends on EXT4DEV_FS
 	default y
 	help
 	  Extended attributes are name:value pairs associated with inodes by
@@ -178,11 +178,11 @@ config EXT3DEV_FS_XATTR
 
 	  If unsure, say N.
 
-	  You need this for POSIX ACL support on ext3dev/ext4.
+	  You need this for POSIX ACL support on ext4dev.
 
-config EXT3DEV_FS_POSIX_ACL
-	bool "Ext3dev POSIX Access Control Lists"
-	depends on EXT3DEV_FS_XATTR
+config EXT4DEV_FS_POSIX_ACL
+	bool "Ext4dev POSIX Access Control Lists"
+	depends on EXT4DEV_FS_XATTR
 	select FS_POSIX_ACL
 	help
 	  POSIX Access Control Lists (ACLs) support permissions for users and
@@ -193,14 +193,14 @@ config EXT3DEV_FS_POSIX_ACL
 
 	  If you don't know what Access Control Lists are, say N
 
-config EXT3DEV_FS_SECURITY
-	bool "Ext3dev Security Labels"
-	depends on EXT3DEV_FS_XATTR
+config EXT4DEV_FS_SECURITY
+	bool "Ext4dev Security Labels"
+	depends on EXT4DEV_FS_XATTR
 	help
 	  Security labels support alternative access control models
 	  implemented by security modules like SELinux.  This option
 	  enables an extended attribute handler for file security
-	  labels in the ext3dev/ext4 filesystem.
+	  labels in the ext4dev filesystem.
 
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
@@ -242,22 +242,22 @@ config JBD2
 	help
 	  This is a generic journaling layer for block devices that support
 	  both 32-bit and 64-bit block numbers.  It is currently used by
-	  the ext3dev/ext4 filesystem, but it could also be used to add
+	  the ext4dev filesystem, but it could also be used to add
 	  journal support to other file systems or block devices such
 	  as RAID or LVM.
 
-	  If you are using ext3dev/ext4, you need to say Y here. If you are not
-	  using ext3dev/ext4 then you will probably want to say N.
+	  If you are using ext4dev, you need to say Y here. If you are not
+	  using ext4dev then you will probably want to say N.
 
 	  To compile this device as a module, choose M here. The module will be
-	  called jbd2.  If you are compiling ext3dev/ext4 into the kernel,
+	  called jbd2.  If you are compiling ext4dev into the kernel,
 	  you cannot compile this code as a module.
 
 config JBD2_DEBUG
-	bool "JBD2 (ext3dev/ext4) debugging support"
+	bool "JBD2 (ext4dev) debugging support"
 	depends on JBD2
 	help
-	  If you are using the ext3dev/ext4 journaled file system (or
+	  If you are using the ext4dev journaled file system (or
 	  potentially any other filesystem/device using JBD2), this option
 	  allows you to enable debugging output while the system is running,
 	  in order to help track down any problems you are having.
@@ -272,9 +272,9 @@ config JBD2_DEBUG
 config FS_MBCACHE
 # Meta block cache for Extended Attributes (ext2/ext3/ext4)
 	tristate
-	depends on EXT2_FS_XATTR || EXT3_FS_XATTR || EXT3DEV_FS_XATTR
-	default y if EXT2_FS=y || EXT3_FS=y || EXT3DEV_FS=y
-	default m if EXT2_FS=m || EXT3_FS=m || EXT3DEV_FS=m
+	depends on EXT2_FS_XATTR || EXT3_FS_XATTR || EXT4DEV_FS_XATTR
+	default y if EXT2_FS=y || EXT3_FS=y || EXT4DEV_FS=y
+	default m if EXT2_FS=m || EXT3_FS=m || EXT4DEV_FS=m
 
 config REISERFS_FS
 	tristate "Reiserfs support"
--- linux-2618-rc4-ext4.orig/fs/Makefile
+++ linux-2618-rc4-ext4/fs/Makefile
@@ -54,7 +54,7 @@ obj-$(CONFIG_PROFILING)		+= dcookies.o
 # Do not add any filesystems before this line
 obj-$(CONFIG_REISERFS_FS)	+= reiserfs/
 obj-$(CONFIG_EXT3_FS)		+= ext3/ # Before ext2 so root fs can be ext3
-obj-$(CONFIG_EXT3DEV_FS)	+= ext4/ # Before ext2 so root fs can be ext3dev
+obj-$(CONFIG_EXT4DEV_FS)	+= ext4/ # Before ext2 so root fs can be ext3dev
 obj-$(CONFIG_JBD)		+= jbd/
 obj-$(CONFIG_JBD2)		+= jbd2/
 obj-$(CONFIG_EXT2_FS)		+= ext2/
--- linux-2618-rc4-ext4.orig/fs/ext4/Makefile
+++ linux-2618-rc4-ext4/fs/ext4/Makefile
@@ -2,11 +2,11 @@
 # Makefile for the linux ext4-filesystem routines.
 #
 
-obj-$(CONFIG_EXT3DEV_FS) += ext3dev.o
+obj-$(CONFIG_EXT4DEV_FS) += ext4dev.o
 
-ext3dev-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
+ext4dev-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
 	   ioctl.o namei.o super.o symlink.o hash.o resize.o extents.o
 
-ext3dev-$(CONFIG_EXT3DEV_FS_XATTR)	+= xattr.o xattr_user.o xattr_trusted.o
-ext3dev-$(CONFIG_EXT3DEV_FS_POSIX_ACL)	+= acl.o
-ext3dev-$(CONFIG_EXT3DEV_FS_SECURITY)	+= xattr_security.o
+ext4dev-$(CONFIG_EXT4DEV_FS_XATTR)	+= xattr.o xattr_user.o xattr_trusted.o
+ext4dev-$(CONFIG_EXT4DEV_FS_POSIX_ACL)	+= acl.o
+ext4dev-$(CONFIG_EXT4DEV_FS_SECURITY)	+= xattr_security.o
--- linux-2618-rc4-ext4.orig/fs/ext4/acl.h
+++ linux-2618-rc4-ext4/fs/ext4/acl.h
@@ -51,7 +51,7 @@ static inline int ext4_acl_count(size_t 
 	}
 }
 
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 
 /* Value for inode->u.ext4_i.i_acl and inode->u.ext4_i.i_default_acl
    if the ACL has not been cached */
@@ -62,7 +62,7 @@ extern int ext4_permission (struct inode
 extern int ext4_acl_chmod (struct inode *);
 extern int ext4_init_acl (handle_t *, struct inode *, struct inode *);
 
-#else  /* CONFIG_EXT3DEV_FS_POSIX_ACL */
+#else  /* CONFIG_EXT4DEV_FS_POSIX_ACL */
 #include <linux/sched.h>
 #define ext4_permission NULL
 
@@ -77,5 +77,5 @@ ext4_init_acl(handle_t *handle, struct i
 {
 	return 0;
 }
-#endif  /* CONFIG_EXT3DEV_FS_POSIX_ACL */
+#endif  /* CONFIG_EXT4DEV_FS_POSIX_ACL */
 
--- linux-2618-rc4-ext4.orig/fs/ext4/xattr.h
+++ linux-2618-rc4-ext4/fs/ext4/xattr.h
@@ -56,7 +56,7 @@ struct ext4_xattr_entry {
 #define EXT4_XATTR_SIZE(size) \
 	(((size) + EXT4_XATTR_ROUND) & ~EXT4_XATTR_ROUND)
 
-# ifdef CONFIG_EXT3DEV_FS_XATTR
+# ifdef CONFIG_EXT4DEV_FS_XATTR
 
 extern struct xattr_handler ext4_xattr_user_handler;
 extern struct xattr_handler ext4_xattr_trusted_handler;
@@ -79,7 +79,7 @@ extern void exit_ext4_xattr(void);
 
 extern struct xattr_handler *ext4_xattr_handlers[];
 
-# else  /* CONFIG_EXT3DEV_FS_XATTR */
+# else  /* CONFIG_EXT4DEV_FS_XATTR */
 
 static inline int
 ext4_xattr_get(struct inode *inode, int name_index, const char *name,
@@ -131,9 +131,9 @@ exit_ext4_xattr(void)
 
 #define ext4_xattr_handlers	NULL
 
-# endif  /* CONFIG_EXT3DEV_FS_XATTR */
+# endif  /* CONFIG_EXT4DEV_FS_XATTR */
 
-#ifdef CONFIG_EXT3DEV_FS_SECURITY
+#ifdef CONFIG_EXT4DEV_FS_SECURITY
 extern int ext4_init_security(handle_t *handle, struct inode *inode,
 				struct inode *dir);
 #else
--- linux-2618-rc4-ext4.orig/fs/ext4/file.c
+++ linux-2618-rc4-ext4/fs/ext4/file.c
@@ -126,7 +126,7 @@ const struct file_operations ext4_file_o
 struct inode_operations ext4_file_inode_operations = {
 	.truncate	= ext4_truncate,
 	.setattr	= ext4_setattr,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
--- linux-2618-rc4-ext4.orig/fs/ext4/inode.c
+++ linux-2618-rc4-ext4/fs/ext4/inode.c
@@ -2589,7 +2589,7 @@ void ext4_read_inode(struct inode * inod
 	struct buffer_head *bh;
 	int block;
 
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	ei->i_acl = EXT4_ACL_NOT_CACHED;
 	ei->i_default_acl = EXT4_ACL_NOT_CACHED;
 #endif
--- linux-2618-rc4-ext4.orig/fs/ext4/namei.c
+++ linux-2618-rc4-ext4/fs/ext4/namei.c
@@ -1689,7 +1689,7 @@ retry:
 	err = PTR_ERR(inode);
 	if (!IS_ERR(inode)) {
 		init_special_inode(inode, inode->i_mode, rdev);
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 		inode->i_op = &ext4_special_inode_operations;
 #endif
 		err = ext4_add_nondir(handle, dentry, inode);
@@ -2364,7 +2364,7 @@ struct inode_operations ext4_dir_inode_o
 	.mknod		= ext4_mknod,
 	.rename		= ext4_rename,
 	.setattr	= ext4_setattr,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
@@ -2375,7 +2375,7 @@ struct inode_operations ext4_dir_inode_o
 
 struct inode_operations ext4_special_inode_operations = {
 	.setattr	= ext4_setattr,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
--- linux-2618-rc4-ext4.orig/fs/ext4/super.c
+++ linux-2618-rc4-ext4/fs/ext4/super.c
@@ -448,7 +448,7 @@ static struct inode *ext4_alloc_inode(st
 	ei = kmem_cache_alloc(ext4_inode_cachep, SLAB_NOFS);
 	if (!ei)
 		return NULL;
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	ei->i_acl = EXT4_ACL_NOT_CACHED;
 	ei->i_default_acl = EXT4_ACL_NOT_CACHED;
 #endif
@@ -470,7 +470,7 @@ static void init_once(void * foo, kmem_c
 	if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) ==
 	    SLAB_CTOR_CONSTRUCTOR) {
 		INIT_LIST_HEAD(&ei->i_orphan);
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 		init_rwsem(&ei->xattr_sem);
 #endif
 		mutex_init(&ei->truncate_mutex);
@@ -499,7 +499,7 @@ static void destroy_inodecache(void)
 static void ext4_clear_inode(struct inode *inode)
 {
 	struct ext4_block_alloc_info *rsv = EXT4_I(inode)->i_block_alloc_info;
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	if (EXT4_I(inode)->i_acl &&
 			EXT4_I(inode)->i_acl != EXT4_ACL_NOT_CACHED) {
 		posix_acl_release(EXT4_I(inode)->i_acl);
@@ -793,7 +793,7 @@ static int parse_options (char *options,
 		case Opt_orlov:
 			clear_opt (sbi->s_mount_opt, OLDALLOC);
 			break;
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 		case Opt_user_xattr:
 			set_opt (sbi->s_mount_opt, XATTR_USER);
 			break;
@@ -806,7 +806,7 @@ static int parse_options (char *options,
 			printk("EXT4 (no)user_xattr options not supported\n");
 			break;
 #endif
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 		case Opt_acl:
 			set_opt(sbi->s_mount_opt, POSIX_ACL);
 			break;
@@ -2683,9 +2683,9 @@ static int ext4_get_sb(struct file_syste
 	return get_sb_bdev(fs_type, flags, dev_name, data, ext4_fill_super, mnt);
 }
 
-static struct file_system_type ext3dev_fs_type = {
+static struct file_system_type ext4_fs_type = {
 	.owner		= THIS_MODULE,
-	.name		= "ext3dev",
+	.name		= "ext4dev",
 	.get_sb		= ext4_get_sb,
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
@@ -2699,7 +2699,7 @@ static int __init init_ext4_fs(void)
 	err = init_inodecache();
 	if (err)
 		goto out1;
-        err = register_filesystem(&ext3dev_fs_type);
+        err = register_filesystem(&ext4_fs_type);
 	if (err)
 		goto out;
 	return 0;
@@ -2712,7 +2712,7 @@ out1:
 
 static void __exit exit_ext4_fs(void)
 {
-	unregister_filesystem(&ext3dev_fs_type);
+	unregister_filesystem(&ext4_fs_type);
 	destroy_inodecache();
 	exit_ext4_xattr();
 }
--- linux-2618-rc4-ext4.orig/fs/ext4/symlink.c
+++ linux-2618-rc4-ext4/fs/ext4/symlink.c
@@ -34,7 +34,7 @@ struct inode_operations ext4_symlink_ino
 	.readlink	= generic_readlink,
 	.follow_link	= page_follow_link_light,
 	.put_link	= page_put_link,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
@@ -45,7 +45,7 @@ struct inode_operations ext4_symlink_ino
 struct inode_operations ext4_fast_symlink_inode_operations = {
 	.readlink	= generic_readlink,
 	.follow_link	= ext4_follow_link,
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	.setxattr	= generic_setxattr,
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
--- linux-2618-rc4-ext4.orig/fs/ext4/xattr.c
+++ linux-2618-rc4-ext4/fs/ext4/xattr.c
@@ -104,12 +104,12 @@ static struct mb_cache *ext4_xattr_cache
 
 static struct xattr_handler *ext4_xattr_handler_map[] = {
 	[EXT4_XATTR_INDEX_USER]		     = &ext4_xattr_user_handler,
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	[EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  = &ext4_xattr_acl_access_handler,
 	[EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &ext4_xattr_acl_default_handler,
 #endif
 	[EXT4_XATTR_INDEX_TRUSTED]	     = &ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT3DEV_FS_SECURITY
+#ifdef CONFIG_EXT4DEV_FS_SECURITY
 	[EXT4_XATTR_INDEX_SECURITY]	     = &ext4_xattr_security_handler,
 #endif
 };
@@ -117,11 +117,11 @@ static struct xattr_handler *ext4_xattr_
 struct xattr_handler *ext4_xattr_handlers[] = {
 	&ext4_xattr_user_handler,
 	&ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	&ext4_xattr_acl_access_handler,
 	&ext4_xattr_acl_default_handler,
 #endif
-#ifdef CONFIG_EXT3DEV_FS_SECURITY
+#ifdef CONFIG_EXT4DEV_FS_SECURITY
 	&ext4_xattr_security_handler,
 #endif
 	NULL
--- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_i.h
+++ linux-2618-rc4-ext4/include/linux/ext4_fs_i.h
@@ -103,7 +103,7 @@ struct ext4_inode_info {
 	struct ext4_block_alloc_info *i_block_alloc_info;
 
 	__u32	i_dir_start_lookup;
-#ifdef CONFIG_EXT3DEV_FS_XATTR
+#ifdef CONFIG_EXT4DEV_FS_XATTR
 	/*
 	 * Extended attributes can be read independently of the main file
 	 * data. Taking i_mutex even when reading would cause contention
@@ -113,7 +113,7 @@ struct ext4_inode_info {
 	 */
 	struct rw_semaphore xattr_sem;
 #endif
-#ifdef CONFIG_EXT3DEV_FS_POSIX_ACL
+#ifdef CONFIG_EXT4DEV_FS_POSIX_ACL
 	struct posix_acl	*i_acl;
 	struct posix_acl	*i_default_acl;
 #endif

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-14 18:05                     ` Randy.Dunlap
@ 2006-08-14 18:13                       ` Jeff Garzik
  0 siblings, 0 replies; 26+ messages in thread
From: Jeff Garzik @ 2006-08-14 18:13 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: cmm, Darrick J. Wong, Andrew Morton, linux-fsdevel, ext2-devel,
	Alex Tomas, linux-kernel

Randy.Dunlap wrote:
> On Mon, 14 Aug 2006 13:52:42 -0400 Jeff Garzik wrote:
> 
>> IMO these non-CONFIG bits should just be ext4_
> 
> Agreed.  Replacement patch below.
> 
> ---
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Rename ext3dev to ext4dev.
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>

ACK



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-11 20:57     ` Randy.Dunlap
                         ` (2 preceding siblings ...)
  2006-08-11 23:00       ` Andrew Morton
@ 2006-08-15 15:40       ` Pavel Machek
  2006-08-18 13:08         ` Andreas Dilger
  3 siblings, 1 reply; 26+ messages in thread
From: Pavel Machek @ 2006-08-15 15:40 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: Alex Tomas, Andrew Morton, cmm, linux-fsdevel, ext2-devel,
	linux-kernel

Hi!

> >  AM> - The existing comments could benefit from some rework by a
> >  AM> native English speaker.
> > 
> > could someone assist here, please?
> 
> See if this helps.
> Patch applies on top of all ext4 patches from
> http://ext2.sourceforge.net/48bitext3/patches/latest/.

> --- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_extents.h
> +++ linux-2618-rc4-ext4/include/linux/ext4_fs_extents.h
> @@ -22,29 +22,29 @@
>  #include <linux/ext4_fs.h>
>  
>  /*
> - * with AGRESSIVE_TEST defined capacity of index/leaf blocks
> - * become very little, so index split, in-depth growing and
> - * other hard changes happens much more often
> - * this is for debug purposes only
> + * With AGRESSIVE_TEST defined, the capacity of index/leaf blocks
> + * becomes very small, so index split, in-depth growing and
> + * other hard changes happen much more often.
> + * This is for debug purposes only.
>   */
>  #define AGRESSIVE_TEST_

Using _ for disabling is unusual/nasty. Can't we simply #undef it?
-- 
Thanks for all the (sleeping) penguins.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
@ 2006-08-15 21:27 Randy Dunlap
  2006-08-15 23:19 ` Mingming Cao
  0 siblings, 1 reply; 26+ messages in thread
From: Randy Dunlap @ 2006-08-15 21:27 UTC (permalink / raw)
  To: Pavel Machek, Randy.Dunlap, Alex Tomas, Andrew Morton, cmm,
	linux-fsdevel, ext2-devel, linux-kernel



> Hi!
> 
> > >  AM> - The existing comments could benefit from some rework by a
> > >  AM> native English speaker.
> > > 
> > > could someone assist here, please?
> > 
> > See if this helps.
> > Patch applies on top of all ext4 patches from
> > http://ext2.sourceforge.net/48bitext3/patches/latest/.
> 
> > --- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_extents.h
> > +++ linux-2618-rc4-ext4/include/linux/ext4_fs_extents.h
> > @@ -22,29 +22,29 @@
> >  #include <linux/ext4_fs.h>
> >  
> >  /*
> > - * with AGRESSIVE_TEST defined capacity of index/leaf blocks
> > - * become very little, so index split, in-depth growing and
> > - * other hard changes happens much more often
> > - * this is for debug purposes only
> > + * With AGRESSIVE_TEST defined, the capacity of index/leaf blocks
> > + * becomes very small, so index split, in-depth growing and
> > + * other hard changes happen much more often.
> > + * This is for debug purposes only.
> >   */
> >  #define AGRESSIVE_TEST_
> 
> Using _ for disabling is unusual/nasty. Can't we simply #undef it?

Yes, that's the right thing to do.
The ext4dev people should do that. :)

---
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-15 21:27 [Ext2-devel] [PATCH 1/9] extents for ext4 Randy Dunlap
@ 2006-08-15 23:19 ` Mingming Cao
  0 siblings, 0 replies; 26+ messages in thread
From: Mingming Cao @ 2006-08-15 23:19 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Pavel Machek, Alex Tomas, Andrew Morton, linux-fsdevel,
	ext2-devel, linux-kernel

Randy Dunlap wrote:
> 
>>Hi!
>>
>>
>>>> AM> - The existing comments could benefit from some rework by a
>>>> AM> native English speaker.
>>>>
>>>>could someone assist here, please?
>>>
>>>See if this helps.
>>>Patch applies on top of all ext4 patches from
>>>http://ext2.sourceforge.net/48bitext3/patches/latest/.
>>
>>>--- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_extents.h
>>>+++ linux-2618-rc4-ext4/include/linux/ext4_fs_extents.h
>>>@@ -22,29 +22,29 @@
>>> #include <linux/ext4_fs.h>
>>> 
>>> /*
>>>- * with AGRESSIVE_TEST defined capacity of index/leaf blocks
>>>- * become very little, so index split, in-depth growing and
>>>- * other hard changes happens much more often
>>>- * this is for debug purposes only
>>>+ * With AGRESSIVE_TEST defined, the capacity of index/leaf blocks
>>>+ * becomes very small, so index split, in-depth growing and
>>>+ * other hard changes happen much more often.
>>>+ * This is for debug purposes only.
>>>  */
>>> #define AGRESSIVE_TEST_
>>
>>Using _ for disabling is unusual/nasty. Can't we simply #undef it?
> 
> 
> Yes, that's the right thing to do.
> The ext4dev people should do that. :)
> 

Okey, I will fixed that. thanks.
> ---
> ~Randy


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Ext2-devel] [PATCH 1/9] extents for ext4
  2006-08-15 15:40       ` Pavel Machek
@ 2006-08-18 13:08         ` Andreas Dilger
  0 siblings, 0 replies; 26+ messages in thread
From: Andreas Dilger @ 2006-08-18 13:08 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Randy.Dunlap, Alex Tomas, Andrew Morton, cmm, linux-fsdevel,
	ext2-devel, linux-kernel

On Aug 15, 2006  15:40 +0000, Pavel Machek wrote:
> > --- linux-2618-rc4-ext4.orig/include/linux/ext4_fs_extents.h
> > +++ linux-2618-rc4-ext4/include/linux/ext4_fs_extents.h
> > @@ -22,29 +22,29 @@
> >  #include <linux/ext4_fs.h>
> >  
> >  /*
> > - * with AGRESSIVE_TEST defined capacity of index/leaf blocks
> > - * become very little, so index split, in-depth growing and
> > - * other hard changes happens much more often
> > - * this is for debug purposes only
> > + * With AGRESSIVE_TEST defined, the capacity of index/leaf blocks
> > + * becomes very small, so index split, in-depth growing and
> > + * other hard changes happen much more often.
> > + * This is for debug purposes only.
> >   */
> >  #define AGRESSIVE_TEST_
> 
> Using _ for disabling is unusual/nasty.

I've always thought the same.  I'd prefer just commenting out the whole
line.

> Can't we simply #undef it?

Use of #undef is not so great, since that means it isn't possible to
#define this flag in another header, on the make command-line, etc.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2006-08-18 13:08 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-15 21:27 [Ext2-devel] [PATCH 1/9] extents for ext4 Randy Dunlap
2006-08-15 23:19 ` Mingming Cao
  -- strict thread matches above, loose matches on Subject: below --
2006-08-10  1:20 Mingming Cao
2006-08-10  6:39 ` Andrew Morton
2006-08-10  9:29   ` [Ext2-devel] " Alex Tomas
2006-08-10  9:48     ` Andrew Morton
2006-08-10 10:08       ` Alex Tomas
2006-08-10 15:55       ` Zach Brown
2006-08-10 17:49     ` Randy.Dunlap
2006-08-10 19:05       ` Alex Tomas
2006-08-11 20:57     ` Randy.Dunlap
2006-08-11 21:05       ` Alex Tomas
2006-08-11 21:49       ` Mingming Cao
2006-08-11 23:00       ` Andrew Morton
2006-08-12  6:02         ` Randy.Dunlap
2006-08-12 17:43           ` Darrick J. Wong
2006-08-12 18:20             ` Randy.Dunlap
2006-08-14 16:26               ` Mingming Cao
2006-08-14 17:22                 ` Randy.Dunlap
2006-08-14 17:52                   ` Jeff Garzik
2006-08-14 18:05                     ` Randy.Dunlap
2006-08-14 18:13                       ` Jeff Garzik
2006-08-15 15:40       ` Pavel Machek
2006-08-18 13:08         ` Andreas Dilger
2006-08-10 17:17   ` Theodore Tso
2006-08-10 18:00     ` Andrew Morton
2006-08-11 22:13       ` Mingming Cao
2006-08-11 23:16         ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox