From: Ira Weiny <[email protected]>
As agreed upon[1]. We make the dax mount option a tri-state. '-o dax'
continues to operate the same. We add 'always', 'never', and 'inode'
(default).
[1] https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from v6:
Use 2 flag bits rather than a field.
change iflag to inode
Changes from v5:
New Patch
---
fs/xfs/xfs_mount.h | 3 ++-
fs/xfs/xfs_super.c | 44 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 88ab09ed29e7..d581b990e59a 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -233,7 +233,8 @@ typedef struct xfs_mount {
allocator */
#define XFS_MOUNT_NOATTR2 (1ULL << 25) /* disable use of attr2 format */
-#define XFS_MOUNT_DAX (1ULL << 62) /* TEST ONLY! */
+#define XFS_MOUNT_DAX (1ULL << 62)
+#define XFS_MOUNT_NODAX (1ULL << 63)
/*
* Max and min values for mount-option defined I/O
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 2094386af8ac..d7bd8f5e00c9 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -47,6 +47,32 @@ static struct kset *xfs_kset; /* top-level xfs sysfs dir */
static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */
#endif
+enum {
+ XFS_DAX_INODE = 0,
+ XFS_DAX_ALWAYS = 1,
+ XFS_DAX_NEVER = 2,
+};
+
+static void xfs_mount_set_dax_mode(struct xfs_mount *mp, u32 val)
+{
+ if (val == XFS_DAX_INODE) {
+ mp->m_flags &= ~(XFS_MOUNT_DAX | XFS_MOUNT_NODAX);
+ } else if (val == XFS_DAX_ALWAYS) {
+ mp->m_flags &= ~XFS_MOUNT_NODAX;
+ mp->m_flags |= XFS_MOUNT_DAX;
+ } else if (val == XFS_DAX_NEVER) {
+ mp->m_flags &= ~XFS_MOUNT_DAX;
+ mp->m_flags |= XFS_MOUNT_NODAX;
+ }
+}
+
+static const struct constant_table dax_param_enums[] = {
+ {"inode", XFS_DAX_INODE },
+ {"always", XFS_DAX_ALWAYS },
+ {"never", XFS_DAX_NEVER },
+ {}
+};
+
/*
* Table driven mount option parser.
*/
@@ -59,7 +85,7 @@ enum {
Opt_filestreams, Opt_quota, Opt_noquota, Opt_usrquota, Opt_grpquota,
Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
- Opt_discard, Opt_nodiscard, Opt_dax,
+ Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum,
};
static const struct fs_parameter_spec xfs_fs_parameters[] = {
@@ -103,6 +129,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
fsparam_flag("discard", Opt_discard),
fsparam_flag("nodiscard", Opt_nodiscard),
fsparam_flag("dax", Opt_dax),
+ fsparam_enum("dax", Opt_dax_enum, dax_param_enums),
{}
};
@@ -129,7 +156,6 @@ xfs_fs_show_options(
{ XFS_MOUNT_GRPID, ",grpid" },
{ XFS_MOUNT_DISCARD, ",discard" },
{ XFS_MOUNT_LARGEIO, ",largeio" },
- { XFS_MOUNT_DAX, ",dax" },
{ 0, NULL }
};
struct xfs_mount *mp = XFS_M(root->d_sb);
@@ -185,6 +211,13 @@ xfs_fs_show_options(
if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
seq_puts(m, ",noquota");
+ if (mp->m_flags & XFS_MOUNT_DAX)
+ seq_puts(m, ",dax=always");
+ else if (mp->m_flags & XFS_MOUNT_NODAX)
+ seq_puts(m, ",dax=never");
+ else
+ seq_puts(m, ",dax=inode");
+
return 0;
}
@@ -1244,7 +1277,10 @@ xfs_fc_parse_param(
return 0;
#ifdef CONFIG_FS_DAX
case Opt_dax:
- mp->m_flags |= XFS_MOUNT_DAX;
+ xfs_mount_set_dax_mode(mp, XFS_DAX_ALWAYS);
+ return 0;
+ case Opt_dax_enum:
+ xfs_mount_set_dax_mode(mp, result.uint_32);
return 0;
#endif
default:
@@ -1451,7 +1487,7 @@ xfs_fc_fill_super(
if (!rtdev_is_dax && !datadev_is_dax) {
xfs_alert(mp,
"DAX unsupported by block device. Turning off DAX.");
- mp->m_flags &= ~XFS_MOUNT_DAX;
+ xfs_mount_set_dax_mode(mp, XFS_DAX_NEVER);
}
if (xfs_sb_version_hasreflink(&mp->m_sb)) {
xfs_alert(mp,
--
2.25.1
On Sun, Apr 12, 2020 at 10:40:41PM -0700, [email protected] wrote:
> From: Ira Weiny <[email protected]>
>
> As agreed upon[1]. We make the dax mount option a tri-state. '-o dax'
> continues to operate the same. We add 'always', 'never', and 'inode'
> (default).
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> Signed-off-by: Ira Weiny <[email protected]>
>
> ---
> Changes from v6:
> Use 2 flag bits rather than a field.
> change iflag to inode
>
> Changes from v5:
> New Patch
> ---
> fs/xfs/xfs_mount.h | 3 ++-
> fs/xfs/xfs_super.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 88ab09ed29e7..d581b990e59a 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -233,7 +233,8 @@ typedef struct xfs_mount {
> allocator */
> #define XFS_MOUNT_NOATTR2 (1ULL << 25) /* disable use of attr2 format */
>
> -#define XFS_MOUNT_DAX (1ULL << 62) /* TEST ONLY! */
> +#define XFS_MOUNT_DAX (1ULL << 62)
> +#define XFS_MOUNT_NODAX (1ULL << 63)
>
> /*
> * Max and min values for mount-option defined I/O
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 2094386af8ac..d7bd8f5e00c9 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -47,6 +47,32 @@ static struct kset *xfs_kset; /* top-level xfs sysfs dir */
> static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */
> #endif
>
> +enum {
> + XFS_DAX_INODE = 0,
> + XFS_DAX_ALWAYS = 1,
> + XFS_DAX_NEVER = 2,
> +};
> +
> +static void xfs_mount_set_dax_mode(struct xfs_mount *mp, u32 val)
> +{
> + if (val == XFS_DAX_INODE) {
> + mp->m_flags &= ~(XFS_MOUNT_DAX | XFS_MOUNT_NODAX);
> + } else if (val == XFS_DAX_ALWAYS) {
> + mp->m_flags &= ~XFS_MOUNT_NODAX;
> + mp->m_flags |= XFS_MOUNT_DAX;
> + } else if (val == XFS_DAX_NEVER) {
> + mp->m_flags &= ~XFS_MOUNT_DAX;
> + mp->m_flags |= XFS_MOUNT_NODAX;
> + }
> +}
> +
> +static const struct constant_table dax_param_enums[] = {
> + {"inode", XFS_DAX_INODE },
> + {"always", XFS_DAX_ALWAYS },
> + {"never", XFS_DAX_NEVER },
> + {}
I think that the dax_param_enums table (and the unnamed enum defining
XFS_DAX_*) probably ought to be part of the VFS so that you don't have
to duplicate these two pieces whenever it's time to bring ext4 in line
with XFS.
That probably doesn't need to be done right away, though...
Reviewed-by: Darrick J. Wong <[email protected]>
--D
> +};
> +
> /*
> * Table driven mount option parser.
> */
> @@ -59,7 +85,7 @@ enum {
> Opt_filestreams, Opt_quota, Opt_noquota, Opt_usrquota, Opt_grpquota,
> Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
> Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
> - Opt_discard, Opt_nodiscard, Opt_dax,
> + Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum,
> };
>
> static const struct fs_parameter_spec xfs_fs_parameters[] = {
> @@ -103,6 +129,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
> fsparam_flag("discard", Opt_discard),
> fsparam_flag("nodiscard", Opt_nodiscard),
> fsparam_flag("dax", Opt_dax),
> + fsparam_enum("dax", Opt_dax_enum, dax_param_enums),
> {}
> };
>
> @@ -129,7 +156,6 @@ xfs_fs_show_options(
> { XFS_MOUNT_GRPID, ",grpid" },
> { XFS_MOUNT_DISCARD, ",discard" },
> { XFS_MOUNT_LARGEIO, ",largeio" },
> - { XFS_MOUNT_DAX, ",dax" },
> { 0, NULL }
> };
> struct xfs_mount *mp = XFS_M(root->d_sb);
> @@ -185,6 +211,13 @@ xfs_fs_show_options(
> if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
> seq_puts(m, ",noquota");
>
> + if (mp->m_flags & XFS_MOUNT_DAX)
> + seq_puts(m, ",dax=always");
> + else if (mp->m_flags & XFS_MOUNT_NODAX)
> + seq_puts(m, ",dax=never");
> + else
> + seq_puts(m, ",dax=inode");
> +
> return 0;
> }
>
> @@ -1244,7 +1277,10 @@ xfs_fc_parse_param(
> return 0;
> #ifdef CONFIG_FS_DAX
> case Opt_dax:
> - mp->m_flags |= XFS_MOUNT_DAX;
> + xfs_mount_set_dax_mode(mp, XFS_DAX_ALWAYS);
> + return 0;
> + case Opt_dax_enum:
> + xfs_mount_set_dax_mode(mp, result.uint_32);
> return 0;
> #endif
> default:
> @@ -1451,7 +1487,7 @@ xfs_fc_fill_super(
> if (!rtdev_is_dax && !datadev_is_dax) {
> xfs_alert(mp,
> "DAX unsupported by block device. Turning off DAX.");
> - mp->m_flags &= ~XFS_MOUNT_DAX;
> + xfs_mount_set_dax_mode(mp, XFS_DAX_NEVER);
> }
> if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> xfs_alert(mp,
> --
> 2.25.1
>
On Mon, Apr 13, 2020 at 08:46:19AM -0700, Darrick J. Wong wrote:
> On Sun, Apr 12, 2020 at 10:40:41PM -0700, [email protected] wrote:
> > From: Ira Weiny <[email protected]>
> >
> > As agreed upon[1]. We make the dax mount option a tri-state. '-o dax'
> > continues to operate the same. We add 'always', 'never', and 'inode'
> > (default).
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> >
> > Signed-off-by: Ira Weiny <[email protected]>
> >
> > ---
> > Changes from v6:
> > Use 2 flag bits rather than a field.
> > change iflag to inode
> >
> > Changes from v5:
> > New Patch
> > ---
> > fs/xfs/xfs_mount.h | 3 ++-
> > fs/xfs/xfs_super.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 42 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > index 88ab09ed29e7..d581b990e59a 100644
> > --- a/fs/xfs/xfs_mount.h
> > +++ b/fs/xfs/xfs_mount.h
> > @@ -233,7 +233,8 @@ typedef struct xfs_mount {
> > allocator */
> > #define XFS_MOUNT_NOATTR2 (1ULL << 25) /* disable use of attr2 format */
> >
> > -#define XFS_MOUNT_DAX (1ULL << 62) /* TEST ONLY! */
> > +#define XFS_MOUNT_DAX (1ULL << 62)
> > +#define XFS_MOUNT_NODAX (1ULL << 63)
> >
> > /*
> > * Max and min values for mount-option defined I/O
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 2094386af8ac..d7bd8f5e00c9 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -47,6 +47,32 @@ static struct kset *xfs_kset; /* top-level xfs sysfs dir */
> > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */
> > #endif
> >
> > +enum {
> > + XFS_DAX_INODE = 0,
> > + XFS_DAX_ALWAYS = 1,
> > + XFS_DAX_NEVER = 2,
> > +};
> > +
> > +static void xfs_mount_set_dax_mode(struct xfs_mount *mp, u32 val)
> > +{
> > + if (val == XFS_DAX_INODE) {
> > + mp->m_flags &= ~(XFS_MOUNT_DAX | XFS_MOUNT_NODAX);
> > + } else if (val == XFS_DAX_ALWAYS) {
> > + mp->m_flags &= ~XFS_MOUNT_NODAX;
> > + mp->m_flags |= XFS_MOUNT_DAX;
> > + } else if (val == XFS_DAX_NEVER) {
> > + mp->m_flags &= ~XFS_MOUNT_DAX;
> > + mp->m_flags |= XFS_MOUNT_NODAX;
> > + }
> > +}
> > +
> > +static const struct constant_table dax_param_enums[] = {
> > + {"inode", XFS_DAX_INODE },
> > + {"always", XFS_DAX_ALWAYS },
> > + {"never", XFS_DAX_NEVER },
> > + {}
>
> I think that the dax_param_enums table (and the unnamed enum defining
> XFS_DAX_*) probably ought to be part of the VFS so that you don't have
> to duplicate these two pieces whenever it's time to bring ext4 in line
> with XFS.
>
> That probably doesn't need to be done right away, though...
Ext4 has a very different param parsing mechanism which I've barely learned.
I'm not really seeing how to use the enum strategy so I've just used a string
option. But I'm open to being corrected.
I am close to having the series working and hope to have that set (which builds
on this one) out for review soon (today?).
>
> Reviewed-by: Darrick J. Wong <[email protected]>
Thanks,
Ira
>
> --D
>
>
> > +};
> > +
> > /*
> > * Table driven mount option parser.
> > */
> > @@ -59,7 +85,7 @@ enum {
> > Opt_filestreams, Opt_quota, Opt_noquota, Opt_usrquota, Opt_grpquota,
> > Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
> > Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
> > - Opt_discard, Opt_nodiscard, Opt_dax,
> > + Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum,
> > };
> >
> > static const struct fs_parameter_spec xfs_fs_parameters[] = {
> > @@ -103,6 +129,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
> > fsparam_flag("discard", Opt_discard),
> > fsparam_flag("nodiscard", Opt_nodiscard),
> > fsparam_flag("dax", Opt_dax),
> > + fsparam_enum("dax", Opt_dax_enum, dax_param_enums),
> > {}
> > };
> >
> > @@ -129,7 +156,6 @@ xfs_fs_show_options(
> > { XFS_MOUNT_GRPID, ",grpid" },
> > { XFS_MOUNT_DISCARD, ",discard" },
> > { XFS_MOUNT_LARGEIO, ",largeio" },
> > - { XFS_MOUNT_DAX, ",dax" },
> > { 0, NULL }
> > };
> > struct xfs_mount *mp = XFS_M(root->d_sb);
> > @@ -185,6 +211,13 @@ xfs_fs_show_options(
> > if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
> > seq_puts(m, ",noquota");
> >
> > + if (mp->m_flags & XFS_MOUNT_DAX)
> > + seq_puts(m, ",dax=always");
> > + else if (mp->m_flags & XFS_MOUNT_NODAX)
> > + seq_puts(m, ",dax=never");
> > + else
> > + seq_puts(m, ",dax=inode");
> > +
> > return 0;
> > }
> >
> > @@ -1244,7 +1277,10 @@ xfs_fc_parse_param(
> > return 0;
> > #ifdef CONFIG_FS_DAX
> > case Opt_dax:
> > - mp->m_flags |= XFS_MOUNT_DAX;
> > + xfs_mount_set_dax_mode(mp, XFS_DAX_ALWAYS);
> > + return 0;
> > + case Opt_dax_enum:
> > + xfs_mount_set_dax_mode(mp, result.uint_32);
> > return 0;
> > #endif
> > default:
> > @@ -1451,7 +1487,7 @@ xfs_fc_fill_super(
> > if (!rtdev_is_dax && !datadev_is_dax) {
> > xfs_alert(mp,
> > "DAX unsupported by block device. Turning off DAX.");
> > - mp->m_flags &= ~XFS_MOUNT_DAX;
> > + xfs_mount_set_dax_mode(mp, XFS_DAX_NEVER);
> > }
> > if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> > xfs_alert(mp,
> > --
> > 2.25.1
> >
On Mon, Apr 13, 2020 at 12:28:11PM -0700, Ira Weiny wrote:
> > I think that the dax_param_enums table (and the unnamed enum defining
> > XFS_DAX_*) probably ought to be part of the VFS so that you don't have
> > to duplicate these two pieces whenever it's time to bring ext4 in line
> > with XFS.
> >
> > That probably doesn't need to be done right away, though...
>
> Ext4 has a very different param parsing mechanism which I've barely learned.
> I'm not really seeing how to use the enum strategy so I've just used a string
> option. But I'm open to being corrected.
>
> I am close to having the series working and hope to have that set (which builds
> on this one) out for review soon (today?).
ext4 still uses the legacy mount option parsing that XFS used until
recently. It needs to be switched over to the new mount API anyway.
On Sun, Apr 12, 2020 at 10:40:41PM -0700, [email protected] wrote:
> +#define XFS_MOUNT_DAX (1ULL << 62)
> +#define XFS_MOUNT_NODAX (1ULL << 63)
Replace this with XFS_MOUNT_DAX_ALWAYS and XFS_MOUNT_DAX_NEVER?
On Tue, Apr 14, 2020 at 08:25:30AM +0200, Christoph Hellwig wrote:
> On Sun, Apr 12, 2020 at 10:40:41PM -0700, [email protected] wrote:
> > +#define XFS_MOUNT_DAX (1ULL << 62)
> > +#define XFS_MOUNT_NODAX (1ULL << 63)
>
> Replace this with XFS_MOUNT_DAX_ALWAYS and XFS_MOUNT_DAX_NEVER?
Sounds reasonable, Done for v8
Ira