[PATCH 0/3 RFC] Remove DAX experimental warnings

[PATCH 0/3 RFC] Remove DAX experimental warnings

[Eric Sandeen]

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

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

[Eric Sandeen]

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 <sandeen@redhat.com>
---
 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

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

[Eric Sandeen]

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 <sandeen@redhat.com>
---
 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

[PATCH 3/3] ext2: remove dax EXPERIMENTAL warning

[Eric Sandeen]

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 <sandeen@redhat.com>
---
 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

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

[Dan Williams]

On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <sandeen@redhat.com> 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/

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

[Eric Sandeen]

On 9/15/21 1:35 PM, Dan Williams wrote:
> On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <sandeen@redhat.com> 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

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

[Dan Williams]

On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <sandeen@redhat.com> 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 <dan.j.williams@intel.com>

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

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

[Dan Williams]

On Wed, Sep 15, 2021 at 11:49 AM Eric Sandeen <sandeen@sandeen.net> wrote:
>
> On 9/15/21 1:35 PM, Dan Williams wrote:
> > On Wed, Sep 15, 2021 at 10:23 AM Eric Sandeen <sandeen@redhat.com> 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.

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

[Jan Kara]

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 <sandeen@redhat.com>

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 <jack@suse.com>
SUSE Labs, CR

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

[Jan Kara]

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 <sandeen@redhat.com>

I agree. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								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 <jack@suse.com>
SUSE Labs, CR

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

[Christoph Hellwig]

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 <sandeen@redhat.com>
> 
> 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.

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

[Eric Sandeen]

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 <sandeen@redhat.com>
>>
>> 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

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

[Darrick J. Wong]

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 <sandeen@redhat.com>
> > 
> > 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

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

[Christoph Hellwig]

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.