Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-12-01 Thread Michal Suchánek
On Thu, Nov 12, 2020 at 08:08:49AM -0800, Ira Weiny wrote:
> On Thu, Nov 12, 2020 at 04:10:46PM +0100, Jan Kara wrote:
> > On Thu 12-11-20 12:12:17, Michal Suchánek wrote:
> > > On Thu, Nov 12, 2020 at 12:49:52PM +1100, Dave Chinner wrote:
> > > > On Wed, Nov 11, 2020 at 11:28:48AM +0100, Michal Suchánek wrote:
> > > > > On Tue, Nov 10, 2020 at 08:08:23AM +1100, Dave Chinner wrote:
> > > > > > On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> > > > > > > On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > > > > > > > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > > > > > > > xfs accepts both dax and dax_enum but shows only dax_enum. 
> > > > > > > > > Show both
> > > > > > > > > options.
> > > > > > > > > 
> > > > > > > > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a 
> > > > > > > > > tri-state")
> > > > > > > > > Signed-off-by: Michal Suchanek 
> > > > > > > > > ---
> > > > > > > > >  fs/xfs/xfs_super.c | 2 +-
> > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > > > > > > index e3e229e52512..a3b3840d 100644
> > > > > > > > > --- a/fs/xfs/xfs_super.c
> > > > > > > > > +++ b/fs/xfs/xfs_super.c
> > > > > > > > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > > > > > > > >   { XFS_MOUNT_GRPID,  ",grpid" },
> > > > > > > > >   { XFS_MOUNT_DISCARD,",discard" },
> > > > > > > > >   { XFS_MOUNT_LARGEIO,",largeio" },
> > > > > > > > > - { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > > > > > > > + { XFS_MOUNT_DAX_ALWAYS, 
> > > > > > > > > ",dax,dax=always" },
> 
> I only caught this thread yesterday sorry about that...
> 
> FWIW I think it odd to have this string.  Because what does 'dax,dax=always'
> mean?  dax=always is not the only way the FS can support DAX.  The default
> 'dax=inode' also means the FS supports DAX.  So I'm not sure this is really
> even giving you what you want.
The dax flag which was accepted previously and is still accepted now
means dax=always. It just shows both the old and new form of the flag.
> 
> > > > > > > > 
> > > > > > > > NAK, programs that require DAX semantics for files stored on 
> > > > > > > > XFS must
> > > > > > > > call statx to detect the STATX_ATTR_DAX flag, as outlined in 
> > > > > > > > "Enabling
> > > > > > > > DAX on xfs" in Documentation/filesystems/dax.txt.
> > > > > > > statx can be used to query S_DAX.  NOTE that only regular files 
> > > > > > > will
> > > > > > > ever have S_DAX set and therefore statx will never indicate that 
> > > > > > > S_DAX
> > > > > > > is set on directories.
> > > > > > 
> > > > > > Yup, by design.
> > > > > > 
> > > > > > The application doesn't need to do anything complex to make this
> > > > > > work. If the app wants to use DAX, then it should use
> > > > > > FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
> > > > > > for it's data dirs and files, and then STATX_ATTR_DAX will *always*
> > > > > > tell it whether DAX is actively in use at runtime. It's pretty
> > > > > > simple, really.
> > > > > > 
> > > > > > > The filesystem may not have any files so statx cannot be used.
> > > > > > 
> > > > > > Really?  The app or installer is about to *write to the fs* and has
> > > > > > all the permissions it needs to modify the contents of the fs. It's
> > > > > > pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
> > > > > > then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
> > > > > > see if DAX is active or not.
> > > > > 
> > > > > Have you ever seen a 'wizard' style installer?
> > > > 
> > > > I wrote my first one in 1995 on Windows NT 3.51 using Installshield.
> 
> I'm confused about this talk of an installer.  If 1 FS supports DAX and the
> other, say 10, do not; what happens when you install something special which
> to support DAX?  But what if the user of that software uses one of the other
> 10 FS's?
> 
> I think it has already been mentioned that the software needs this check at 
> run
> time to be correct.

Sure. The installer is the first part the gets in contact with the
change. There is no point trying to run the application if your system
does not support dax so the installer chacks that early on. I think that
makes sense. You may need to re-format the filesystem with different
block size or other options to enable dax.

> 
> > > > 
> > > > > Like one that firsts asks what to install, and then presents a list of
> > > > > suitable locations that have enough space, supported filesystem 
> > > > > features
> > > > > enabled, and whatnot?
> > > > 
> > > > Hold on, 1995 is calling me. The application I was packaging used
> > > > ACLs. But the NTFS version created by windows NT 3.1 was
> > > > incompatible as ACL support didn't arrive until NT 3.51 and service
> > > > pack 4(?) for 

Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-12 Thread Ira Weiny
On Thu, Nov 12, 2020 at 04:10:46PM +0100, Jan Kara wrote:
> On Thu 12-11-20 12:12:17, Michal Suchánek wrote:
> > On Thu, Nov 12, 2020 at 12:49:52PM +1100, Dave Chinner wrote:
> > > On Wed, Nov 11, 2020 at 11:28:48AM +0100, Michal Suchánek wrote:
> > > > On Tue, Nov 10, 2020 at 08:08:23AM +1100, Dave Chinner wrote:
> > > > > On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> > > > > > On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > > > > > > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > > > > > > xfs accepts both dax and dax_enum but shows only dax_enum. Show 
> > > > > > > > both
> > > > > > > > options.
> > > > > > > > 
> > > > > > > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a 
> > > > > > > > tri-state")
> > > > > > > > Signed-off-by: Michal Suchanek 
> > > > > > > > ---
> > > > > > > >  fs/xfs/xfs_super.c | 2 +-
> > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > > > > > index e3e229e52512..a3b3840d 100644
> > > > > > > > --- a/fs/xfs/xfs_super.c
> > > > > > > > +++ b/fs/xfs/xfs_super.c
> > > > > > > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > > > > > > > { XFS_MOUNT_GRPID,  ",grpid" },
> > > > > > > > { XFS_MOUNT_DISCARD,",discard" },
> > > > > > > > { XFS_MOUNT_LARGEIO,",largeio" },
> > > > > > > > -   { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > > > > > > +   { XFS_MOUNT_DAX_ALWAYS, 
> > > > > > > > ",dax,dax=always" },

I only caught this thread yesterday sorry about that...

FWIW I think it odd to have this string.  Because what does 'dax,dax=always'
mean?  dax=always is not the only way the FS can support DAX.  The default
'dax=inode' also means the FS supports DAX.  So I'm not sure this is really
even giving you what you want.

> > > > > > > 
> > > > > > > NAK, programs that require DAX semantics for files stored on XFS 
> > > > > > > must
> > > > > > > call statx to detect the STATX_ATTR_DAX flag, as outlined in 
> > > > > > > "Enabling
> > > > > > > DAX on xfs" in Documentation/filesystems/dax.txt.
> > > > > > statx can be used to query S_DAX.  NOTE that only regular files will
> > > > > > ever have S_DAX set and therefore statx will never indicate that 
> > > > > > S_DAX
> > > > > > is set on directories.
> > > > > 
> > > > > Yup, by design.
> > > > > 
> > > > > The application doesn't need to do anything complex to make this
> > > > > work. If the app wants to use DAX, then it should use
> > > > > FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
> > > > > for it's data dirs and files, and then STATX_ATTR_DAX will *always*
> > > > > tell it whether DAX is actively in use at runtime. It's pretty
> > > > > simple, really.
> > > > > 
> > > > > > The filesystem may not have any files so statx cannot be used.
> > > > > 
> > > > > Really?  The app or installer is about to *write to the fs* and has
> > > > > all the permissions it needs to modify the contents of the fs. It's
> > > > > pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
> > > > > then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
> > > > > see if DAX is active or not.
> > > > 
> > > > Have you ever seen a 'wizard' style installer?
> > > 
> > > I wrote my first one in 1995 on Windows NT 3.51 using Installshield.

I'm confused about this talk of an installer.  If 1 FS supports DAX and the
other, say 10, do not; what happens when you install something special which
to support DAX?  But what if the user of that software uses one of the other
10 FS's?

I think it has already been mentioned that the software needs this check at run
time to be correct.

> > > 
> > > > Like one that firsts asks what to install, and then presents a list of
> > > > suitable locations that have enough space, supported filesystem features
> > > > enabled, and whatnot?
> > > 
> > > Hold on, 1995 is calling me. The application I was packaging used
> > > ACLs. But the NTFS version created by windows NT 3.1 was
> > > incompatible as ACL support didn't arrive until NT 3.51 and service
> > > pack 4(?) for NT 3.1. Yes, I had to write code to probe the
> > > filesystems to detect whether ACL support was available or not by
> > > -trying to create an ACL-.
> > > 
> > > I guess you could say "been there, done that, learnt the lesson".
> > So we are trying to be as bad as Windows now?
> > > 
> > > > So to present a list of mountpoints that support DAX one has to scribble
> > > > over every mountpoint on the system?
> > > 
> > > If you are filtering storage options presented to the user by
> > > supported features, then you have to probe for them in some way.
> > > And that means you have to consider that many option filesystem
> > > features that applications use cannot be detected via mount options
> > > checking 

Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-12 Thread Jan Kara
On Thu 12-11-20 12:12:17, Michal Suchánek wrote:
> On Thu, Nov 12, 2020 at 12:49:52PM +1100, Dave Chinner wrote:
> > On Wed, Nov 11, 2020 at 11:28:48AM +0100, Michal Suchánek wrote:
> > > On Tue, Nov 10, 2020 at 08:08:23AM +1100, Dave Chinner wrote:
> > > > On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> > > > > On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > > > > > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > > > > > xfs accepts both dax and dax_enum but shows only dax_enum. Show 
> > > > > > > both
> > > > > > > options.
> > > > > > > 
> > > > > > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> > > > > > > Signed-off-by: Michal Suchanek 
> > > > > > > ---
> > > > > > >  fs/xfs/xfs_super.c | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > > > > index e3e229e52512..a3b3840d 100644
> > > > > > > --- a/fs/xfs/xfs_super.c
> > > > > > > +++ b/fs/xfs/xfs_super.c
> > > > > > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > > > > > >   { XFS_MOUNT_GRPID,  ",grpid" },
> > > > > > >   { XFS_MOUNT_DISCARD,",discard" },
> > > > > > >   { XFS_MOUNT_LARGEIO,",largeio" },
> > > > > > > - { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > > > > > + { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
> > > > > > 
> > > > > > NAK, programs that require DAX semantics for files stored on XFS 
> > > > > > must
> > > > > > call statx to detect the STATX_ATTR_DAX flag, as outlined in 
> > > > > > "Enabling
> > > > > > DAX on xfs" in Documentation/filesystems/dax.txt.
> > > > > statx can be used to query S_DAX.  NOTE that only regular files will
> > > > > ever have S_DAX set and therefore statx will never indicate that S_DAX
> > > > > is set on directories.
> > > > 
> > > > Yup, by design.
> > > > 
> > > > The application doesn't need to do anything complex to make this
> > > > work. If the app wants to use DAX, then it should use
> > > > FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
> > > > for it's data dirs and files, and then STATX_ATTR_DAX will *always*
> > > > tell it whether DAX is actively in use at runtime. It's pretty
> > > > simple, really.
> > > > 
> > > > > The filesystem may not have any files so statx cannot be used.
> > > > 
> > > > Really?  The app or installer is about to *write to the fs* and has
> > > > all the permissions it needs to modify the contents of the fs. It's
> > > > pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
> > > > then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
> > > > see if DAX is active or not.
> > > 
> > > Have you ever seen a 'wizard' style installer?
> > 
> > I wrote my first one in 1995 on Windows NT 3.51 using Installshield.
> > 
> > > Like one that firsts asks what to install, and then presents a list of
> > > suitable locations that have enough space, supported filesystem features
> > > enabled, and whatnot?
> > 
> > Hold on, 1995 is calling me. The application I was packaging used
> > ACLs. But the NTFS version created by windows NT 3.1 was
> > incompatible as ACL support didn't arrive until NT 3.51 and service
> > pack 4(?) for NT 3.1. Yes, I had to write code to probe the
> > filesystems to detect whether ACL support was available or not by
> > -trying to create an ACL-.
> > 
> > I guess you could say "been there, done that, learnt the lesson".
> So we are trying to be as bad as Windows now?
> > 
> > > So to present a list of mountpoints that support DAX one has to scribble
> > > over every mountpoint on the system?
> > 
> > If you are filtering storage options presented to the user by
> > supported features, then you have to probe for them in some way.
> > And that means you have to consider that many option filesystem
> > features that applications use cannot be detected via mount options
> > checking the filesytem config. That is, there are features that can
> > only be discovered by actually testing whether they work or not.
> > 
> > > That sounds ridiculous.
> > 
> > Reality is a harsh mistress. :/
> > 
> > [snip the rest because you're being ridiculous]
> > 
> > Are you aware of ndctl?
> > 
> > $ ndctl list
> > [
> >   {
> > "dev":"namespace1.0",
> > "mode":"fsdax",
> > "map":"mem",
> > "size":8589934592,
> > "sector_size":512,
> > "blockdev":"pmem1"
> >   },
> >   {
> > "dev":"namespace0.0",
> > "mode":"fsdax",
> > "map":"mem",
> > "size":8589934592,
> > "sector_size":512,
> > "blockdev":"pmem0"
> >   }
> > ]
> Yes, that tells me that the device can be configured for dax. Not if the
> filesystem will use it.
> > 
> > Oh, look there are two block devices on this machine that are
> > configured for filesystem DAX (fsdax). They are /dev/pmem0 and
> > /dev/pmem1.
> > 
> > What filesytsems 

Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-12 Thread Michal Suchánek
On Thu, Nov 12, 2020 at 12:49:52PM +1100, Dave Chinner wrote:
> On Wed, Nov 11, 2020 at 11:28:48AM +0100, Michal Suchánek wrote:
> > On Tue, Nov 10, 2020 at 08:08:23AM +1100, Dave Chinner wrote:
> > > On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> > > > On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > > > > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > > > > xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> > > > > > options.
> > > > > > 
> > > > > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> > > > > > Signed-off-by: Michal Suchanek 
> > > > > > ---
> > > > > >  fs/xfs/xfs_super.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > > > index e3e229e52512..a3b3840d 100644
> > > > > > --- a/fs/xfs/xfs_super.c
> > > > > > +++ b/fs/xfs/xfs_super.c
> > > > > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > > > > > { XFS_MOUNT_GRPID,  ",grpid" },
> > > > > > { XFS_MOUNT_DISCARD,",discard" },
> > > > > > { XFS_MOUNT_LARGEIO,",largeio" },
> > > > > > -   { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > > > > +   { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
> > > > > 
> > > > > NAK, programs that require DAX semantics for files stored on XFS must
> > > > > call statx to detect the STATX_ATTR_DAX flag, as outlined in "Enabling
> > > > > DAX on xfs" in Documentation/filesystems/dax.txt.
> > > > statx can be used to query S_DAX.  NOTE that only regular files will
> > > > ever have S_DAX set and therefore statx will never indicate that S_DAX
> > > > is set on directories.
> > > 
> > > Yup, by design.
> > > 
> > > The application doesn't need to do anything complex to make this
> > > work. If the app wants to use DAX, then it should use
> > > FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
> > > for it's data dirs and files, and then STATX_ATTR_DAX will *always*
> > > tell it whether DAX is actively in use at runtime. It's pretty
> > > simple, really.
> > > 
> > > > The filesystem may not have any files so statx cannot be used.
> > > 
> > > Really?  The app or installer is about to *write to the fs* and has
> > > all the permissions it needs to modify the contents of the fs. It's
> > > pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
> > > then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
> > > see if DAX is active or not.
> > 
> > Have you ever seen a 'wizard' style installer?
> 
> I wrote my first one in 1995 on Windows NT 3.51 using Installshield.
> 
> > Like one that firsts asks what to install, and then presents a list of
> > suitable locations that have enough space, supported filesystem features
> > enabled, and whatnot?
> 
> Hold on, 1995 is calling me. The application I was packaging used
> ACLs. But the NTFS version created by windows NT 3.1 was
> incompatible as ACL support didn't arrive until NT 3.51 and service
> pack 4(?) for NT 3.1. Yes, I had to write code to probe the
> filesystems to detect whether ACL support was available or not by
> -trying to create an ACL-.
> 
> I guess you could say "been there, done that, learnt the lesson".
So we are trying to be as bad as Windows now?
> 
> > So to present a list of mountpoints that support DAX one has to scribble
> > over every mountpoint on the system?
> 
> If you are filtering storage options presented to the user by
> supported features, then you have to probe for them in some way.
> And that means you have to consider that many option filesystem
> features that applications use cannot be detected via mount options
> checking the filesytem config. That is, there are features that can
> only be discovered by actually testing whether they work or not.
> 
> > That sounds ridiculous.
> 
> Reality is a harsh mistress. :/
> 
> [snip the rest because you're being ridiculous]
> 
> Are you aware of ndctl?
> 
> $ ndctl list
> [
>   {
> "dev":"namespace1.0",
> "mode":"fsdax",
> "map":"mem",
> "size":8589934592,
> "sector_size":512,
> "blockdev":"pmem1"
>   },
>   {
> "dev":"namespace0.0",
> "mode":"fsdax",
> "map":"mem",
> "size":8589934592,
> "sector_size":512,
> "blockdev":"pmem0"
>   }
> ]
Yes, that tells me that the device can be configured for dax. Not if the
filesystem will use it.
> 
> Oh, look there are two block devices on this machine that are
> configured for filesystem DAX (fsdax). They are /dev/pmem0 and
> /dev/pmem1.
> 
> What filesytsems are on them?
> 
> $ lsblk -o NAME,SIZE,FSTYPE,MOUNTPOINT /dev/pmem0 /dev/pmem1
> NAME  SIZE FSTYPE MOUNTPOINT
> pmem1   8G ext4   /mnt/test
> pmem0   8G xfs/mnt/scratch
> $
> 
> One XFs, one ext4, both of which will be using DAX capable unless
> the dax=never mount option is set. Which:
Or the bock 

Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-11 Thread Dave Chinner
On Wed, Nov 11, 2020 at 11:28:48AM +0100, Michal Suchánek wrote:
> On Tue, Nov 10, 2020 at 08:08:23AM +1100, Dave Chinner wrote:
> > On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> > > On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > > > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > > > xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> > > > > options.
> > > > > 
> > > > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> > > > > Signed-off-by: Michal Suchanek 
> > > > > ---
> > > > >  fs/xfs/xfs_super.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > > index e3e229e52512..a3b3840d 100644
> > > > > --- a/fs/xfs/xfs_super.c
> > > > > +++ b/fs/xfs/xfs_super.c
> > > > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > > > >   { XFS_MOUNT_GRPID,  ",grpid" },
> > > > >   { XFS_MOUNT_DISCARD,",discard" },
> > > > >   { XFS_MOUNT_LARGEIO,",largeio" },
> > > > > - { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > > > + { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
> > > > 
> > > > NAK, programs that require DAX semantics for files stored on XFS must
> > > > call statx to detect the STATX_ATTR_DAX flag, as outlined in "Enabling
> > > > DAX on xfs" in Documentation/filesystems/dax.txt.
> > > statx can be used to query S_DAX.  NOTE that only regular files will
> > > ever have S_DAX set and therefore statx will never indicate that S_DAX
> > > is set on directories.
> > 
> > Yup, by design.
> > 
> > The application doesn't need to do anything complex to make this
> > work. If the app wants to use DAX, then it should use
> > FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
> > for it's data dirs and files, and then STATX_ATTR_DAX will *always*
> > tell it whether DAX is actively in use at runtime. It's pretty
> > simple, really.
> > 
> > > The filesystem may not have any files so statx cannot be used.
> > 
> > Really?  The app or installer is about to *write to the fs* and has
> > all the permissions it needs to modify the contents of the fs. It's
> > pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
> > then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
> > see if DAX is active or not.
> 
> Have you ever seen a 'wizard' style installer?

I wrote my first one in 1995 on Windows NT 3.51 using Installshield.

> Like one that firsts asks what to install, and then presents a list of
> suitable locations that have enough space, supported filesystem features
> enabled, and whatnot?

Hold on, 1995 is calling me. The application I was packaging used
ACLs. But the NTFS version created by windows NT 3.1 was
incompatible as ACL support didn't arrive until NT 3.51 and service
pack 4(?) for NT 3.1. Yes, I had to write code to probe the
filesystems to detect whether ACL support was available or not by
-trying to create an ACL-.

I guess you could say "been there, done that, learnt the lesson".

> So to present a list of mountpoints that support DAX one has to scribble
> over every mountpoint on the system?

If you are filtering storage options presented to the user by
supported features, then you have to probe for them in some way.
And that means you have to consider that many option filesystem
features that applications use cannot be detected via mount options
checking the filesytem config. That is, there are features that can
only be discovered by actually testing whether they work or not.

> That sounds ridiculous.

Reality is a harsh mistress. :/

[snip the rest because you're being ridiculous]

Are you aware of ndctl?

$ ndctl list
[
  {
"dev":"namespace1.0",
"mode":"fsdax",
"map":"mem",
"size":8589934592,
"sector_size":512,
"blockdev":"pmem1"
  },
  {
"dev":"namespace0.0",
"mode":"fsdax",
"map":"mem",
"size":8589934592,
"sector_size":512,
"blockdev":"pmem0"
  }
]

Oh, look there are two block devices on this machine that are
configured for filesystem DAX (fsdax). They are /dev/pmem0 and
/dev/pmem1.

What filesytsems are on them?

$ lsblk -o NAME,SIZE,FSTYPE,MOUNTPOINT /dev/pmem0 /dev/pmem1
NAME  SIZE FSTYPE MOUNTPOINT
pmem1   8G ext4   /mnt/test
pmem0   8G xfs/mnt/scratch
$

One XFs, one ext4, both of which will be using DAX capable unless
the dax=never mount option is set. Which:

$ mount  |grep pmem
/dev/pmem0 on /mnt/scratch type xfs 
(rw,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/pmem1 on /mnt/test type ext4 (rw,relatime)
$

is not set on either mount.

Hence both filesystems at DAX capable and enabled, and should be
presented as options to the user as such.

And all this comes about because DAX is a property of the block
device, not the filesystem. Hence the only time a DAX capable
filesystem on a 

Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-11 Thread Michal Suchánek
On Tue, Nov 10, 2020 at 08:08:23AM +1100, Dave Chinner wrote:
> On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> > On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > > xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> > > > options.
> > > > 
> > > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> > > > Signed-off-by: Michal Suchanek 
> > > > ---
> > > >  fs/xfs/xfs_super.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > index e3e229e52512..a3b3840d 100644
> > > > --- a/fs/xfs/xfs_super.c
> > > > +++ b/fs/xfs/xfs_super.c
> > > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > > > { XFS_MOUNT_GRPID,  ",grpid" },
> > > > { XFS_MOUNT_DISCARD,",discard" },
> > > > { XFS_MOUNT_LARGEIO,",largeio" },
> > > > -   { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > > +   { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
> > > 
> > > NAK, programs that require DAX semantics for files stored on XFS must
> > > call statx to detect the STATX_ATTR_DAX flag, as outlined in "Enabling
> > > DAX on xfs" in Documentation/filesystems/dax.txt.
> > statx can be used to query S_DAX.  NOTE that only regular files will
> > ever have S_DAX set and therefore statx will never indicate that S_DAX
> > is set on directories.
> 
> Yup, by design.
> 
> The application doesn't need to do anything complex to make this
> work. If the app wants to use DAX, then it should use
> FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
> for it's data dirs and files, and then STATX_ATTR_DAX will *always*
> tell it whether DAX is actively in use at runtime. It's pretty
> simple, really.
> 
> > The filesystem may not have any files so statx cannot be used.
> 
> Really?  The app or installer is about to *write to the fs* and has
> all the permissions it needs to modify the contents of the fs. It's
> pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
> then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
> see if DAX is active or not.

Have you ever seen a 'wizard' style installer?

Like one that firsts asks what to install, and then presents a list of
suitable locations that have enough space, supported filesystem features
enabled, and whatnot?

So to present a list of mountpoints that support DAX one has to scribble
over every mountpoint on the system?

That sounds ridiculous.
> 
> However, keep in mind that from a system design perspective having
> the installer detect DAX properties to make application level
> install/config decisions is problematic from a lot of different
> angles.
> 
> - DAX is property of the *block device*, not the filesystem, so the
>   filesystem can make arbitrary decisions on whether to use DAX or
>   not to access data and these can change at any time without
>   warning.
It is property of the mount point. Device supporting DAX is useless on
its own - the filesystem must support and enable it as well. Filesystem
support on its own is useless - it must be on a device that supports
DAX, has matching block sise, it must be enabled at mount time.

Then don't be surprised that the users use 'creative' ways to determine
the information the kernel chooses to not share with them.
> 
> - Some filesystems may not have any user visible signs they are
>   using DAX to access data except for STATX_ATTR_DAX because they
>   always use DAX and only work on DAX capable block devices. e.g
>   NVFS.
That's broken indeed.
> 
> - For filesystems where DAX is optional, the user can -always-
>   change the dax state of the fs (mount options) or even parts of
>   the filesystem (per inode flags) at any time after the installer
>   has run.
The user can do a lot of thing indeed. They can even unmount the
filesystem.
> 
> - The application might be storing it's data on a different
>   filesystem that isn't mounted at install time, so the installer
>   has no chance of detecting that the application is going to use
>   DAX enabled storage.
> 
> IOWs, the installer cannot make decisions based on DAX state on
> behalf of applications because it does not know what environment the
> application is going to be configured to run in.  DAX can only be
> deteted reliably by the application at runtime inside it's
> production execution environment.
It can check that the mount point is suitable at the time of
installation. Of course, if the system is reconfigured afterwards the
application might fail. However, checking the requirements prior to
installtion still provides valuable feedback to the user. They can be
sure that the system was configured properly.

Thanks

Michal


Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-10 Thread Christoph Hellwig
On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> options.

Which is very much intentional.  The -o dax was an experimental hack
that fortunately is gone now.


Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-09 Thread Dave Chinner
On Mon, Nov 09, 2020 at 09:27:05PM +0100, Michal Suchánek wrote:
> On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> > On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > > xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> > > options.
> > > 
> > > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> > > Signed-off-by: Michal Suchanek 
> > > ---
> > >  fs/xfs/xfs_super.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index e3e229e52512..a3b3840d 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > >   { XFS_MOUNT_GRPID,  ",grpid" },
> > >   { XFS_MOUNT_DISCARD,",discard" },
> > >   { XFS_MOUNT_LARGEIO,",largeio" },
> > > - { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > > + { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
> > 
> > NAK, programs that require DAX semantics for files stored on XFS must
> > call statx to detect the STATX_ATTR_DAX flag, as outlined in "Enabling
> > DAX on xfs" in Documentation/filesystems/dax.txt.
> statx can be used to query S_DAX.  NOTE that only regular files will
> ever have S_DAX set and therefore statx will never indicate that S_DAX
> is set on directories.

Yup, by design.

The application doesn't need to do anything complex to make this
work. If the app wants to use DAX, then it should use
FS_IOC_FS{GS}ETXATTR to always set the on disk per inode DAX flags
for it's data dirs and files, and then STATX_ATTR_DAX will *always*
tell it whether DAX is actively in use at runtime. It's pretty
simple, really.

> The filesystem may not have any files so statx cannot be used.

Really?  The app or installer is about to *write to the fs* and has
all the permissions it needs to modify the contents of the fs. It's
pretty simple to create a tmpdir, set the DAX flag on the tmpdir,
then create a tmpfile in the tmpdir and run STATX_ATTR_DAX on it to
see if DAX is active or not.

However, keep in mind that from a system design perspective having
the installer detect DAX properties to make application level
install/config decisions is problematic from a lot of different
angles.

- DAX is property of the *block device*, not the filesystem, so the
  filesystem can make arbitrary decisions on whether to use DAX or
  not to access data and these can change at any time without
  warning.

- Some filesystems may not have any user visible signs they are
  using DAX to access data except for STATX_ATTR_DAX because they
  always use DAX and only work on DAX capable block devices. e.g
  NVFS.

- For filesystems where DAX is optional, the user can -always-
  change the dax state of the fs (mount options) or even parts of
  the filesystem (per inode flags) at any time after the installer
  has run.

- The application might be storing it's data on a different
  filesystem that isn't mounted at install time, so the installer
  has no chance of detecting that the application is going to use
  DAX enabled storage.

IOWs, the installer cannot make decisions based on DAX state on
behalf of applications because it does not know what environment the
application is going to be configured to run in.  DAX can only be
deteted reliably by the application at runtime inside it's
production execution environment.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com


Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-09 Thread Michal Suchánek
On Mon, Nov 09, 2020 at 11:24:19AM -0800, Darrick J. Wong wrote:
> On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> > xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> > options.
> > 
> > Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> > Signed-off-by: Michal Suchanek 
> > ---
> >  fs/xfs/xfs_super.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index e3e229e52512..a3b3840d 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -163,7 +163,7 @@ xfs_fs_show_options(
> > { XFS_MOUNT_GRPID,  ",grpid" },
> > { XFS_MOUNT_DISCARD,",discard" },
> > { XFS_MOUNT_LARGEIO,",largeio" },
> > -   { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> > +   { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
> 
> NAK, programs that require DAX semantics for files stored on XFS must
> call statx to detect the STATX_ATTR_DAX flag, as outlined in "Enabling
> DAX on xfs" in Documentation/filesystems/dax.txt.
statx can be used to query S_DAX.  NOTE that only regular files will
ever have S_DAX set and therefore statx will never indicate that S_DAX
is set on directories.

The filesystem may not have any files so statx cannot be used.

If you reject this method of detection please provide an actual usable
alternative.

Thanks

Michal

> 
> --D
> 
> > { XFS_MOUNT_DAX_NEVER,  ",dax=never" },
> > { 0, NULL }
> > };
> > -- 
> > 2.26.2
> > 


Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-09 Thread Darrick J. Wong
On Mon, Nov 09, 2020 at 08:10:08PM +0100, Michal Suchanek wrote:
> xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> options.
> 
> Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> Signed-off-by: Michal Suchanek 
> ---
>  fs/xfs/xfs_super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e3e229e52512..a3b3840d 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -163,7 +163,7 @@ xfs_fs_show_options(
>   { XFS_MOUNT_GRPID,  ",grpid" },
>   { XFS_MOUNT_DISCARD,",discard" },
>   { XFS_MOUNT_LARGEIO,",largeio" },
> - { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> + { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },

NAK, programs that require DAX semantics for files stored on XFS must
call statx to detect the STATX_ATTR_DAX flag, as outlined in "Enabling
DAX on xfs" in Documentation/filesystems/dax.txt.

--D

>   { XFS_MOUNT_DAX_NEVER,  ",dax=never" },
>   { 0, NULL }
>   };
> -- 
> 2.26.2
> 


Re: [PATCH 1/2] xfs: show the dax option in mount options.

2020-11-09 Thread Eric Sandeen
On 11/9/20 1:10 PM, Michal Suchanek wrote:
> xfs accepts both dax and dax_enum but shows only dax_enum. Show both
> options.

Should probably indicate why this is needed in the changelog.

> Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
> Signed-off-by: Michal Suchanek 

This seems ok to me.  I wanted to make sure that "mount -o dax,dax=always"
was accepted properly, because a remount looks to /proc/mounts for existing
options and re-uses what it finds.  It does seem to work correctly.

It's a little weird to emit 2 equivalent options when only one was given, but if
legacy apps rely on finding a bare "dax," then I guess it seems harmless 
enough...

Reviewed-by: Eric Sandeen 

> ---
>  fs/xfs/xfs_super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e3e229e52512..a3b3840d 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -163,7 +163,7 @@ xfs_fs_show_options(
>   { XFS_MOUNT_GRPID,  ",grpid" },
>   { XFS_MOUNT_DISCARD,",discard" },
>   { XFS_MOUNT_LARGEIO,",largeio" },
> - { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
> + { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
>   { XFS_MOUNT_DAX_NEVER,  ",dax=never" },
>   { 0, NULL }
>   };
> 


[PATCH 1/2] xfs: show the dax option in mount options.

2020-11-09 Thread Michal Suchanek
xfs accepts both dax and dax_enum but shows only dax_enum. Show both
options.

Fixes: 8d6c3446ec23 ("fs/xfs: Make DAX mount option a tri-state")
Signed-off-by: Michal Suchanek 
---
 fs/xfs/xfs_super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e3e229e52512..a3b3840d 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -163,7 +163,7 @@ xfs_fs_show_options(
{ XFS_MOUNT_GRPID,  ",grpid" },
{ XFS_MOUNT_DISCARD,",discard" },
{ XFS_MOUNT_LARGEIO,",largeio" },
-   { XFS_MOUNT_DAX_ALWAYS, ",dax=always" },
+   { XFS_MOUNT_DAX_ALWAYS, ",dax,dax=always" },
{ XFS_MOUNT_DAX_NEVER,  ",dax=never" },
{ 0, NULL }
};
-- 
2.26.2