[PATCH] sanity check inode size vs inode ratio

[PATCH] sanity check inode size vs inode ratio

[Andreas Dilger]

A quick patch to sanity check the inode ratio vs the inode size.  In some
cases Lustre users have tried specifying an inode size of 4096 bytes, while
keeping an inode ratio of one inode per 4096 bytes, causing mke2fs to spin
forever trying to allocate the inode tables.  I'm sure more people will do
this now that large inodes are available in ext4 and documented in e2fsprogs.

Signed-off-by: Andreas Dilger <adilger@clusterfs.com>

======================== e2fsprogs-inode_ratio.patch ========================
--- misc/mke2fs.c	2007-05-01 05:52:35.000000000 -0600
+++ misc/mke2fs.c.sav	2007-05-14 16:53:10.000000000 -0600
@@ -1504,6 +1504,18 @@
 		((__u64) fs_param.s_blocks_count * blocksize)
 			/ inode_ratio;
 
+	if ((long long)fs_param.s_inodes_count *
+	    inode_size ?: EXT2_GOOD_OLD_INODE_SIZE >
+	    (long long)fs_param.s_blocks_count * EXT2_BLOCK_SIZE(&fs_param)) {
+		com_err(program_name, 0, _("inode_size %u * inodes_count %u "
+					  "too bit for filesystem size %lu,\n"
+					  "\tspecify higher inode_ratio (-i) "
+					  "or lower inode count (-N)\n"),
+			inode_size ?: EXT2_GOOD_OLD_INODE_SIZE,
+			fs_param.s_inodes_count, fs_param.s_blocks_count);
+		exit(1);
+	}
+
 	/*
 	 * Calculate number of blocks to reserve
 	 */
======================== e2fsprogs-inode_ratio.patch ========================
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

Re: [PATCH] sanity check inode size vs inode ratio

[Theodore Tso]

On Mon, May 14, 2007 at 05:03:11PM -0600, Andreas Dilger wrote:
> A quick patch to sanity check the inode ratio vs the inode size.  In some
> cases Lustre users have tried specifying an inode size of 4096 bytes, while
> keeping an inode ratio of one inode per 4096 bytes, causing mke2fs to spin
> forever trying to allocate the inode tables.  I'm sure more people will do
> this now that large inodes are available in ext4 and documented in e2fsprogs.

I can't replicate this.  I'm guessing you are doing this with the
clusterfs codebase that has the extent patches?  Mke2fs shouldn't be
spinning if it can't allocate the inode tables.  Instead it should
print the error message:

% ./mke2fs -F -I 4096 -i 4096 -b 4096 -j /tmp/foo.img
mke2fs 1.40-WIP (07-Apr-2007)
Warning: 4096-byte inodes not usable on older systems
/tmp/foo.img: Not enough space to build proposed filesystem while setting up superblock

I don't have an objection with a patch that prints a more explicit
patch, but even with your change, there will still be combinations
that won't trigger your patch, but if there is a bug in your applied
patch series, we should root cause the problem that is causing mke2fs
spin forever, since that will probably cause problems elsewhere:

% ./mke2fs -F -I 4096 -N 4095 -b 4096 -j /tmp/foo.img 4096
mke2fs 1.40-WIP (07-Apr-2007)
Warning: 4096-byte inodes not usable on older systems
/tmp/foo.img: Not enough space to build proposed filesystem while setting up superblock

						- Ted

Re: [PATCH] sanity check inode size vs inode ratio

[Andreas Dilger]

On May 22, 2007  15:45 -0400, Theodore Tso wrote:
> On Mon, May 14, 2007 at 05:03:11PM -0600, Andreas Dilger wrote:
> > A quick patch to sanity check the inode ratio vs the inode size.  In some
> > cases Lustre users have tried specifying an inode size of 4096 bytes, while
> > keeping an inode ratio of one inode per 4096 bytes, causing mke2fs to spin
> > forever trying to allocate the inode tables.  I'm sure more people will do
> > this now that large inodes are available in ext4 and documented in e2fsprogs.
> 
> I can't replicate this.  I'm guessing you are doing this with the
> clusterfs codebase that has the extent patches?  Mke2fs shouldn't be
> spinning if it can't allocate the inode tables.  Instead it should
> print the error message:

Hmm, I suppose it might be due to running this on a 2TB filesystem that is
trying to allocate a huge number of inodes.  It could also have been with
an older version of mke2fs - it was a customer that reported the problem.

I just happened to be poking in that bit of code recently and thought I'd
add the sanity check.  I don't think it relates to CFS patches, since we
don't change mke2fs at all for the extent code.

We can let it drop for now, until I have a reproducer.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.