On Thu, Jun 20, 2013 at 04:17:47PM -0500, Geoffrey Wehrman wrote: > On Fri, Jun 07, 2013 at 10:26:02AM +1000, Dave Chinner wrote: > | From: Dave Chinner <dchinner@xxxxxxxxxx> > | > | With CRC enabled filesystems, certain options are now not optional > | and so are always enabled. Validate these options up front and > | abort if options are specified that cannot be set. > | > | Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > | --- > | mkfs/xfs_mkfs.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++----- > | 1 file changed, 56 insertions(+), 5 deletions(-) > | > | diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > | index 291bab4..9987dde 100644 > | --- a/mkfs/xfs_mkfs.c > | +++ b/mkfs/xfs_mkfs.c > ... > | @@ -1754,6 +1754,57 @@ _("block size %d cannot be smaller than logical sector size %d\n"), > | logversion = 2; > | } > | > | + /* > | + * Now we have blocks and sector sizes set up, check parameters that are > | + * no longer optional for CRC enabled filesystems. Catch them up front > | + * here before doing anything else. > | + */ > | + if (crcs_enabled) { > | + /* minimum inode size is 512 bytes, ipflag checked later */ > | + if ((isflag || ilflag) && inodelog < XFS_DINODE_DFL_CRC_LOG) { > | + fprintf(stderr, > | +_("Minimum inode size for CRCs is %d bytes\n"), > | + 1 << XFS_DINODE_DFL_CRC_LOG); > | + usage(); > | + } > > I am not satisfied with the explanation for not allowing 256 byte inodes > with CRCs, and I am requesting that this limitation not be implemented. > I have no issue with making the default inode size 512 bytes, but > removing the option for 256 byte inodes is an issue, especially with the > initial implementation. Making the minimum inode size 256 is fine. As I said on the call, it makes no sense to support 256 byte inodes for CRC enabled filesystems for either a performance or a support point of view. For the purpose of this discussion on the list, I'll redo the calculations from first principles as the inode core size has grown since these checks were originally done way back in 2008- 2009 so everyone can see why it doesn't make sense. To start with, the inode core size for version 1/2 inodes (i.e. without CRCs) is 100 bytes (including the di_next_unlinked field). With the 8 byte alignment rounding that the forks need, that gives us a literal area size of 152 bytes. Back in 2008-2009, the new v3 inode format did not have all the self describing metadata, and so the core size increased to about 140 bytes. This left roughly 112 bytes of literal space available. With the addition of the extra self describing metadata, the new v3 inode has grown to it's current size of 176 bytes, bring the literal area down to 80 bytes. That's smaller than I realised it was.... The minimum physical inode fork sizes are defined in xfs_types.h: /* * Min numbers of data/attr fork btree root pointers. */ #define MINDBTPTRS 3 #define MINABTPTRS 2 And their sizes are defined in xfs_bmap_btree.h: #define XFS_BMDR_SPACE_CALC(nrecs) \ (int)(sizeof(xfs_bmdr_block_t) + \ ((nrecs) * (sizeof(xfs_bmbt_key_t) + sizeof(xfs_bmbt_ptr_t)))) So: minimum data fork size = XFS_BMDR_SPACE_CALC(3) = 4 + 3 * (8 + 8) = 52 bytes minimum attr fork size = XFS_BMDR_SPACE_CALC(2) = 36 bytes And when we align these to 8 byte, we have minimum sizes of 56 bytes and 40 bytes for the data and attr forks respectively. That means we need at least 96 bytes of literal space available in the inode So, we have: literal space required available inode size v2 old v3 final v3 256 96 156 112 80 512 96 408 368 336 And so for the final v3 inode format there is only be 80 bytes of literal space available, which is not enough to fit minimally sized data and attr forks simultaneously with a 256 byte inode size. i.e. it's not a physically valid configuration. IOWs, there's nothing to debate - 256 byte inodes in v3 format is not physically possible with the current on-disk format definitions... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs