On Fri, Sep 15, 2017 at 04:42:27PM -0600, Ross Zwisler wrote: > On Thu, Sep 14, 2017 at 02:57:41PM +0800, Eryu Guan wrote: > > Hi Ross, > > > > On Mon, Sep 11, 2017 at 02:01:03PM -0600, Ross Zwisler wrote: > > > This adds a regression test for the following kernel patch: > > > > > > xfs: always use DAX if mount option is used > > > > > > This test will also pass with kernel v4.14-rc1 and beyond because the XFS > > > DAX I/O mount option has been disabled (but not removed), so the > > > "chattr -x" to turn off DAX doesn't actually do anything. > > > > > > Signed-off-by: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx> > > > Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx> > > > --- > > > > > > Changes since v1: > > > - Use perf instead of tracepoints to detect whether DAX is used. (Dan) > > > > Thanks for the test! But I agreed with Dave here, it doesn't seem like a > > good idea to depend on the kernel tracepoints in a test, but I can't > > think of a better solution either, so I didn't get to this patch > > earlier.. > > > > Before XFS disabled the ability to switch on & off per-inode DAX flag, > > the x flag was only shown after an explicit 'chattr +x', even if XFS was > > mounted with dax option, e.g. Of course. lsattr reports the *on-disk inode flag state*. It does not report whether the file is using that feature or not, just whether the flag is set on disk. Remember, lsattr does not report a "sum of all config states for a feature". It reports on disk state and on disk state only. It follows the same semantics as other flags that have equivalent mount options like dirsync, sync, noatime, etc. > > # mkfs -t xfs -f /dev/ram0 > > # mount -o dax /dev/ram0 /mnt/xfs > > # echo "test" > /mnt/xfs/testfile > > # xfs_io -c "lsattr" /mnt/xfs/testfile > > ---------------- /mnt/xfs/testfile > > # xfs_io -c "chattr +x" /mnt/xfs/testfile > > # xfs_io -c "lsattr" /mnt/xfs/testfile > > ---------------x /mnt/xfs/testfile > > XFS actually still works this way, you just don't get dax now when you chattr > +x. :-/ But the inode flag is actually still there, gets updated by chattr > and can be listed with lsattr. > > Actually, that feels like a really bad situation to be in - Christoph & Dave, > should we do more to remove the flag as long as it's not working? i.e. remove > it from the lsattr output and make "chattr +x" fail with -EINVAL or similar? How about we concentrate on fixing the problem that led us to disable it temporarily in XFS? Then it can be re-enabled and problems like this go away... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
