[PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[Sedat Dilek]

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

Hi,

today I pulled ext4 for_linus GIT branch into linus-tree (2.6.24-rc2-git1).

Unfortunately, the build breaks here:
...
  CC [M]  fs/ext4/super.o
  CC [M]  sound/isa/wavefront/wavefront_fx.o
  CC [M]  drivers/ata/pata_acpi.o
fs/ext4/super.c: In function ‘ext4_fill_super’:
fs/ext4/super.c:2553: error: ‘ext3_fs_type’ undeclared (first use in
this function)
fs/ext4/super.c:2553: error: (Each undeclared identifier is reported only once
fs/ext4/super.c:2553: error: for each function it appears in.)
make[5]: *** [fs/ext4/super.o] Error 1
...

While investigating the problem I found several typos in fs/ext4/super.c.
One of them was introduced with:

commit 2c7532a9b5fc3c27775f59e28d5f1ee4b1a1052e
"ext4: Don't use delayed allocation by default when used instead of ext3"

Furthermore, I have set the following CONFIG_EXT*FS* parameters:

$ zgrep CONFIG_EXT /proc/config.gz | grep FS
CONFIG_EXT2_FS=m
CONFIG_EXT2_FS_XATTR=y
CONFIG_EXT2_FS_POSIX_ACL=y
CONFIG_EXT2_FS_SECURITY=y
# CONFIG_EXT2_FS_XIP is not set
CONFIG_EXT3_FS=m
CONFIG_EXT3_FS_XATTR=y
CONFIG_EXT3_FS_POSIX_ACL=y
CONFIG_EXT3_FS_SECURITY=y
CONFIG_EXT4_FS=m
CONFIG_EXT4_FS_XATTR=y
CONFIG_EXT4_FS_POSIX_ACL=y
CONFIG_EXT4_FS_SECURITY=y

My patch is not fixing the problem, just the typos.
Please have a closer look into it, thanks.

--
Sedat

[1] http://git.kernel.org/?p=linux/kernel/git/tytso/ext4.git;a=commit;h=2c7532a9b5fc3c27775f59e28d5f1ee4b1a1052e

[-- Attachment #2: 0001-ext4-super.c-Replace-CONTIG_EXT-2-3-_FS-by-CONFIG_EX.patch --]
[-- Type: text/x-diff, Size: 2004 bytes --]

From a3d5886d3cdaadcdc154413347f20ca2bdb5c615 Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Wed, 24 Mar 2010 11:21:24 +0100
Subject: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

One typo was introduced with:

commit 2c7532a9b5fc3c27775f59e28d5f1ee4b1a1052e
"ext4: Don't use delayed allocation by default when used instead of ext3"
---
 fs/ext4/super.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 1a1ccd4..1dbb37a 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -71,7 +71,7 @@ static int ext4_freeze(struct super_block *sb);
 static int ext4_get_sb(struct file_system_type *fs_type, int flags,
 		       const char *dev_name, void *data, struct vfsmount *mnt);
 
-#if !defined(CONTIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
 static struct file_system_type ext3_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ext3",
@@ -4080,7 +4080,7 @@ static int ext4_get_sb(struct file_system_type *fs_type, int flags,
 	return get_sb_bdev(fs_type, flags, dev_name, data, ext4_fill_super,mnt);
 }
 
-#if !defined(CONTIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
 static struct file_system_type ext2_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ext2",
@@ -4107,7 +4107,7 @@ static inline void register_as_ext2(void) { }
 static inline void unregister_as_ext2(void) { }
 #endif
 
-#if !defined(CONTIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
 static inline void register_as_ext3(void)
 {
 	int err = register_filesystem(&ext3_fs_type);
-- 
1.7.0.3

Re: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[tytso]

Thanks for catching this!  This should fix it up...

       	   	    	   	       	   - Ted

ext4: Fix build error when EXT4_USE_FOR_EXT23 disabled

Fix a stupid type (CONTIG != CONFIG) and a stupid assumption that
ext3_fs_type would always be defined.

(The build failure could also happen when EXT4_USE_FOR_EXT23 is
enabled and ext3 is built as a module.)

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3d1a056..29c6875 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -71,7 +71,7 @@ static int ext4_freeze(struct super_block *sb);
 static int ext4_get_sb(struct file_system_type *fs_type, int flags,
 		       const char *dev_name, void *data, struct vfsmount *mnt);
 
-#if !defined(CONTIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
 static struct file_system_type ext3_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ext3",
@@ -79,6 +79,9 @@ static struct file_system_type ext3_fs_type = {
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
+#define IS_EXT3_SB(sb) ((sb)->s_bdev->bd_holder == &ext3_fs_type)
+#else
+#define IS_EXT3_SB(sb) (0)
 #endif
 
 ext4_fsblk_t ext4_block_bitmap(struct super_block *sb,
@@ -2550,7 +2553,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * enable delayed allocation by default
 	 * Use -o nodelalloc to turn it off
 	 */
-	if (sb->s_bdev->bd_holder != &ext3_fs_type)
+	if (!IS_EXT3_SB(sb))
 		set_opt(sbi->s_mount_opt, DELALLOC);
 
 	if (!parse_options((char *) data, sb, &journal_devnum,
@@ -4080,7 +4083,7 @@ static int ext4_get_sb(struct file_system_type *fs_type, int flags,
 	return get_sb_bdev(fs_type, flags, dev_name, data, ext4_fill_super,mnt);
 }
 
-#if !defined(CONTIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
 static struct file_system_type ext2_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ext2",
@@ -4107,7 +4110,7 @@ static inline void register_as_ext2(void) { }
 static inline void unregister_as_ext2(void) { }
 #endif
 
-#if !defined(CONTIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
 static inline void register_as_ext3(void)
 {
 	int err = register_filesystem(&ext3_fs_type);

Re: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[Eric Sandeen]

On 03/24/2010 11:12 AM, tytso@mit.edu wrote:
> Thanks for catching this!  This should fix it up...
> 
>        	   	    	   	       	   - Ted
> 
> ext4: Fix build error when EXT4_USE_FOR_EXT23 disabled
> 
> Fix a stupid type (CONTIG != CONFIG) and a stupid assumption that
> ext3_fs_type would always be defined.
> 
> (The build failure could also happen when EXT4_USE_FOR_EXT23 is
> enabled and ext3 is built as a module.)

A little late, but right now we have this in Kconfig:

config EXT4_USE_FOR_EXT23
        bool "Use ext4 for ext2/ext3 file systems"
        depends on EXT4_FS
        depends on EXT3_FS=n || EXT2_FS=n

do we really want "||" ?

so, if you turn off ext2, but leave ext3 on, it will ask you if you want
to use ext4 for ext3?  That doesn't make sense to me.

Shouldn't it be:

-        depends on EXT3_FS=n || EXT2_FS=n
+        depends on EXT3_FS=n && EXT2_FS=n

?

-Eric

Re: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[tytso]

On Wed, Mar 24, 2010 at 11:21:59AM -0500, Eric Sandeen wrote:
> On 03/24/2010 11:12 AM, tytso@mit.edu wrote:
> > Thanks for catching this!  This should fix it up...
> > 
> >        	   	    	   	       	   - Ted
> > 
> > ext4: Fix build error when EXT4_USE_FOR_EXT23 disabled
> > 
> > Fix a stupid type (CONTIG != CONFIG) and a stupid assumption that
> > ext3_fs_type would always be defined.
> > 
> > (The build failure could also happen when EXT4_USE_FOR_EXT23 is
> > enabled and ext3 is built as a module.)
> 
> A little late, but right now we have this in Kconfig:
> 
> config EXT4_USE_FOR_EXT23
>         bool "Use ext4 for ext2/ext3 file systems"
>         depends on EXT4_FS
>         depends on EXT3_FS=n || EXT2_FS=n
> 
> do we really want "||" ?
> 
> so, if you turn off ext2, but leave ext3 on, it will ask you if you want
> to use ext4 for ext3?  That doesn't make sense to me.

Well, the idea is you can use ext4 instead of ext2 or ext3.  So if
ext3 is turned on, but ext2 is off, then the option will use ext4 for
ext2, but ext3 will stay ext3.

This is explained in more detail in the help message.  I guess we
could make "ext2/ext3 filesystems" be "ext2 or ext3 filesystems" and
then add a slightly more explicit explanation in the help text....

     	   	    	 	  	      	 - Ted

Re: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[Eric Sandeen]

On 03/24/2010 11:27 AM, tytso@mit.edu wrote:
> On Wed, Mar 24, 2010 at 11:21:59AM -0500, Eric Sandeen wrote:

...

>> so, if you turn off ext2, but leave ext3 on, it will ask you if you want
>> to use ext4 for ext3?  That doesn't make sense to me.
> 
> Well, the idea is you can use ext4 instead of ext2 or ext3.  So if
> ext3 is turned on, but ext2 is off, then the option will use ext4 for
> ext2, but ext3 will stay ext3.

Oh, ok, I shoulda looked at super.c.  Got it now.  :)

Thanks,
-Eric

Re: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[Sedat Dilek]

Thanks for the fast fix - hope this goes soon into linus-tree.

(Missed a Reported-By, sorry).

--
Sedat

On Wed, Mar 24, 2010 at 5:12 PM,  <tytso@mit.edu> wrote:
> Thanks for catching this!  This should fix it up...
>
>                                           - Ted
>
> ext4: Fix build error when EXT4_USE_FOR_EXT23 disabled
>
> Fix a stupid type (CONTIG != CONFIG) and a stupid assumption that
> ext3_fs_type would always be defined.
>
> (The build failure could also happen when EXT4_USE_FOR_EXT23 is
> enabled and ext3 is built as a module.)
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
>  fs/ext4/super.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 3d1a056..29c6875 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -71,7 +71,7 @@ static int ext4_freeze(struct super_block *sb);
>  static int ext4_get_sb(struct file_system_type *fs_type, int flags,
>                       const char *dev_name, void *data, struct vfsmount *mnt);
>
> -#if !defined(CONTIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
> +#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
>  static struct file_system_type ext3_fs_type = {
>        .owner          = THIS_MODULE,
>        .name           = "ext3",
> @@ -79,6 +79,9 @@ static struct file_system_type ext3_fs_type = {
>        .kill_sb        = kill_block_super,
>        .fs_flags       = FS_REQUIRES_DEV,
>  };
> +#define IS_EXT3_SB(sb) ((sb)->s_bdev->bd_holder == &ext3_fs_type)
> +#else
> +#define IS_EXT3_SB(sb) (0)
>  #endif
>
>  ext4_fsblk_t ext4_block_bitmap(struct super_block *sb,
> @@ -2550,7 +2553,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>         * enable delayed allocation by default
>         * Use -o nodelalloc to turn it off
>         */
> -       if (sb->s_bdev->bd_holder != &ext3_fs_type)
> +       if (!IS_EXT3_SB(sb))
>                set_opt(sbi->s_mount_opt, DELALLOC);
>
>        if (!parse_options((char *) data, sb, &journal_devnum,
> @@ -4080,7 +4083,7 @@ static int ext4_get_sb(struct file_system_type *fs_type, int flags,
>        return get_sb_bdev(fs_type, flags, dev_name, data, ext4_fill_super,mnt);
>  }
>
> -#if !defined(CONTIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
> +#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
>  static struct file_system_type ext2_fs_type = {
>        .owner          = THIS_MODULE,
>        .name           = "ext2",
> @@ -4107,7 +4110,7 @@ static inline void register_as_ext2(void) { }
>  static inline void unregister_as_ext2(void) { }
>  #endif
>
> -#if !defined(CONTIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
> +#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
>  static inline void register_as_ext3(void)
>  {
>        int err = register_filesystem(&ext3_fs_type);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ext4/super.c: Replace CONTIG_EXT{2,3}_FS by CONFIG_EXT{2,3}_FS

[tytso]

On Wed, Mar 24, 2010 at 05:57:34PM +0100, Sedat Dilek wrote:
> Thanks for the fast fix - hope this goes soon into linus-tree.

So I hadn't sent the pull request to Linus, and when I was looking
over the patches, I realized that (a) the patch that broke the build
hadn't been pushed to Linus yet, and (b) I wanted to pull a patch from
the patch series since I didn't think it was appropriate at this point
in the release cycle.  So I ended up rewinding my for_linus branch,
and I'll be pushing something to Linus tonight.

Regards,

						- Ted