do not set extents feature from the kernel

PATCH In review Eric Sandeen 3 participants 4 messages Jul 7, 2008 – Jul 9, 2008 ext4 file system

Share & tools

Eric Sandeen

author

2008-07-07 20:15 UTC #0 lore raw

PATCH 1/1 do not set extents feature from the kernel

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 <[email protected]>

---

fs/ext4/ialloc.c +1 -5

Index: linux-2.6/fs/ext4/ialloc.c
===================================================================
@@ -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;
 		}
 	}

fs/ext4/super.c +17 -4

Index: linux-2.6/fs/ext4/super.c
===================================================================
@@ -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);

Peng tao

2008-07-08 02:23 UTC #1 lore raw

Hi, Eric

Quoted 9 lines · Eric Sandeen

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?

Quoted 73 lines

Does this seem like a decent approach?

Signed-off-by Eric Sandeen <[email protected]>

---

Index: linux-2.6/fs/ext4/ialloc.c
===================================================================

fs/ext4/ialloc.c 2008-07-07 12:07:02.133292077 -0500 renamed from fs/ext4/ialloc.c 2008-07-07 10:22:00.000000000 -0500 +1 -5

@@ -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
===================================================================

fs/ext4/super.c 2008-07-07 14:08:43.684291817 -0500 renamed from fs/ext4/super.c 2008-07-07 10:22:19.000000000 -0500 +17 -5

@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 6 lines

RGDS

Bergwolf

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

Eric Sandeen

author

2008-07-08 02:25 UTC #2 lore raw

Quoted 8 lines · Peng tao

Hi, Eric
Quoted 5 lines · Eric Sandeen
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].

Quoted 5 lines

Quoted 4 lines
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

Quoted 75 lines

Quoted 72 lines

Does this seem like a decent approach?

Signed-off-by Eric Sandeen <[email protected]>

---

Index: linux-2.6/fs/ext4/ialloc.c
===================================================================

fs/ext4/ialloc.c 2008-07-07 12:07:02.133292077 -0500 renamed from fs/ext4/ialloc.c 2008-07-07 10:22:00.000000000 -0500 +1 -5

@@ -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
===================================================================

fs/ext4/super.c 2008-07-07 14:08:43.684291817 -0500 renamed from fs/ext4/super.c 2008-07-07 10:22:19.000000000 -0500 +17 -5

@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Theodore Tso

maintainer

2008-07-09 03:10 UTC #3 lore raw

In reply to Eric Sandeen

Quoted 10 lines · Eric Sandeen

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