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
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" <[email protected]>
---
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);
On 03/24/2010 11:12 AM, [email protected] 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
On 03/24/2010 11:27 AM, [email protected] 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
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, <[email protected]> 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" <[email protected]>
> ---
> 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);
>
On Wed, Mar 24, 2010 at 11:21:59AM -0500, Eric Sandeen wrote:
> On 03/24/2010 11:12 AM, [email protected] 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
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