public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] do not set extents feature from the kernel
@ 2008-07-07 20:15 Eric Sandeen
  2008-07-08  2:23 ` Peng tao
  2008-07-09  3:10 ` Theodore Tso
  0 siblings, 2 replies; 4+ messages in thread
From: Eric Sandeen @ 2008-07-07 20:15 UTC (permalink / raw)
  To: ext4 development

We've talked for a while about getting rid of any feature-
setting from the kernel; this gets rid of the code which would
set the INCOMPAT_EXTENTS flag on the first file write when mounted
as ext4[dev].

With this patch, if the extents feature is not already set on disk,
then mounting as ext4 will fall back to noextents with a warning,
and if -o extents is explicitly requested, the mount will fail,
also with warning.

Does this seem like a decent approach?

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Index: linux-2.6/fs/ext4/ialloc.c
===================================================================
--- linux-2.6.orig/fs/ext4/ialloc.c	2008-07-07 10:22:00.000000000 -0500
+++ linux-2.6/fs/ext4/ialloc.c	2008-07-07 12:07:02.133292077 -0500
@@ -740,14 +740,10 @@ got:
 		goto fail_free_drop;
 
 	if (test_opt(sb, EXTENTS)) {
-		/* set extent flag only for diretory, file and normal symlink*/
+		/* set extent flag only for directory, file and normal symlink*/
 		if (S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode)) {
 			EXT4_I(inode)->i_flags |= EXT4_EXTENTS_FL;
 			ext4_ext_tree_init(handle, inode);
-			err = ext4_update_incompat_feature(handle, sb,
-					EXT4_FEATURE_INCOMPAT_EXTENTS);
-			if (err)
-				goto fail_free_drop;
 		}
 	}
 
Index: linux-2.6/fs/ext4/super.c
===================================================================
--- linux-2.6.orig/fs/ext4/super.c	2008-07-07 10:22:19.000000000 -0500
+++ linux-2.6/fs/ext4/super.c	2008-07-07 14:08:43.684291817 -0500
@@ -1309,6 +1309,13 @@ set_qf_format:
 			clear_opt(sbi->s_mount_opt, NOBH);
 			break;
 		case Opt_extents:
+			if (!EXT4_HAS_INCOMPAT_FEATURE(sb,
+					EXT4_FEATURE_INCOMPAT_EXTENTS)) {
+				ext4_warning(sb, __func__,
+					"extents feature not enabled "
+					"on this filesystem, use tune2fs\n");
+				return 0;
+			}
 			set_opt (sbi->s_mount_opt, EXTENTS);
 			break;
 		case Opt_noextents:
@@ -1919,12 +1926,18 @@ static int ext4_fill_super (struct super
 
 	/*
 	 * turn on extents feature by default in ext4 filesystem
-	 * User -o noextents to turn it off
+	 * only if feature flag already set by mkfs or tune2fs.
+	 * Use -o noextents to turn it off
 	 */
-	set_opt(sbi->s_mount_opt, EXTENTS);
+	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_EXTENTS))
+		set_opt(sbi->s_mount_opt, EXTENTS);
+	else
+		ext4_warning(sb, __func__,
+			"extents feature not enabled on this filesystem, "
+			"use tune2fs.\n");
 	/*
-	 * turn on mballoc feature by default in ext4 filesystem
-	 * User -o nomballoc to turn it off
+	 * turn on mballoc code by default in ext4 filesystem
+	 * Use -o nomballoc to turn it off
 	 */
 	set_opt(sbi->s_mount_opt, MBALLOC);
 


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

* Re: [PATCH] do not set extents feature from the kernel
  2008-07-07 20:15 [PATCH] do not set extents feature from the kernel Eric Sandeen
@ 2008-07-08  2:23 ` Peng tao
  2008-07-08  2:25   ` Eric Sandeen
  2008-07-09  3:10 ` Theodore Tso
  1 sibling, 1 reply; 4+ messages in thread
From: Peng tao @ 2008-07-08  2:23 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: ext4 development

Hi, Eric

On Tue, Jul 8, 2008 at 4:15 AM, Eric Sandeen <sandeen@redhat.com> wrote:
> We've talked for a while about getting rid of any feature-
> setting from the kernel; this gets rid of the code which would
> set the INCOMPAT_EXTENTS flag on the first file write when mounted
> as ext4[dev].
>
> With this patch, if the extents feature is not already set on disk,
> then mounting as ext4 will fall back to noextents with a warning,
> and if -o extents is explicitly requested, the mount will fail,
> also with warning.
Does this mean no more migration from ext3 to ext4?
>
> Does this seem like a decent approach?
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> Index: linux-2.6/fs/ext4/ialloc.c
> ===================================================================
> --- linux-2.6.orig/fs/ext4/ialloc.c     2008-07-07 10:22:00.000000000 -0500
> +++ linux-2.6/fs/ext4/ialloc.c  2008-07-07 12:07:02.133292077 -0500
> @@ -740,14 +740,10 @@ got:
>                goto fail_free_drop;
>
>        if (test_opt(sb, EXTENTS)) {
> -               /* set extent flag only for diretory, file and normal symlink*/
> +               /* set extent flag only for directory, file and normal symlink*/
>                if (S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode)) {
>                        EXT4_I(inode)->i_flags |= EXT4_EXTENTS_FL;
>                        ext4_ext_tree_init(handle, inode);
> -                       err = ext4_update_incompat_feature(handle, sb,
> -                                       EXT4_FEATURE_INCOMPAT_EXTENTS);
> -                       if (err)
> -                               goto fail_free_drop;
>                }
>        }
>
> Index: linux-2.6/fs/ext4/super.c
> ===================================================================
> --- linux-2.6.orig/fs/ext4/super.c      2008-07-07 10:22:19.000000000 -0500
> +++ linux-2.6/fs/ext4/super.c   2008-07-07 14:08:43.684291817 -0500
> @@ -1309,6 +1309,13 @@ set_qf_format:
>                        clear_opt(sbi->s_mount_opt, NOBH);
>                        break;
>                case Opt_extents:
> +                       if (!EXT4_HAS_INCOMPAT_FEATURE(sb,
> +                                       EXT4_FEATURE_INCOMPAT_EXTENTS)) {
> +                               ext4_warning(sb, __func__,
> +                                       "extents feature not enabled "
> +                                       "on this filesystem, use tune2fs\n");
> +                               return 0;
> +                       }
>                        set_opt (sbi->s_mount_opt, EXTENTS);
>                        break;
>                case Opt_noextents:
> @@ -1919,12 +1926,18 @@ static int ext4_fill_super (struct super
>
>        /*
>         * turn on extents feature by default in ext4 filesystem
> -        * User -o noextents to turn it off
> +        * only if feature flag already set by mkfs or tune2fs.
> +        * Use -o noextents to turn it off
>         */
> -       set_opt(sbi->s_mount_opt, EXTENTS);
> +       if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_EXTENTS))
> +               set_opt(sbi->s_mount_opt, EXTENTS);
> +       else
> +               ext4_warning(sb, __func__,
> +                       "extents feature not enabled on this filesystem, "
> +                       "use tune2fs.\n");
>        /*
> -        * turn on mballoc feature by default in ext4 filesystem
> -        * User -o nomballoc to turn it off
> +        * turn on mballoc code by default in ext4 filesystem
> +        * Use -o nomballoc to turn it off
>         */
>        set_opt(sbi->s_mount_opt, MBALLOC);
>
>
> --
> 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
>



-- 
RGDS

Bergwolf

................
Here lieth one whose name was writ on water.

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

* Re: [PATCH] do not set extents feature from the kernel
  2008-07-08  2:23 ` Peng tao
@ 2008-07-08  2:25   ` Eric Sandeen
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2008-07-08  2:25 UTC (permalink / raw)
  To: Peng tao; +Cc: ext4 development

Peng tao wrote:
> Hi, Eric
> 
> On Tue, Jul 8, 2008 at 4:15 AM, Eric Sandeen <sandeen@redhat.com> wrote:
>> We've talked for a while about getting rid of any feature-
>> setting from the kernel; this gets rid of the code which would
>> set the INCOMPAT_EXTENTS flag on the first file write when mounted
>> as ext4[dev].
>>
>> With this patch, if the extents feature is not already set on disk,
>> then mounting as ext4 will fall back to noextents with a warning,
>> and if -o extents is explicitly requested, the mount will fail,
>> also with warning.
> Does this mean no more migration from ext3 to ext4?

I think it just means you need to tune2fs -O extents before you can
mount as ext4 *with* extents.

.... which is, I guess, a bit of a convoluted story, but consistent with
"don't surprise users by setting incompat flags behind their back"

-Eric

>> Does this seem like a decent approach?
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> Index: linux-2.6/fs/ext4/ialloc.c
>> ===================================================================
>> --- linux-2.6.orig/fs/ext4/ialloc.c     2008-07-07 10:22:00.000000000 -0500
>> +++ linux-2.6/fs/ext4/ialloc.c  2008-07-07 12:07:02.133292077 -0500
>> @@ -740,14 +740,10 @@ got:
>>                goto fail_free_drop;
>>
>>        if (test_opt(sb, EXTENTS)) {
>> -               /* set extent flag only for diretory, file and normal symlink*/
>> +               /* set extent flag only for directory, file and normal symlink*/
>>                if (S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode)) {
>>                        EXT4_I(inode)->i_flags |= EXT4_EXTENTS_FL;
>>                        ext4_ext_tree_init(handle, inode);
>> -                       err = ext4_update_incompat_feature(handle, sb,
>> -                                       EXT4_FEATURE_INCOMPAT_EXTENTS);
>> -                       if (err)
>> -                               goto fail_free_drop;
>>                }
>>        }
>>
>> Index: linux-2.6/fs/ext4/super.c
>> ===================================================================
>> --- linux-2.6.orig/fs/ext4/super.c      2008-07-07 10:22:19.000000000 -0500
>> +++ linux-2.6/fs/ext4/super.c   2008-07-07 14:08:43.684291817 -0500
>> @@ -1309,6 +1309,13 @@ set_qf_format:
>>                        clear_opt(sbi->s_mount_opt, NOBH);
>>                        break;
>>                case Opt_extents:
>> +                       if (!EXT4_HAS_INCOMPAT_FEATURE(sb,
>> +                                       EXT4_FEATURE_INCOMPAT_EXTENTS)) {
>> +                               ext4_warning(sb, __func__,
>> +                                       "extents feature not enabled "
>> +                                       "on this filesystem, use tune2fs\n");
>> +                               return 0;
>> +                       }
>>                        set_opt (sbi->s_mount_opt, EXTENTS);
>>                        break;
>>                case Opt_noextents:
>> @@ -1919,12 +1926,18 @@ static int ext4_fill_super (struct super
>>
>>        /*
>>         * turn on extents feature by default in ext4 filesystem
>> -        * User -o noextents to turn it off
>> +        * only if feature flag already set by mkfs or tune2fs.
>> +        * Use -o noextents to turn it off
>>         */
>> -       set_opt(sbi->s_mount_opt, EXTENTS);
>> +       if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_EXTENTS))
>> +               set_opt(sbi->s_mount_opt, EXTENTS);
>> +       else
>> +               ext4_warning(sb, __func__,
>> +                       "extents feature not enabled on this filesystem, "
>> +                       "use tune2fs.\n");
>>        /*
>> -        * turn on mballoc feature by default in ext4 filesystem
>> -        * User -o nomballoc to turn it off
>> +        * turn on mballoc code by default in ext4 filesystem
>> +        * Use -o nomballoc to turn it off
>>         */
>>        set_opt(sbi->s_mount_opt, MBALLOC);
>>
>>
>> --
>> 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
>>
> 
> 
> 


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

* Re: [PATCH] do not set extents feature from the kernel
  2008-07-07 20:15 [PATCH] do not set extents feature from the kernel Eric Sandeen
  2008-07-08  2:23 ` Peng tao
@ 2008-07-09  3:10 ` Theodore Tso
  1 sibling, 0 replies; 4+ messages in thread
From: Theodore Tso @ 2008-07-09  3:10 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: ext4 development

On Mon, Jul 07, 2008 at 03:15:16PM -0500, Eric Sandeen wrote:
> We've talked for a while about getting rid of any feature-
> setting from the kernel; this gets rid of the code which would
> set the INCOMPAT_EXTENTS flag on the first file write when mounted
> as ext4[dev].
> 
> With this patch, if the extents feature is not already set on disk,
> then mounting as ext4 will fall back to noextents with a warning,
> and if -o extents is explicitly requested, the mount will fail,
> also with warning.
> 

Looks good, I've added it to the patch queue.

						- Ted

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

end of thread, other threads:[~2008-07-09  3:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-07 20:15 [PATCH] do not set extents feature from the kernel Eric Sandeen
2008-07-08  2:23 ` Peng tao
2008-07-08  2:25   ` Eric Sandeen
2008-07-09  3:10 ` Theodore Tso

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