xfs
[Top] [All Lists]

Re: [PATCH] mkfs: default to CRC enabled filesystems

To: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Subject: Re: [PATCH] mkfs: default to CRC enabled filesystems
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 19 Mar 2015 09:55:25 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1426720967-8215-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1426720967-8215-1-git-send-email-david@xxxxxxxxxxxxx>
On 3/18/15 6:22 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> It's time to change the mkfs defaults to enable CRCs for all new
> filesystems. While there, also enable the free inode btree by
> default, too, as that functioanlity has also had long enough to make

"functionality" ;)

> it into distro kernels, too.

> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>

> ---
>  mkfs/xfs_mkfs.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 484e7a8..d7e70fa 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1024,8 +1024,8 @@ main(
>       force_overwrite = 0;
>       worst_freelist = 0;
>       lazy_sb_counters = 1;
> -     crcs_enabled = 0;
> -     finobt = 0;
> +     crcs_enabled = 1;
> +     finobt = 1;
>       memset(&fsx, 0, sizeof(fsx));
>  
>       memset(&xi, 0, sizeof(xi));
> @@ -1866,15 +1866,14 @@ _("V2 attribute format always enabled on CRC enabled 
> filesytems\n"));
>  _("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
>                       usage();
>               }
> -     }
> -
> -     /*
> -      * The kernel doesn't currently support crc=0,finobt=1 filesystems.
> -      * Catch it here, disable finobt and warn the user.
> -      */
> -     if (finobt && !crcs_enabled) {
> -             fprintf(stderr,
> -_("warning: finobt not supported without CRC support, disabled.\n"));
> +     } else {
> +             /*
> +              * The kernel doesn't currently support crc=0,finobt=1
> +              * filesystems. If crcs are not enabled, the user has
> +              * explicitly turned them off, so just silently turn off finobt
> +              * so that we don't issue unnecessary warnings when non-default
> +              * options are selected.
> +              */
>               finobt = 0;
>       }

Problem here is that if both are explicitly specified, one is ignored, rather
than letting the user know they've selected an invalid set of options:

# mkfs/mkfs.xfs -dfile,name=fsfile,size=1g -m crc=0,finobt=1
meta-data=fsfile                 isize=256    agcount=4, agsize=65536 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=0        finobt=0
...

This might require a "finobtflag" to keep track of whether it's user-specified,
as we do with other options?

-Eric

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