On Fri, May 13, 2016 at 10:53:37AM -0700, Dan Williams wrote:

On Fri, May 13, 2016 at 10:33 AM, Jeff Moyer [off-list ref] wrote:

quoted

Dan Williams [off-list ref] writes:

quoted

On Thu, May 12, 2016 at 10:43 AM, Jon Derrick
[off-list ref] wrote:

quoted

When DAX is not compiled into the kernel or the device does not support
direct-access, the block device file's inode flags are fully cleared.
This patch changes it to only clear the S_DAX flag when DAX is disabled.

This reverts to i_flags behavior prior to
bbab37ddc20bae4709bca8745c128c4f46fe63c5

Signed-off-by: Jon Derrick <redacted>
---
 fs/block_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 20a2c02..d4fa725 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c

@@ -1208,7 +1208,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
                if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && disk->fops->direct_access)
                        bdev->bd_inode->i_flags = S_DAX;
                else
-                       bdev->bd_inode->i_flags = 0;
+                       bdev->bd_inode->i_flags &= ~S_DAX;

Setting S_DAX is atomic, but the above change makes it a non-atomic
read-modify-write.  Do we need exclusion / locking in this path?

First, I don't see how you'd ever get a read-modify-write here, as S_DAX
surely won't ever be set if direct_access isn't supported.  Second,
there is existing code that already does this very thing.  See further
down in __blkdev_get:

                        if (!ret) {
                                bd_set_size(bdev,(loff_t)get_capacity(disk)<<9);
                                if (!blkdev_dax_capable(bdev))
                                        bdev->bd_inode->i_flags &= ~S_DAX;
                        }

What relies on S_DAX being set or cleared atomically?

So, when I wrote the above "&= ~S_DAX" I was worried about the
non-atomic update with respect to the BLKDAXSET ioctl, but we killed
BLKDAXSET so the worry went away.  Now an inode flag setting ioctl is
back for the S_HIPRI case.  Can that collide with these other flag
touches?

The S_DAX case is unclear to me, but the new ioctl certainly does need a i_mutex lock and to use inode_set_flags.