[PATCH] default to 64 bit inodes & add feature flag

[PATCH] default to 64 bit inodes & add feature flag

[Eric Sandeen]

From: Dave Chinner <dchinner@redhat.com>

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 <dchinner@redhat.com>
---

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

diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index dad1a31..c80790e 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -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:
diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
index cb6ae71..5e28c99 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -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
  */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index ee5b695..376a12d 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Josef 'Jeff' Sipek]

On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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 <dchinner@redhat.com>

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.

> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index dad1a31..c80790e 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -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:
> diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
> index cb6ae71..5e28c99 100644
> --- a/fs/xfs/xfs_sb.h
> +++ b/fs/xfs/xfs_sb.h
> @@ -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
>   */
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ee5b695..376a12d 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -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[] = {
> 

-- 
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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Arkadiusz Miśkiewicz]

On Wednesday 07 of March 2012, Josef 'Jeff' Sipek wrote:
> On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > 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 <dchinner@redhat.com>
> 
> Love it! 

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

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Dave Chinner]

On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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 <dchinner@redhat.com>
> ---
> 
> 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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Eric Sandeen]

On 3/7/12 7:34 PM, Dave Chinner wrote:
> On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>>
>> 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 <dchinner@redhat.com>
>> ---
>>
>> 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.

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

> 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.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Ben Myers]

On Wed, Mar 07, 2012 at 08:05:12PM -0600, Eric Sandeen wrote:
> On 3/7/12 7:34 PM, Dave Chinner wrote:
> > On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
> >> From: Dave Chinner <dchinner@redhat.com>
> >>
> >> 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 <dchinner@redhat.com>
> >> ---
> >>
> >> 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.
> 
> 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Eric Sandeen]

On 3/8/12 9:37 AM, Ben Myers wrote:
> On Wed, Mar 07, 2012 at 08:05:12PM -0600, Eric Sandeen wrote:
>> On 3/7/12 7:34 PM, Dave Chinner wrote:
>>> On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
>>>> From: Dave Chinner <dchinner@redhat.com>
>>>>
>>>> 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 <dchinner@redhat.com>
>>>> ---
>>>>
>>>> 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.
>>
>> 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
 
> -Ben
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Josef 'Jeff' Sipek]

On Thu, Mar 08, 2012 at 09:42:21AM -0600, Eric Sandeen wrote:
> 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?

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.

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Ben Myers]

On Thu, Mar 08, 2012 at 09:42:21AM -0600, Eric Sandeen wrote:
> On 3/8/12 9:37 AM, Ben Myers wrote:
> > On Wed, Mar 07, 2012 at 08:05:12PM -0600, Eric Sandeen wrote:
> >> On 3/7/12 7:34 PM, Dave Chinner wrote:
> >>> On Wed, Mar 07, 2012 at 11:20:57AM -0600, Eric Sandeen wrote:
> >>>> From: Dave Chinner <dchinner@redhat.com>
> >>>>
> >>>> 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 <dchinner@redhat.com>
> >>>> ---
> >>>>
> >>>> 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.
> >>
> >> 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?

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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Dave Chinner]

On Thu, Mar 08, 2012 at 10:38:32AM -0600, Ben Myers wrote:
> On Thu, Mar 08, 2012 at 09:42:21AM -0600, Eric Sandeen wrote:
> > 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?
> 
> 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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Eric Sandeen]

On 3/8/12 5:39 PM, Dave Chinner wrote:
> On Thu, Mar 08, 2012 at 10:38:32AM -0600, Ben Myers wrote:
>> On Thu, Mar 08, 2012 at 09:42:21AM -0600, Eric Sandeen wrote:
>>> 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?
>>
>> 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....

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

Thanks,
-Eric

> Cheers,
> 
> Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] default to 64 bit inodes & add feature flag

[Ben Myers]

Hi Dave,

On Fri, Mar 09, 2012 at 10:39:53AM +1100, Dave Chinner wrote:
> On Thu, Mar 08, 2012 at 10:38:32AM -0600, Ben Myers wrote:
> > On Thu, Mar 08, 2012 at 09:42:21AM -0600, Eric Sandeen wrote:
> > > 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?
> > 
> > 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....

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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs