Re: [PATCH 06/11] xfs: factor out a xfs_setup_dax helper

[Dan Williams]

On Tue, Oct 19, 2021 at 12:24 AM Christoph Hellwig  wrote:
>
> On Mon, Oct 18, 2021 at 09:43:51AM -0700, Darrick J. Wong wrote:
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -339,6 +339,32 @@ xfs_buftarg_is_dax(
> > > bdev_nr_sectors(bt->bt_bdev));
> > >  }
> > >
> > > +static int
> > > +xfs_setup_dax(
> >
> > /me wonders if this should be named xfs_setup_dax_always, since this
> > doesn't handle the dax=inode mode?
>
> Sure, why not.

I went ahead and made that change locally.

>
> > The only reason I bring that up is that Eric reminded me a while ago
> > that we don't actually print any kind of EXPERIMENTAL warning for the
> > auto-detection behavior.
>
> Yes, I actually noticed that as well when preparing this series.

The rest looks good to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 06/11] xfs: factor out a xfs_setup_dax helper

[Christoph Hellwig]

On Mon, Oct 18, 2021 at 09:43:51AM -0700, Darrick J. Wong wrote:
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -339,6 +339,32 @@ xfs_buftarg_is_dax(
> > bdev_nr_sectors(bt->bt_bdev));
> >  }
> >  
> > +static int
> > +xfs_setup_dax(
> 
> /me wonders if this should be named xfs_setup_dax_always, since this
> doesn't handle the dax=inode mode?

Sure, why not.

> The only reason I bring that up is that Eric reminded me a while ago
> that we don't actually print any kind of EXPERIMENTAL warning for the
> auto-detection behavior.

Yes, I actually noticed that as well when preparing this series.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 06/11] xfs: factor out a xfs_setup_dax helper

[Christoph Hellwig]

Factor out another DAX setup helper to simplify future changes.  Also
move the experimental warning after the checks to not clutter the log
too much if the setup failed.

Signed-off-by: Christoph Hellwig 
---
 fs/xfs/xfs_super.c | 47 +++---
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index c4e0cd1c1c8ca..d07020a8eb9e3 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -339,6 +339,32 @@ xfs_buftarg_is_dax(
bdev_nr_sectors(bt->bt_bdev));
 }
 
+static int
+xfs_setup_dax(
+   struct xfs_mount*mp)
+{
+   struct super_block  *sb = mp->m_super;
+
+   if (!xfs_buftarg_is_dax(sb, mp->m_ddev_targp) &&
+  (!mp->m_rtdev_targp || !xfs_buftarg_is_dax(sb, mp->m_rtdev_targp))) {
+   xfs_alert(mp,
+   "DAX unsupported by block device. Turning off DAX.");
+   goto disable_dax;
+   }
+
+   if (xfs_has_reflink(mp)) {
+   xfs_alert(mp, "DAX and reflink cannot be used together!");
+   return -EINVAL;
+   }
+
+   xfs_warn(mp, "DAX enabled. Warning: EXPERIMENTAL, use at your own 
risk");
+   return 0;
+
+disable_dax:
+   xfs_mount_set_dax_mode(mp, XFS_DAX_NEVER);
+   return 0;
+}
+
 STATIC int
 xfs_blkdev_get(
xfs_mount_t *mp,
@@ -1592,26 +1618,9 @@ xfs_fs_fill_super(
sb->s_flags |= SB_I_VERSION;
 
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,
-   mp->m_rtdev_targp);
-   if (!rtdev_is_dax && !datadev_is_dax) {
-   xfs_alert(mp,
-   "DAX unsupported by block device. Turning off DAX.");
-   xfs_mount_set_dax_mode(mp, XFS_DAX_NEVER);
-   }
-   if (xfs_has_reflink(mp)) {
-   xfs_alert(mp,
-   "DAX and reflink cannot be used together!");
-   error = -EINVAL;
+   error = xfs_setup_dax(mp);
+   if (error)
goto out_filestream_unmount;
-   }
}
 
if (xfs_has_discard(mp)) {
-- 
2.30.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization