default to 64 bit inodes & add feature flag

PATCH In review Eric Sandeen 6 participants 12 messages Mar 7, 2012 – Mar 9, 2012 XFS filesystem

lore

Share & tools

lore

Eric Sandeen

author

2012-03-07 17:21 UTC #0 lore raw

PATCH 1/1 default to 64 bit inodes & add feature flag

From: Dave Chinner <[email protected]>

Default to allowing 64-bit inodes on the filesystem.

Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.

Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.

To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.

Signed-off-by Dave Chinner <[email protected]>

---

Passing this along to revive the old discussion ... Thanks,
-Eric

fs/xfs/xfs_ialloc.c +13 -0

index dad1a31..c80790e 100644
@@ -1011,6 +1011,19 @@ alloc_inode:
 	xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
 	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
 	xfs_perag_put(pag);
+
+	/* set the inode64 feature bit if necessary */
+	if (ino > XFS_MAXINUMBER_32 &&
+	    !xfs_sb_version_hasinode64(&mp->m_sb)) {
+		spin_lock(&mp->m_sb_lock);
+		if (!xfs_sb_version_hasinode64(&mp->m_sb)) {
+			xfs_sb_version_addinode64(&mp->m_sb);
+			spin_unlock(&mp->m_sb_lock);
+			xfs_mod_sb(tp, XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);
+		} else
+			spin_unlock(&mp->m_sb_lock);
+	}
+
 	*inop = ino;
 	return 0;
 error1:

fs/xfs/xfs_sb.h +13 -0

index cb6ae71..5e28c99 100644
@@ -80,6 +80,7 @@ struct xfs_mount;
 #define XFS_SB_VERSION2_RESERVED4BIT	0x00000004
 #define XFS_SB_VERSION2_ATTR2BIT	0x00000008	/* Inline attr rework */
 #define XFS_SB_VERSION2_PARENTBIT	0x00000010	/* parent pointers */
+#define XFS_SB_VERSION2_INODE64		0x00000020	/* 64 bit inodes */
 #define XFS_SB_VERSION2_PROJID32BIT	0x00000080	/* 32 bit project id */
 
 #define	XFS_SB_VERSION2_OKREALFBITS	\

@@ -503,6 +504,18 @@ static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp)
 		(sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT);
 }
 
+static inline int xfs_sb_version_hasinode64(xfs_sb_t *sbp)
+{
+	return xfs_sb_version_hasmorebits(sbp) &&
+		(sbp->sb_features2 & XFS_SB_VERSION2_INODE64);
+}
+
+static inline void xfs_sb_version_addinode64(xfs_sb_t *sbp)
+{
+	sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT;
+	sbp->sb_features2 |= XFS_SB_VERSION2_INODE64;
+}
+
 /*
  * end of superblock version macros
  */

fs/xfs/xfs_super.c +13 -1

index ee5b695..376a12d 100644
@@ -88,6 +88,7 @@ mempool_t *xfs_ioend_pool;
 #define MNTOPT_BARRIER	"barrier"	/* use writer barriers for log write and
 					 * unwritten extent conversion */
 #define MNTOPT_NOBARRIER "nobarrier"	/* .. disable */
+#define MNTOPT_32BITINODE   "inode32"	/* inodes allowed in first 32 bits */
 #define MNTOPT_64BITINODE   "inode64"	/* inodes can be allocated anywhere */
 #define MNTOPT_IKEEP	"ikeep"		/* do not free empty inode clusters */
 #define MNTOPT_NOIKEEP	"noikeep"	/* free empty inode clusters */

@@ -198,7 +199,8 @@ xfs_parseargs(
 	 */
 	mp->m_flags |= XFS_MOUNT_BARRIER;
 	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
-	mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+	if (!xfs_sb_version_hasinode64(&mp->m_sb))
+		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
 
 	/*
 	 * These can be overridden by the mount option parsing.

@@ -295,6 +297,15 @@ xfs_parseargs(
 				return EINVAL;
 			}
 			dswidth = simple_strtoul(value, &eov, 10);
+		} else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
+			if (xfs_sb_version_hasinode64(&mp->m_sb)) {
+				xfs_warn(mp,
+					"XFS: 64 bit inodes present. "
+					"%s option not allowed on this system",
+					this_char);
+				return EINVAL;
+			}
+			mp->m_flags |= ~XFS_MOUNT_SMALL_INUMS;
 		} else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
 			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
 #if !XFS_BIG_INUMS

@@ -494,6 +505,7 @@ xfs_showargs(
 		{ XFS_MOUNT_FILESTREAMS,	"," MNTOPT_FILESTREAM },
 		{ XFS_MOUNT_GRPID,		"," MNTOPT_GRPID },
 		{ XFS_MOUNT_DISCARD,		"," MNTOPT_DISCARD },
+		{ XFS_MOUNT_SMALL_INUMS, 	"," MNTOPT_32BITINODE },
 		{ 0, NULL }
 	};
 	static struct proc_xfs_info xfs_info_unset[] = {

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Josef 'Jeff' Sipek

2012-03-07 17:33 UTC #1 lore raw

Quoted 12 lines · Eric Sandeen

From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.

Quoted 5 lines

To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>

Love it! Just last night, I ended up with a filesystem (x86_64, ~36TB, RAID
60) that started giving me ENOSPC for no reason (~1% disk utilization). Eric pointed out that I forgot to use inode64. (For whatever reason, I thought that a 64-bit system would implicitly use 64-bit inodes. We'll *never* mount this fs on a 32-bit system, so I don't care about the "incompatibility".)

I'm liking the feature-bit approach.

Jeff.

Quoted 102 lines

fs/xfs/xfs_ialloc.c +13 -0

index dad1a31..c80790e 100644
@@ -1011,6 +1011,19 @@ alloc_inode:
 	xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
 	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
 	xfs_perag_put(pag);
+
+	/* set the inode64 feature bit if necessary */
+	if (ino > XFS_MAXINUMBER_32 &&
+	    !xfs_sb_version_hasinode64(&mp->m_sb)) {
+		spin_lock(&mp->m_sb_lock);
+		if (!xfs_sb_version_hasinode64(&mp->m_sb)) {
+			xfs_sb_version_addinode64(&mp->m_sb);
+			spin_unlock(&mp->m_sb_lock);
+			xfs_mod_sb(tp, XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);
+		} else
+			spin_unlock(&mp->m_sb_lock);
+	}
+
 	*inop = ino;
 	return 0;
 error1:

fs/xfs/xfs_sb.h +13 -0

index cb6ae71..5e28c99 100644
@@ -80,6 +80,7 @@ struct xfs_mount;
 #define XFS_SB_VERSION2_RESERVED4BIT	0x00000004
 #define XFS_SB_VERSION2_ATTR2BIT	0x00000008	/* Inline attr rework */
 #define XFS_SB_VERSION2_PARENTBIT	0x00000010	/* parent pointers */
+#define XFS_SB_VERSION2_INODE64		0x00000020	/* 64 bit inodes */
 #define XFS_SB_VERSION2_PROJID32BIT	0x00000080	/* 32 bit project id */
 
 #define	XFS_SB_VERSION2_OKREALFBITS	\

@@ -503,6 +504,18 @@ static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp)
 		(sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT);
 }
 
+static inline int xfs_sb_version_hasinode64(xfs_sb_t *sbp)
+{
+	return xfs_sb_version_hasmorebits(sbp) &&
+		(sbp->sb_features2 & XFS_SB_VERSION2_INODE64);
+}
+
+static inline void xfs_sb_version_addinode64(xfs_sb_t *sbp)
+{
+	sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT;
+	sbp->sb_features2 |= XFS_SB_VERSION2_INODE64;
+}
+
 /*
  * end of superblock version macros
  */

fs/xfs/xfs_super.c +13 -1

index ee5b695..376a12d 100644
@@ -88,6 +88,7 @@ mempool_t *xfs_ioend_pool;
 #define MNTOPT_BARRIER	"barrier"	/* use writer barriers for log write and
 					 * unwritten extent conversion */
 #define MNTOPT_NOBARRIER "nobarrier"	/* .. disable */
+#define MNTOPT_32BITINODE   "inode32"	/* inodes allowed in first 32 bits */
 #define MNTOPT_64BITINODE   "inode64"	/* inodes can be allocated anywhere */
 #define MNTOPT_IKEEP	"ikeep"		/* do not free empty inode clusters */
 #define MNTOPT_NOIKEEP	"noikeep"	/* free empty inode clusters */

@@ -198,7 +199,8 @@ xfs_parseargs(
 	 */
 	mp->m_flags |= XFS_MOUNT_BARRIER;
 	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
-	mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+	if (!xfs_sb_version_hasinode64(&mp->m_sb))
+		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
 
 	/*
 	 * These can be overridden by the mount option parsing.

@@ -295,6 +297,15 @@ xfs_parseargs(
 				return EINVAL;
 			}
 			dswidth = simple_strtoul(value, &eov, 10);
+		} else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
+			if (xfs_sb_version_hasinode64(&mp->m_sb)) {
+				xfs_warn(mp,
+					"XFS: 64 bit inodes present. "
+					"%s option not allowed on this system",
+					this_char);
+				return EINVAL;
+			}
+			mp->m_flags |= ~XFS_MOUNT_SMALL_INUMS;
 		} else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
 			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
 #if !XFS_BIG_INUMS

@@ -494,6 +505,7 @@ xfs_showargs(
 		{ XFS_MOUNT_FILESTREAMS,	"," MNTOPT_FILESTREAM },
 		{ XFS_MOUNT_GRPID,		"," MNTOPT_GRPID },
 		{ XFS_MOUNT_DISCARD,		"," MNTOPT_DISCARD },
+		{ XFS_MOUNT_SMALL_INUMS, 	"," MNTOPT_32BITINODE },
 		{ 0, NULL }
 	};
 	static struct proc_xfs_info xfs_info_unset[] = {

Signature · 9 lines

I have always wished for my computer to be as easy to use as my telephone;
my wish has come true because I can no longer figure out how to use my
telephone.
		- Bjarne Stroustrup

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Arkadiusz Miśkiewicz

2012-03-07 18:07 UTC #2 lore raw

Quoted 14 lines · Josef 'Jeff' Sipek

Quoted 13 lines · Eric Sandeen
From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.

Quoted 7 lines

Quoted 5 lines
To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>
Love it!

+1 for love. I had too many problems with forgetting to mount with inode64 option.

Signature · 7 lines

Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Dave Chinner

2012-03-08 01:34 UTC #3 lore raw

In reply to Eric Sandeen

Quoted 12 lines · Eric Sandeen

From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.

Quoted 8 lines

To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>
---
Passing this along to revive the old discussion ...

I have no objections to do this. However, the kernel patch is just the tip of the iceberg when it comes to implementing this.

Were there patches for userspace support of the feature bit? I don't recall if there were. I'm thinking that xfs_info needs to output whether this is set, which means the flag needs to be added to the xfs geometry ioctls in the kernel.

I'd also think that this should also be made a mkfs option (it will have to default to "not enabled" for a couple of years until distro kernels catch up) along with outputting the current value. This will then require xfstests mkfs filter changes, too.

We'll also need xfs_check, xfs_db (e.g. the version command) and xfs_repair knowledge of the new feature bit so they don't think the superblock is corrupted when this new bit gets set....

Cheers,

Dave.

Signature · 7 lines

Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2012-03-08 02:05 UTC #4 lore raw

Quoted 22 lines · Dave Chinner

Quoted 12 lines · Eric Sandeen
From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.
Quoted 8 lines
To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>
---
Passing this along to revive the old discussion ...

Quoted 7 lines

I have no objections to do this. However, the kernel patch is just the tip of the iceberg when it comes to implementing this.
Were there patches for userspace support of the feature bit? I don't recall if there were. I'm thinking that xfs_info needs to output whether this is set, which means the flag needs to be added to the xfs geometry ioctls in the kernel.

Nope, you just put this patch out as a suggestion, and pointed out that userspace needed updates too.

If people are in agreement about this then we can proceed with the rest...

-Eric

Quoted 5 lines

I'd also think that this should also be made a mkfs option (it will have to default to "not enabled" for a couple of years until distro kernels catch up) along with outputting the current value. This will then require xfstests mkfs filter changes, too.

Quoted 8 lines

We'll also need xfs_check, xfs_db (e.g. the version command) and xfs_repair knowledge of the new feature bit so they don't think the superblock is corrupted when this new bit gets set....
Cheers,
Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Ben Myers

2012-03-08 15:37 UTC #5 lore raw

Quoted 31 lines · Eric Sandeen

Quoted 22 lines · Dave Chinner
Quoted 12 lines · Eric Sandeen
From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.
Quoted 8 lines
To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>
---
Passing this along to revive the old discussion ...
Quoted 7 lines
I have no objections to do this. However, the kernel patch is just the tip of the iceberg when it comes to implementing this.
Were there patches for userspace support of the feature bit? I don't recall if there were. I'm thinking that xfs_info needs to output whether this is set, which means the flag needs to be added to the xfs geometry ioctls in the kernel.

Quoted 4 lines

Nope, you just put this patch out as a suggestion, and pointed out that userspace needed updates too.
If people are in agreement about this then we can proceed with the rest...

Please do. I too have been burned by mounting a filesystem with big inos without the correct mount option. This is a great idea.

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2012-03-08 15:42 UTC #6 lore raw

Quoted 32 lines · Ben Myers

Quoted 31 lines · Eric Sandeen
Quoted 29 lines · Dave Chinner
Quoted 12 lines · Eric Sandeen
From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.
Quoted 8 lines
To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>
---
Passing this along to revive the old discussion ...
I have no objections to do this. However, the kernel patch is just the tip of the iceberg when it comes to implementing this.
Were there patches for userspace support of the feature bit? I don't recall if there were. I'm thinking that xfs_info needs to output whether this is set, which means the flag needs to be added to the xfs geometry ioctls in the kernel.

Quoted 7 lines

Quoted 4 lines
Nope, you just put this patch out as a suggestion, and pointed out that userspace needed updates too.
If people are in agreement about this then we can proceed with the rest...
Please do. I too have been burned by mounting a filesystem with big inos without the correct mount option. This is a great idea.

So, after thinking about this (and talking on irc) some more, I'm not convinced that a feature flag is the way to go.

If we set a feature flag, suddenly old filesystems with 64-bit inodes will grow a new feature, and this will force a userspace upgrade - but there is no real new feature. This seems like a bad idea. My original patch (which Dave responded to with this one) simply made inode64 default, with no feature flags.

Unless someone has a really compelling argument for the flag, I'm thinking this is the wrong approach after all.

Perhaps I should resend the just-make-it-default patch.

Comments?

-Eric

Quoted 2 lines

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Josef 'Jeff' Sipek

2012-03-08 16:14 UTC #7 lore raw

Quoted 9 lines · Eric Sandeen

So, after thinking about this (and talking on irc) some more, I'm not convinced that a feature flag is the way to go.
If we set a feature flag, suddenly old filesystems with 64-bit inodes will grow a new feature, and this will force a userspace upgrade - but there is no real new feature. This seems like a bad idea. My original patch (which Dave responded to with this one) simply made inode64 default, with no feature flags.

Quoted 6 lines

Unless someone has a really compelling argument for the flag, I'm thinking this is the wrong approach after all.
Perhaps I should resend the just-make-it-default patch.
Comments?

I was thinking about this sort of scenario. You are right, there's no on-disk format change. My initial thought about how to handle this was to just make inode64 the default on 64-bit builds. I think the feature flag idea is good because it essentially acts as a taint flag - much like the attr2 feature flag. The difference is, in the inode64 case...

1) it's the same on-disk format
2) there are years of ambiguous-inode filesystems out there

Out of curiosity...is there a reason we can't do both? Default to 64-bit, and slowly introduce the 64-bit inodes feature flag?

Jeff.

Signature · 7 lines

What is the difference between Mechanical Engineers and Civil Engineers?
Mechanical Engineers build weapons, Civil Engineers build targets.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Ben Myers

2012-03-08 16:38 UTC #8 lore raw

In reply to Eric Sandeen

Quoted 50 lines · Eric Sandeen

Quoted 32 lines · Ben Myers
Quoted 31 lines · Eric Sandeen
Quoted 29 lines · Dave Chinner
Quoted 12 lines · Eric Sandeen
From: Dave Chinner <[email protected]>
Default to allowing 64-bit inodes on the filesystem.
Add a feature bit to the the superblock to record whether 64 bit inodes have been allocated on the filesystem or not. This allows us to reject mounting the filesytem with inode32 if 64 bit inodes are present.
Once a 64 bitinode is allocated, the inode64 superblock feature bit will be set. Once the superblock feature bit is set, the filesystem will default to 64 bit inodes regardless of whether inode64 is specified as a mount option.
Quoted 8 lines
To ensure only 32 bit inodes are created, the inode32 mount option must be used. If there are already 64 bit inodes as flagged by the superblock feature bit, then the inode32 mount will be refused.
Signed-off-by Dave Chinner <[email protected]>
---
Passing this along to revive the old discussion ...
I have no objections to do this. However, the kernel patch is just the tip of the iceberg when it comes to implementing this.
Were there patches for userspace support of the feature bit? I don't recall if there were. I'm thinking that xfs_info needs to output whether this is set, which means the flag needs to be added to the xfs geometry ioctls in the kernel.
Quoted 7 lines
Quoted 4 lines
Nope, you just put this patch out as a suggestion, and pointed out that userspace needed updates too.
If people are in agreement about this then we can proceed with the rest...
Please do. I too have been burned by mounting a filesystem with big inos without the correct mount option. This is a great idea.
So, after thinking about this (and talking on irc) some more, I'm not convinced that a feature flag is the way to go.
If we set a feature flag, suddenly old filesystems with 64-bit inodes will grow a new feature, and this will force a userspace upgrade - but there is no real new feature. This seems like a bad idea. My original patch (which Dave responded to with this one) simply made inode64 default, with no feature flags.

Quoted 6 lines

Unless someone has a really compelling argument for the flag, I'm thinking this is the wrong approach after all.
Perhaps I should resend the just-make-it-default patch.
Comments?

Ew! Forcing a userspace upgrade is not desireable. Since we would only want to set the feature bit if userspace were already upgraded, and only if there are 64 bit inos... How about two bits: one is set by mkfs and checked by the kernel to see if it is ok to set the other. ;)

The first bit could also act as 'now its ok to default to inode64'.

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Dave Chinner

2012-03-08 23:39 UTC #9 lore raw

Quoted 15 lines · Ben Myers

Quoted 14 lines · Eric Sandeen
So, after thinking about this (and talking on irc) some more, I'm not convinced that a feature flag is the way to go.
If we set a feature flag, suddenly old filesystems with 64-bit inodes will grow a new feature, and this will force a userspace upgrade - but there is no real new feature. This seems like a bad idea. My original patch (which Dave responded to with this one) simply made inode64 default, with no feature flags.
Unless someone has a really compelling argument for the flag, I'm thinking this is the wrong approach after all.
Perhaps I should resend the just-make-it-default patch.

Quoted 8 lines

Quoted 1 line
Comments?
Ew! Forcing a userspace upgrade is not desireable. Since we would only want to set the feature bit if userspace were already upgraded, and only if there are 64 bit inos... How about two bits: one is set by mkfs and checked by the kernel to see if it is ok to set the other. ;)
The first bit could also act as 'now its ok to default to inode64'.

Too complex, IMO. Just add an xfs_admin command to set the inode64 feature bit. That then overrides the inode64/inode32 mount option, and guarantees that the user has already upgraded userspace.

i.e. the mount options are only valid if the feature bit it not set, and the feature bit can only be set via xfs_admin after a userspace upgrade. Kernels that don't understand the feature bit will refuse to mount, keeping in line with the current practise of requiring both kernel and userspace upgrades to occur in step to use new features....

Cheers,

Dave.

Signature · 7 lines

Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2012-03-08 23:41 UTC #10 lore raw

Quoted 29 lines · Dave Chinner

Quoted 15 lines · Ben Myers
Quoted 14 lines · Eric Sandeen
So, after thinking about this (and talking on irc) some more, I'm not convinced that a feature flag is the way to go.
If we set a feature flag, suddenly old filesystems with 64-bit inodes will grow a new feature, and this will force a userspace upgrade - but there is no real new feature. This seems like a bad idea. My original patch (which Dave responded to with this one) simply made inode64 default, with no feature flags.
Unless someone has a really compelling argument for the flag, I'm thinking this is the wrong approach after all.
Perhaps I should resend the just-make-it-default patch.
Quoted 8 lines
Quoted 1 line
Comments?
Ew! Forcing a userspace upgrade is not desireable. Since we would only want to set the feature bit if userspace were already upgraded, and only if there are 64 bit inos... How about two bits: one is set by mkfs and checked by the kernel to see if it is ok to set the other. ;)
The first bit could also act as 'now its ok to default to inode64'.
Too complex, IMO. Just add an xfs_admin command to set the inode64 feature bit. That then overrides the inode64/inode32 mount option, and guarantees that the user has already upgraded userspace.

Quoted 6 lines

i.e. the mount options are only valid if the feature bit it not set, and the feature bit can only be set via xfs_admin after a userspace upgrade. Kernels that don't understand the feature bit will refuse to mount, keeping in line with the current practise of requiring both kernel and userspace upgrades to occur in step to use new features....

Yep, I think that's the right path forward (had been thinking along these lines too, today).

Thanks,
-Eric

Quoted 3 lines

Cheers,
Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Ben Myers

2012-03-09 02:08 UTC #11 lore raw

In reply to Dave Chinner

Hi Dave,

Quoted 29 lines · Dave Chinner

Quoted 15 lines · Ben Myers
Quoted 14 lines · Eric Sandeen
So, after thinking about this (and talking on irc) some more, I'm not convinced that a feature flag is the way to go.
If we set a feature flag, suddenly old filesystems with 64-bit inodes will grow a new feature, and this will force a userspace upgrade - but there is no real new feature. This seems like a bad idea. My original patch (which Dave responded to with this one) simply made inode64 default, with no feature flags.
Unless someone has a really compelling argument for the flag, I'm thinking this is the wrong approach after all.
Perhaps I should resend the just-make-it-default patch.
Quoted 8 lines
Quoted 1 line
Comments?
Ew! Forcing a userspace upgrade is not desireable. Since we would only want to set the feature bit if userspace were already upgraded, and only if there are 64 bit inos... How about two bits: one is set by mkfs and checked by the kernel to see if it is ok to set the other. ;)
The first bit could also act as 'now its ok to default to inode64'.
Too complex, IMO. Just add an xfs_admin command to set the inode64 feature bit. That then overrides the inode64/inode32 mount option, and guarantees that the user has already upgraded userspace.

Quoted 6 lines

i.e. the mount options are only valid if the feature bit it not set, and the feature bit can only be set via xfs_admin after a userspace upgrade. Kernels that don't understand the feature bit will refuse to mount, keeping in line with the current practise of requiring both kernel and userspace upgrades to occur in step to use new features....

When I hit this problem it was because I had mounted the filesystem w/ inode64 and created large inos, and then forgot all about it sometime later and didn't use the mount option. Unless the inode64 mount option is completely disabled in favor of using the 'override inode*' feature bit exclusively, this aspect of the problem is not resolved.

That'd be just fine, imo. I wonder if there are others that would be better tracked in the superblock than as mount options. 'dmi' comes to mind.

Regards,
Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs