xfs
[Top] [All Lists]

Re: [PATCH 8/8] xfs: add initial DAX support

To: Boaz Harrosh <boaz@xxxxxxxxxxxxx>
Subject: Re: [PATCH 8/8] xfs: add initial DAX support
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 25 Mar 2015 08:25:50 +1100
Cc: xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, willy@xxxxxxxxxxxxxxx, jack@xxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <55115E20.7090105@xxxxxxxxxxxxx>
References: <1427194266-2885-1-git-send-email-david@xxxxxxxxxxxxx> <1427194266-2885-9-git-send-email-david@xxxxxxxxxxxxx> <55115E20.7090105@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Mar 24, 2015 at 02:52:48PM +0200, Boaz Harrosh wrote:
> On 03/24/2015 12:51 PM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > @@ -363,6 +365,10 @@ xfs_parseargs(
> >                     mp->m_flags |= XFS_MOUNT_DISCARD;
> >             } else if (!strcmp(this_char, MNTOPT_NODISCARD)) {
> >                     mp->m_flags &= ~XFS_MOUNT_DISCARD;
> > +#ifdef CONFIG_FS_DAX
> > +           } else if (!strcmp(this_char, MNTOPT_DAX)) {
> > +                   mp->m_flags |= XFS_MOUNT_DAX;
> 
> Hi
> 
> So what I see, (I might be wrong), is that once this flag is set here the
> fs (At above xfs_diflags_to_iflags() ) will start serving DAX inodes.

No, it won't, because...

> > @@ -1501,6 +1508,20 @@ xfs_fs_fill_super(
> >     if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
> >             sb->s_flags |= MS_I_VERSION;
> >  
> > +   if (mp->m_flags & XFS_MOUNT_DAX) {
> > +           xfs_warn(mp,
> > +   "DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
> > +           if (sb->s_blocksize != PAGE_SIZE) {
> > +                   xfs_alert(mp,
> > +           "Filesystem block size invalid for DAX Turning DAX off.");
> > +                   mp->m_flags &= ~XFS_MOUNT_DAX;
> > +           } else if (!sb->s_bdev->bd_disk->fops->direct_access) {
> > +                   xfs_alert(mp,
> > +           "Block device does not support DAX Turning DAX off.");
> > +                   mp->m_flags &= ~XFS_MOUNT_DAX;
> > +           }
> > +   }

We run these tests and then clear the XFS_MOUNT_DAX flag later.

> If we agree about the s_flags MS_MOUNT_DAX  then we can define a
>       if (MNTOPT_DAX)
>               dax_enable_if_supported(sb);

No, I don't see any reason for a mount flag for this, because we
do not want to be stuck relying on a mount option forever. Once
This code has been shaken out, I fully intend DAX to be turned on
automatically for any device that supports, and for it to be turned
on and off on a per-inode basis through on-disk inode flags. i.e.
the mount option is really a temporary solution and I don't want to
code in any dependencies on DAX being mount wide existing.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>