xfs
[Top] [All Lists]

Re: [PATCH 39/48] mkfs.xfs: validate options for CRCs up front.

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 39/48] mkfs.xfs: validate options for CRCs up front.
From: Geoffrey Wehrman <gwehrman@xxxxxxx>
Date: Fri, 21 Jun 2013 08:44:02 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130620230534.GV29376@dastard>
References: <1370564771-4929-1-git-send-email-david@xxxxxxxxxxxxx> <1370564771-4929-40-git-send-email-david@xxxxxxxxxxxxx> <20130620211747.GB7800@xxxxxxx> <20130620230534.GV29376@dastard>
User-agent: Mutt/1.5.14 (2007-02-12)
On Fri, Jun 21, 2013 at 09:05:34AM +1000, Dave Chinner wrote:
| 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.
| 
| IOWs, there's nothing to debate - 256 byte inodes in v3 format is
| not physically possible with the current on-disk format
| definitions...

I should have done the math.  I didn't realize how bloated v4 inodes are.


-- 
Geoffrey Wehrman  651-683-5496  gwehrman@xxxxxxx

<Prev in Thread] Current Thread [Next in Thread>