2021-09-15 17:23:31

by Eric Sandeen

[permalink] [raw]
Subject: [PATCH 0/3 RFC] Remove DAX experimental warnings

For six years now, when mounting xfs, ext4, or ext2 with dax, the drivers
have logged "DAX enabled. Warning: EXPERIMENTAL, use at your own risk."

IIRC, dchinner added this to the original XFS patchset, and Dan Williams
followed suit for ext4 and ext2.

After brief conversations with some ext4 and xfs developers and maintainers,
it seems that it may be time to consider removing this warning.

For XFS, we had been holding out for reflink+dax capability, but proposals
which had seemed promising now appear to be indefinitely stalled, and
I think we might want to consider that dax-without-reflink is no longer
EXPERIMENTAL, while dax-with-reflink is simply an unimplemented future
feature.

For EXT4/EXT2, I'm not aware of significant outstanding concerns that would
continue to require the dire warning.

Thoughts?

Thanks,
-Eric


2021-09-15 17:24:24

by Eric Sandeen

[permalink] [raw]
Subject: [PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

As there seems to be no significant outstanding concern about
dax on ext2 at this point, remove the scary EXPERIMENTAL
warning when in use.

Signed-off-by: Eric Sandeen <[email protected]>
---
fs/ext2/super.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index d8d580b..1915733 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -587,8 +587,6 @@ static int parse_options(char *options, struct super_block *sb,
fallthrough;
case Opt_dax:
#ifdef CONFIG_FS_DAX
- ext2_msg(sb, KERN_WARNING,
- "DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
set_opt(opts->s_mount_opt, DAX);
#else
ext2_msg(sb, KERN_INFO, "dax option not supported");
--
1.8.3.1

2021-09-15 17:24:32

by Eric Sandeen

[permalink] [raw]
Subject: [PATCH 1/3] xfs: remove dax EXPERIMENTAL warning

As there seems to be no significant outstanding concern about
dax on xfs at this point, remove the scary EXPERIMENTAL
warning when in use.

(dax+reflink is still unimplemented, but that can be considered
a future feature, and doesn't require a warning for the
non-reflink usecase.)

Signed-off-by: Eric Sandeen <[email protected]>
---
fs/xfs/xfs_super.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index c4e0cd1..0c71dbb 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1594,9 +1594,6 @@ struct proc_xfs_info {
if (xfs_has_dax_always(mp)) {
bool rtdev_is_dax = false, datadev_is_dax;

- xfs_warn(mp,
- "DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
-
datadev_is_dax = xfs_buftarg_is_dax(sb, mp->m_ddev_targp);
if (mp->m_rtdev_targp)
rtdev_is_dax = xfs_buftarg_is_dax(sb,
--
1.8.3.1

2021-09-15 17:24:34

by Eric Sandeen

[permalink] [raw]
Subject: [PATCH 2/3] ext4: remove dax EXPERIMENTAL warning

As there seems to be no significant outstanding concern about
dax on ext4 at this point, remove the scary EXPERIMENTAL
warning when in use.

Signed-off-by: Eric Sandeen <[email protected]>
---
fs/ext4/super.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0775950..82948d6 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2346,8 +2346,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
"both data=journal and dax");
return -1;
}
- ext4_msg(sb, KERN_WARNING,
- "DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
sbi->s_mount_opt |= EXT4_MOUNT_DAX_ALWAYS;
sbi->s_mount_opt2 &= ~EXT4_MOUNT2_DAX_NEVER;
break;
--
1.8.3.1

2021-09-15 18:36:42

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 0/3 RFC] Remove DAX experimental warnings

On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <[email protected]> wrote:
>
> For six years now, when mounting xfs, ext4, or ext2 with dax, the drivers
> have logged "DAX enabled. Warning: EXPERIMENTAL, use at your own risk."
>
> IIRC, dchinner added this to the original XFS patchset, and Dan Williams
> followed suit for ext4 and ext2.
>
> After brief conversations with some ext4 and xfs developers and maintainers,
> it seems that it may be time to consider removing this warning.
>
> For XFS, we had been holding out for reflink+dax capability, but proposals
> which had seemed promising now appear to be indefinitely stalled, and
> I think we might want to consider that dax-without-reflink is no longer
> EXPERIMENTAL, while dax-with-reflink is simply an unimplemented future
> feature.

I do regret my gap in engagement since the last review as I got
distracted by CXL, but I've recently gotten my act together and picked
up the review again to help get Ruan's patches over the goal line [1].
I am currently awaiting Ruan's response to latest review feedback
(looks like a new posting this morning). During that review Christoph
identified some cleanups that would help Ruan's series, and those are
now merged upstream [2]. The last remaining stumbling block (further
block-device entanglements with dax-devices) I noted here [2]. The
proposal is to consider eliding device-mapper dax-reflink support for
now and proceed with just xfs-on-/dev/pmem until Mike, Jens, and
Christoph can chime in on the future of dax on block devices.

As far as I can see we have line of sight to land xfs-dax-reflink
support for v5.16, does anyone see that differently at this point?

[1]: https://lore.kernel.org/linux-xfs/CAPcyv4h0p+zD5tsT8HDUpNq_ZDCqo249KsmPLX-U8ia146r2Tg@mail.gmail.com/
[2]: https://lore.kernel.org/linux-xfs/CAPcyv4ic+LDagR8uF18tO3cCb6t=YTZNkAOK=vnsnERqY6Ze_g@mail.gmail.com/
[3]: https://lore.kernel.org/nvdimm/CAPcyv4hvzS1c01BweBkgDsjg=VGnaUUKi7b6j+1X=Rqzzm961Q@mail.gmail.com/

2021-09-15 18:49:32

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 0/3 RFC] Remove DAX experimental warnings

On 9/15/21 1:35 PM, Dan Williams wrote:
> On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <[email protected]> wrote:
>>
>> For six years now, when mounting xfs, ext4, or ext2 with dax, the drivers
>> have logged "DAX enabled. Warning: EXPERIMENTAL, use at your own risk."
>>
>> IIRC, dchinner added this to the original XFS patchset, and Dan Williams
>> followed suit for ext4 and ext2.
>>
>> After brief conversations with some ext4 and xfs developers and maintainers,
>> it seems that it may be time to consider removing this warning.
>>
>> For XFS, we had been holding out for reflink+dax capability, but proposals
>> which had seemed promising now appear to be indefinitely stalled, and
>> I think we might want to consider that dax-without-reflink is no longer
>> EXPERIMENTAL, while dax-with-reflink is simply an unimplemented future
>> feature.
>
> I do regret my gap in engagement since the last review as I got
> distracted by CXL, but I've recently gotten my act together and picked
> up the review again to help get Ruan's patches over the goal line [1].
> I am currently awaiting Ruan's response to latest review feedback
> (looks like a new posting this morning). During that review Christoph
> identified some cleanups that would help Ruan's series, and those are
> now merged upstream [2]. The last remaining stumbling block (further
> block-device entanglements with dax-devices) I noted here [2]. The
> proposal is to consider eliding device-mapper dax-reflink support for
> now and proceed with just xfs-on-/dev/pmem until Mike, Jens, and
> Christoph can chime in on the future of dax on block devices.
>
> As far as I can see we have line of sight to land xfs-dax-reflink
> support for v5.16, does anyone see that differently at this point?

Thanks for that update, Dan. I'm wondering, even if we have renewed
hopes and dreams for dax+reflink, would it make sense to go ahead and
declare dax without reflink non-experimental, and tag dax+reflink as
a new "EXPERIMENTAL feature if and when it lands?

-Eric

2021-09-15 18:54:41

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 1/3] xfs: remove dax EXPERIMENTAL warning

On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <[email protected]> wrote:
>
> As there seems to be no significant outstanding concern about
> dax on xfs at this point, remove the scary EXPERIMENTAL
> warning when in use.
>
> (dax+reflink is still unimplemented, but that can be considered
> a future feature, and doesn't require a warning for the
> non-reflink usecase.)

The original concern was that dax-reflink could not be implemented
without ABI regressions. As far as I can see that concern has been put
to rest by the proposed patches. Am I wrong? So, if we're committed to
not breaking past promises I think this change can be made
out-of-order from when the reflink support patches land.

Acked-by: Dan Williams <[email protected]>

...but I'm also fine with waiting for the final reflink merge.

2021-09-15 19:01:10

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 0/3 RFC] Remove DAX experimental warnings

On Wed, Sep 15, 2021 at 11:49 AM Eric Sandeen <[email protected]> wrote:
>
> On 9/15/21 1:35 PM, Dan Williams wrote:
> > On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <[email protected]> wrote:
> >>
> >> For six years now, when mounting xfs, ext4, or ext2 with dax, the drivers
> >> have logged "DAX enabled. Warning: EXPERIMENTAL, use at your own risk."
> >>
> >> IIRC, dchinner added this to the original XFS patchset, and Dan Williams
> >> followed suit for ext4 and ext2.
> >>
> >> After brief conversations with some ext4 and xfs developers and maintainers,
> >> it seems that it may be time to consider removing this warning.
> >>
> >> For XFS, we had been holding out for reflink+dax capability, but proposals
> >> which had seemed promising now appear to be indefinitely stalled, and
> >> I think we might want to consider that dax-without-reflink is no longer
> >> EXPERIMENTAL, while dax-with-reflink is simply an unimplemented future
> >> feature.
> >
> > I do regret my gap in engagement since the last review as I got
> > distracted by CXL, but I've recently gotten my act together and picked
> > up the review again to help get Ruan's patches over the goal line [1].
> > I am currently awaiting Ruan's response to latest review feedback
> > (looks like a new posting this morning). During that review Christoph
> > identified some cleanups that would help Ruan's series, and those are
> > now merged upstream [2]. The last remaining stumbling block (further
> > block-device entanglements with dax-devices) I noted here [2]. The
> > proposal is to consider eliding device-mapper dax-reflink support for
> > now and proceed with just xfs-on-/dev/pmem until Mike, Jens, and
> > Christoph can chime in on the future of dax on block devices.
> >
> > As far as I can see we have line of sight to land xfs-dax-reflink
> > support for v5.16, does anyone see that differently at this point?
>
> Thanks for that update, Dan. I'm wondering, even if we have renewed
> hopes and dreams for dax+reflink, would it make sense to go ahead and
> declare dax without reflink non-experimental, and tag dax+reflink as
> a new "EXPERIMENTAL feature if and when it lands?

As I replied to the xfs patch in your series, I say "yes" EXPERIMENTAL
can go now, because the concern was reflink support might regress
dax-semantics wrt MAP_SYNC and the like. That concern seems to be
avoided by the current direction.

2021-09-17 13:36:25

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

On Wed 15-09-21 12:22:41, Eric Sandeen wrote:
> As there seems to be no significant outstanding concern about
> dax on ext2 at this point, remove the scary EXPERIMENTAL
> warning when in use.
>
> Signed-off-by: Eric Sandeen <[email protected]>

Agreed. Do you want my ack or should I just merge this patch?

Honza

> ---
> fs/ext2/super.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index d8d580b..1915733 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -587,8 +587,6 @@ static int parse_options(char *options, struct super_block *sb,
> fallthrough;
> case Opt_dax:
> #ifdef CONFIG_FS_DAX
> - ext2_msg(sb, KERN_WARNING,
> - "DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
> set_opt(opts->s_mount_opt, DAX);
> #else
> ext2_msg(sb, KERN_INFO, "dax option not supported");
> --
> 1.8.3.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2021-09-17 13:36:37

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 2/3] ext4: remove dax EXPERIMENTAL warning

On Wed 15-09-21 12:22:40, Eric Sandeen wrote:
> As there seems to be no significant outstanding concern about
> dax on ext4 at this point, remove the scary EXPERIMENTAL
> warning when in use.
>
> Signed-off-by: Eric Sandeen <[email protected]>

I agree. You can add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/super.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 0775950..82948d6 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2346,8 +2346,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> "both data=journal and dax");
> return -1;
> }
> - ext4_msg(sb, KERN_WARNING,
> - "DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
> sbi->s_mount_opt |= EXT4_MOUNT_DAX_ALWAYS;
> sbi->s_mount_opt2 &= ~EXT4_MOUNT2_DAX_NEVER;
> break;
> --
> 1.8.3.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2021-09-17 15:05:02

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

On Fri, Sep 17, 2021 at 11:47:07AM +0200, Jan Kara wrote:
> On Wed 15-09-21 12:22:41, Eric Sandeen wrote:
> > As there seems to be no significant outstanding concern about
> > dax on ext2 at this point, remove the scary EXPERIMENTAL
> > warning when in use.
> >
> > Signed-off-by: Eric Sandeen <[email protected]>
>
> Agreed. Do you want my ack or should I just merge this patch?

Please do not merge it. The whole DAX path is still a mess and should
not be elevated to non-EXPERMINTAL state in this form.

2021-09-17 15:17:14

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

On 9/17/21 7:59 AM, Christoph Hellwig wrote:
> On Fri, Sep 17, 2021 at 11:47:07AM +0200, Jan Kara wrote:
>> On Wed 15-09-21 12:22:41, Eric Sandeen wrote:
>>> As there seems to be no significant outstanding concern about
>>> dax on ext2 at this point, remove the scary EXPERIMENTAL
>>> warning when in use.
>>>
>>> Signed-off-by: Eric Sandeen <[email protected]>
>>
>> Agreed. Do you want my ack or should I just merge this patch?
>
> Please do not merge it. The whole DAX path is still a mess and should
> not be elevated to non-EXPERMINTAL state in this form.

Hi Christoph, "a mess" is tough to work with. What work remains before
we can lift the warning?

-Eric

2021-09-22 02:37:32

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

On Fri, Sep 17, 2021 at 01:59:10PM +0100, Christoph Hellwig wrote:
> On Fri, Sep 17, 2021 at 11:47:07AM +0200, Jan Kara wrote:
> > On Wed 15-09-21 12:22:41, Eric Sandeen wrote:
> > > As there seems to be no significant outstanding concern about
> > > dax on ext2 at this point, remove the scary EXPERIMENTAL
> > > warning when in use.
> > >
> > > Signed-off-by: Eric Sandeen <[email protected]>
> >
> > Agreed. Do you want my ack or should I just merge this patch?
>
> Please do not merge it. The whole DAX path is still a mess and should
> not be elevated to non-EXPERMINTAL state in this form.

Hi Christoph,

'still a mess' isn't all that useful for figuring out what still needs
to be done and splitting up the work. Do you have items beyond my own
list below?

- still arguing over what exactly FALLOC_FL_ZERO_REINIT_WHATEVER_PONIES
should be doing
- no reflink support, encompassing:
- hwpoison from mmap regions really ought to tell the fs that bad stuff
happened
- mm rmap can't handle more than one owner

--D

2021-09-22 05:28:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

On Tue, Sep 21, 2021 at 07:36:22PM -0700, Darrick J. Wong wrote:
> 'still a mess' isn't all that useful for figuring out what still needs
> to be done and splitting up the work. Do you have items beyond my own
> list below?
>
> - still arguing over what exactly FALLOC_FL_ZERO_REINIT_WHATEVER_PONIES
> should be doing
> - no reflink support, encompassing:
> - hwpoison from mmap regions really ought to tell the fs that bad stuff
> happened
> - mm rmap can't handle more than one owner

My main really big item is that we're still mounting through a fake
block device, suporting partitions and all that crap. We need to sort
out the whole story of how pmem/nvdimm is actually treated, because
what we have right now is not sustainable at all.