Re: [PATCH 2/2] mkfs: allow setting dax flag on root directory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/11/20 9:42 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> 
> Teach mkfs to set the DAX flag on the root directory so that all new
> files can be created in dax mode.  This is a complement to removing the
> mount option.

So, a new -d option, "-d dax"

This is ~analogous to cowextsize, rtinherit, projinherit, and extszinherit
so there is certainly precedence for this.  (where only rtinherit is a boolean
like this, but they are all inheritable behaviors)

(I wonder if "daxinherit" would be more consistent, but won't bikeshed
that (much))

> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
>  man/man8/mkfs.xfs.8 |   11 +++++++++++
>  mkfs/xfs_mkfs.c     |   14 ++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> 
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index 9d762a43011a..4b4fdd86b2f4 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -394,6 +394,17 @@ All inodes created by
>  will have this extent size hint applied.
>  The value must be provided in units of filesystem blocks.
>  Directories will pass on this hint to newly created children.
> +.TP
> +.BI dax= value
> +All inodes created by
> +.B mkfs.xfs
> +will have the DAX flag set.
> +This means that directories will pass the flag on to newly created files

let's call this "children" to match the other similar options?

(because technically it is passed on not only to regular files, right?)

> +and files will use the DAX IO paths when possible.
> +This value is either 1 to enable the use or 0 to disable.
> +By default,
> +.B mkfs.xfs
> +will not enable DAX mode.
>  .RE
>  .TP
>  .B \-f
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 2e6cd280e388..33507f6ea21c 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -60,6 +60,7 @@ enum {
>  	D_PROJINHERIT,
>  	D_EXTSZINHERIT,
>  	D_COWEXTSIZE,
> +	D_DAX,
>  	D_MAX_OPTS,
>  };
>  
> @@ -254,6 +255,7 @@ static struct opt_params dopts = {
>  		[D_PROJINHERIT] = "projinherit",
>  		[D_EXTSZINHERIT] = "extszinherit",
>  		[D_COWEXTSIZE] = "cowextsize",
> +		[D_DAX] = "dax",
>  	},
>  	.subopt_params = {
>  		{ .index = D_AGCOUNT,
> @@ -369,6 +371,12 @@ static struct opt_params dopts = {
>  		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
> +		{ .index = D_DAX,
> +		  .conflicts = { { NULL, LAST_CONFLICT } },

er....  should we conflict with reflink ....  ?

> +		  .minval = 0,
> +		  .maxval = 1,
> +		  .defaultval = 1,

Hm, interesting that this is a little different from rtinherit:

                { .index = D_RTINHERIT,
                  .conflicts = { { NULL, LAST_CONFLICT } },
                  .minval = 1,
                  .maxval = 1,
                  .defaultval = 1,
                },

I think this means that:

-d rtinherit
-d rtinherit=1

are valid, but

-d rtinherit=0 is not, but

-d dax
-d dax=1
-d dax=0

are all valid?

While the latter makes a bit more sense, I wonder if we should stay consistent w/ the rtinherit semantics.  Or do you envision some sort of automatic enabling of this based on device typethat we'd need to override in the future?

> +		},
>  	},
>  };
>  
> @@ -1434,6 +1442,12 @@ data_opts_parser(
>  		cli->fsx.fsx_cowextsize = getnum(value, opts, subopt);
>  		cli->fsx.fsx_xflags |= FS_XFLAG_COWEXTSIZE;
>  		break;
> +	case D_DAX:
> +		if (getnum(value, opts, subopt))
> +			cli->fsx.fsx_xflags |= FS_XFLAG_DAX;
> +		else
> +			cli->fsx.fsx_xflags &= ~FS_XFLAG_DAX;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> 



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux