This commit introduced in v5.13 prevents building FS_DAX on 32-bit ARM,
even on ARMv7 which does not have virtually aliased dcaches:
commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
It used to work fine before: I have customers using dax over pmem on
ARMv7, but this regression will likely prevent them from upgrading their
kernel.
The root of the issue here is the fact that DAX was never designed to
handle virtually aliased dcache (VIVT and VIPT with aliased dcache). It
touches the pages through their linear mapping, which is not consistent
with the userspace mappings on virtually aliased dcaches.
This patch series introduces cache_is_aliasing() with new Kconfig
options:
* ARCH_HAS_CACHE_ALIASING
* ARCH_HAS_CACHE_ALIASING_DYNAMIC
and implements it for all architectures. The "DYNAMIC" implementation
implements cache_is_aliasing() as a runtime check, which is what is
needed on architectures like 32-bit ARMV6 and ARMV6K.
With this we can basically narrow down the list of architectures which
are unsupported by DAX to those which are really affected.
Feedback is welcome,
Thanks,
Mathieu
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Dan Williams <[email protected]>
Cc: Vishal Verma <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: [email protected]
Cc: [email protected]
Mathieu Desnoyers (7):
Introduce cache_is_aliasing() across all architectures
dax: Fix incorrect list of cache aliasing architectures
erofs: Use dax_is_supported()
ext2: Use dax_is_supported()
ext4: Use dax_is_supported()
fuse: Introduce fuse_dax_is_supported()
xfs: Use dax_is_supported()
arch/arc/Kconfig | 1 +
arch/arm/include/asm/cachetype.h | 3 ++
arch/arm/mm/Kconfig | 2 ++
arch/csky/Kconfig | 1 +
arch/m68k/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/mips/include/asm/cachetype.h | 9 +++++
arch/nios2/Kconfig | 1 +
arch/nios2/include/asm/cachetype.h | 10 ++++++
arch/parisc/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/sparc/include/asm/cachetype.h | 14 ++++++++
arch/xtensa/Kconfig | 1 +
arch/xtensa/include/asm/cachetype.h | 10 ++++++
fs/Kconfig | 2 +-
fs/erofs/super.c | 10 +++---
fs/ext2/super.c | 14 ++++----
fs/ext4/super.c | 52 ++++++++++++++---------------
fs/fuse/file.c | 2 +-
fs/fuse/fuse_i.h | 36 +++++++++++++++++++-
fs/fuse/inode.c | 47 +++++++++++++-------------
fs/fuse/virtio_fs.c | 4 +--
fs/xfs/xfs_super.c | 20 +++++++----
include/linux/cacheinfo.h | 8 +++++
include/linux/dax.h | 9 +++++
mm/Kconfig | 10 ++++++
27 files changed, 198 insertions(+), 73 deletions(-)
create mode 100644 arch/mips/include/asm/cachetype.h
create mode 100644 arch/nios2/include/asm/cachetype.h
create mode 100644 arch/sparc/include/asm/cachetype.h
create mode 100644 arch/xtensa/include/asm/cachetype.h
--
2.39.2
Use dax_is_supported() to validate whether the architecture has
virtually aliased caches at mount time.
This is relevant for architectures which require a dynamic check
to validate whether they have virtually aliased data caches
(ARCH_HAS_CACHE_ALIASING_DYNAMIC=y).
Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
Signed-off-by: Mathieu Desnoyers <[email protected]>
Cc: Chandan Babu R <[email protected]>
Cc: Darrick J. Wong <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Dan Williams <[email protected]>
Cc: Vishal Verma <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
fs/xfs/xfs_super.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 764304595e8b..b27ecb11db66 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1376,14 +1376,22 @@ xfs_fs_parse_param(
case Opt_nodiscard:
parsing_mp->m_features &= ~XFS_FEAT_DISCARD;
return 0;
-#ifdef CONFIG_FS_DAX
case Opt_dax:
- xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
- return 0;
+ if (dax_is_supported()) {
+ xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
+ return 0;
+ } else {
+ xfs_warn(parsing_mp, "dax option not supported.");
+ return -EINVAL;
+ }
case Opt_dax_enum:
- xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
- return 0;
-#endif
+ if (dax_is_supported()) {
+ xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
+ return 0;
+ } else {
+ xfs_warn(parsing_mp, "dax option not supported.");
+ return -EINVAL;
+ }
/* Following mount options will be removed in September 2025 */
case Opt_ikeep:
xfs_fs_warn_deprecated(fc, param, XFS_FEAT_IKEEP, true);
--
2.39.2
Use dax_is_supported() in addition to IS_ENABLED(CONFIG_FUSE_DAX) to
validate whether CONFIG_FUSE_DAX is enabled and the architecture does
not have virtually aliased caches.
This is relevant for architectures which require a dynamic check
to validate whether they have virtually aliased data caches
(ARCH_HAS_CACHE_ALIASING_DYNAMIC=y).
Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
Signed-off-by: Mathieu Desnoyers <[email protected]>
Cc: Miklos Szeredi <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Dan Williams <[email protected]>
Cc: Vishal Verma <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
fs/fuse/file.c | 2 +-
fs/fuse/fuse_i.h | 36 +++++++++++++++++++++++++++++++++-
fs/fuse/inode.c | 47 +++++++++++++++++++++++----------------------
fs/fuse/virtio_fs.c | 4 ++--
4 files changed, 62 insertions(+), 27 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a660f1f21540..133ac8524064 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3247,6 +3247,6 @@ void fuse_init_file_inode(struct inode *inode, unsigned int flags)
init_waitqueue_head(&fi->page_waitq);
fi->writepages = RB_ROOT;
- if (IS_ENABLED(CONFIG_FUSE_DAX))
+ if (fuse_dax_is_supported())
fuse_dax_inode_init(inode, flags);
}
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 1df83eebda92..1cbe37106669 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -31,6 +31,7 @@
#include <linux/pid_namespace.h>
#include <linux/refcount.h>
#include <linux/user_namespace.h>
+#include <linux/dax.h>
/** Default max number of pages that can be used in a single read request */
#define FUSE_DEFAULT_MAX_PAGES_PER_REQ 32
@@ -979,6 +980,38 @@ static inline void fuse_sync_bucket_dec(struct fuse_sync_bucket *bucket)
rcu_read_unlock();
}
+#ifdef CONFIG_FUSE_DAX
+static inline struct fuse_inode_dax *fuse_inode_get_dax(struct fuse_inode *inode)
+{
+ return inode->dax;
+}
+
+static inline enum fuse_dax_mode fuse_conn_get_dax_mode(struct fuse_conn *fc)
+{
+ return fc->dax_mode;
+}
+
+static inline struct fuse_conn_dax *fuse_conn_get_dax(struct fuse_conn *fc)
+{
+ return fc->dax;
+}
+#else
+static inline struct fuse_inode_dax *fuse_inode_get_dax(struct fuse_inode *inode)
+{
+ return NULL;
+}
+
+static inline enum fuse_dax_mode fuse_conn_get_dax_mode(struct fuse_conn *fc)
+{
+ return FUSE_DAX_INODE_DEFAULT;
+}
+
+static inline struct fuse_conn_dax *fuse_conn_get_dax(struct fuse_conn *fc)
+{
+ return NULL;
+}
+#endif
+
/** Device operations */
extern const struct file_operations fuse_dev_operations;
@@ -1324,7 +1357,8 @@ void fuse_free_conn(struct fuse_conn *fc);
/* dax.c */
-#define FUSE_IS_DAX(inode) (IS_ENABLED(CONFIG_FUSE_DAX) && IS_DAX(inode))
+#define fuse_dax_is_supported() (IS_ENABLED(CONFIG_FUSE_DAX) && dax_is_supported())
+#define FUSE_IS_DAX(inode) (fuse_dax_is_supported() && IS_DAX(inode))
ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct iov_iter *to);
ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2a6d44f91729..030e6ce5486d 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -108,7 +108,7 @@ static struct inode *fuse_alloc_inode(struct super_block *sb)
if (!fi->forget)
goto out_free;
- if (IS_ENABLED(CONFIG_FUSE_DAX) && !fuse_dax_inode_alloc(sb, fi))
+ if (fuse_dax_is_supported() && !fuse_dax_inode_alloc(sb, fi))
goto out_free_forget;
return &fi->inode;
@@ -126,9 +126,8 @@ static void fuse_free_inode(struct inode *inode)
mutex_destroy(&fi->mutex);
kfree(fi->forget);
-#ifdef CONFIG_FUSE_DAX
- kfree(fi->dax);
-#endif
+ if (fuse_dax_is_supported())
+ kfree(fuse_inode_get_dax(fi));
kmem_cache_free(fuse_inode_cachep, fi);
}
@@ -361,7 +360,7 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
invalidate_inode_pages2(inode->i_mapping);
}
- if (IS_ENABLED(CONFIG_FUSE_DAX))
+ if (fuse_dax_is_supported())
fuse_dax_dontcache(inode, attr->flags);
}
@@ -856,14 +855,16 @@ static int fuse_show_options(struct seq_file *m, struct dentry *root)
if (sb->s_bdev && sb->s_blocksize != FUSE_DEFAULT_BLKSIZE)
seq_printf(m, ",blksize=%lu", sb->s_blocksize);
}
-#ifdef CONFIG_FUSE_DAX
- if (fc->dax_mode == FUSE_DAX_ALWAYS)
- seq_puts(m, ",dax=always");
- else if (fc->dax_mode == FUSE_DAX_NEVER)
- seq_puts(m, ",dax=never");
- else if (fc->dax_mode == FUSE_DAX_INODE_USER)
- seq_puts(m, ",dax=inode");
-#endif
+ if (fuse_dax_is_supported()) {
+ enum fuse_dax_mode dax_mode = fuse_conn_get_dax_mode(fc);
+
+ if (dax_mode == FUSE_DAX_ALWAYS)
+ seq_puts(m, ",dax=always");
+ else if (dax_mode == FUSE_DAX_NEVER)
+ seq_puts(m, ",dax=never");
+ else if (dax_mode == FUSE_DAX_INODE_USER)
+ seq_puts(m, ",dax=inode");
+ }
return 0;
}
@@ -936,7 +937,7 @@ void fuse_conn_put(struct fuse_conn *fc)
struct fuse_iqueue *fiq = &fc->iq;
struct fuse_sync_bucket *bucket;
- if (IS_ENABLED(CONFIG_FUSE_DAX))
+ if (fuse_dax_is_supported())
fuse_dax_conn_free(fc);
if (fiq->ops->release)
fiq->ops->release(fiq);
@@ -1264,7 +1265,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
min_t(unsigned int, fc->max_pages_limit,
max_t(unsigned int, arg->max_pages, 1));
}
- if (IS_ENABLED(CONFIG_FUSE_DAX)) {
+ if (fuse_dax_is_supported()) {
if (flags & FUSE_MAP_ALIGNMENT &&
!fuse_dax_check_alignment(fc, arg->map_alignment)) {
ok = false;
@@ -1331,12 +1332,12 @@ void fuse_send_init(struct fuse_mount *fm)
FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP;
-#ifdef CONFIG_FUSE_DAX
- if (fm->fc->dax)
- flags |= FUSE_MAP_ALIGNMENT;
- if (fuse_is_inode_dax_mode(fm->fc->dax_mode))
- flags |= FUSE_HAS_INODE_DAX;
-#endif
+ if (fuse_dax_is_supported()) {
+ if (fuse_conn_get_dax(fm->fc))
+ flags |= FUSE_MAP_ALIGNMENT;
+ if (fuse_is_inode_dax_mode(fuse_conn_get_dax_mode(fm->fc)))
+ flags |= FUSE_HAS_INODE_DAX;
+ }
if (fm->fc->auto_submounts)
flags |= FUSE_SUBMOUNTS;
@@ -1643,7 +1644,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
sb->s_subtype = ctx->subtype;
ctx->subtype = NULL;
- if (IS_ENABLED(CONFIG_FUSE_DAX)) {
+ if (fuse_dax_is_supported()) {
err = fuse_dax_conn_alloc(fc, ctx->dax_mode, ctx->dax_dev);
if (err)
goto err;
@@ -1709,7 +1710,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
if (fud)
fuse_dev_free(fud);
err_free_dax:
- if (IS_ENABLED(CONFIG_FUSE_DAX))
+ if (fuse_dax_is_supported())
fuse_dax_conn_free(fc);
err:
return err;
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 5f1be1da92ce..99f8f2a18ee4 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -801,7 +801,7 @@ static int virtio_fs_setup_dax(struct virtio_device *vdev, struct virtio_fs *fs)
struct dev_pagemap *pgmap;
bool have_cache;
- if (!IS_ENABLED(CONFIG_FUSE_DAX))
+ if (fuse_dax_is_supported())
return 0;
/* Get cache region */
@@ -1366,7 +1366,7 @@ static void virtio_fs_conn_destroy(struct fuse_mount *fm)
/* Stop dax worker. Soon evict_inodes() will be called which
* will free all memory ranges belonging to all inodes.
*/
- if (IS_ENABLED(CONFIG_FUSE_DAX))
+ if (fuse_dax_is_supported())
fuse_dax_cancel_work(fc);
/* Stop forget queue. Soon destroy will be sent */
--
2.39.2
Mathieu Desnoyers wrote:
> This commit introduced in v5.13 prevents building FS_DAX on 32-bit ARM,
> even on ARMv7 which does not have virtually aliased dcaches:
>
> commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
>
> It used to work fine before: I have customers using dax over pmem on
> ARMv7, but this regression will likely prevent them from upgrading their
> kernel.
>
> The root of the issue here is the fact that DAX was never designed to
> handle virtually aliased dcache (VIVT and VIPT with aliased dcache). It
> touches the pages through their linear mapping, which is not consistent
> with the userspace mappings on virtually aliased dcaches.
>
> This patch series introduces cache_is_aliasing() with new Kconfig
> options:
>
> * ARCH_HAS_CACHE_ALIASING
> * ARCH_HAS_CACHE_ALIASING_DYNAMIC
>
> and implements it for all architectures. The "DYNAMIC" implementation
> implements cache_is_aliasing() as a runtime check, which is what is
> needed on architectures like 32-bit ARMV6 and ARMV6K.
>
> With this we can basically narrow down the list of architectures which
> are unsupported by DAX to those which are really affected.
>
> Feedback is welcome,
Hi Mathieu, this looks good overall, just some quibbling about the
ordering.
I would introduce dax_is_supported() with the current overly broad
interpretation of "!(ARM || MIPS || SPARC)" using IS_ENABLED(), then
fixup the filesystems to use the new helper, and finally go back and
convert dax_is_supported() to use cache_is_aliasing() internally.
Separately, it is not clear to me why ARCH_HAS_CACHE_ALIASING_DYNAMIC
needs to exist. As long as all paths that care are calling
cache_is_aliasing() then whether it is dynamic or not is something only
the compiler cares about. If those dynamic archs do not want to pay the
text size increase they can always do CONFIG_FS_DAX=n, right?
On Mon, Jan 29, 2024 at 04:06:31PM -0500, Mathieu Desnoyers wrote:
> Use dax_is_supported() to validate whether the architecture has
> virtually aliased caches at mount time.
>
> This is relevant for architectures which require a dynamic check
> to validate whether they have virtually aliased data caches
> (ARCH_HAS_CACHE_ALIASING_DYNAMIC=y).
Where's the rest of this patchset? I have no idea what
dax_is_supported() actually does, how it interacts with
CONFIG_FS_DAX, etc.
If you are changing anything to do with FSDAX, the cc-ing the
-entire- patchset to linux-fsdevel is absolutely necessary so the
entire patchset lands in our inboxes and not just a random patch
from the middle of a bigger change.
> Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> Signed-off-by: Mathieu Desnoyers <[email protected]>
> Cc: Chandan Babu R <[email protected]>
> Cc: Darrick J. Wong <[email protected]>
> Cc: [email protected]
> Cc: Andrew Morton <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Dan Williams <[email protected]>
> Cc: Vishal Verma <[email protected]>
> Cc: Dave Jiang <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> fs/xfs/xfs_super.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 764304595e8b..b27ecb11db66 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1376,14 +1376,22 @@ xfs_fs_parse_param(
> case Opt_nodiscard:
> parsing_mp->m_features &= ~XFS_FEAT_DISCARD;
> return 0;
> -#ifdef CONFIG_FS_DAX
> case Opt_dax:
> - xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
> - return 0;
> + if (dax_is_supported()) {
> + xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
> + return 0;
> + } else {
> + xfs_warn(parsing_mp, "dax option not supported.");
> + return -EINVAL;
> + }
> case Opt_dax_enum:
> - xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
> - return 0;
> -#endif
> + if (dax_is_supported()) {
> + xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
> + return 0;
> + } else {
> + xfs_warn(parsing_mp, "dax option not supported.");
> + return -EINVAL;
> + }
Assuming that I understand what dax_is_supported() is doing, this
change isn't right. We're just setting the DAX configuration flags
from the mount options here, we don't validate them until
we've parsed all options and eliminated conflicts and rejected
conflicting options. We validate whether the options are
appropriate for the underlying hardware configuration later in the
mount process.
dax=always suitability is check in xfs_setup_dax_always() called
later in the mount process when we have enough context and support
to open storage devices and check them for DAX support. If the
hardware does not support DAX then we simply we turn off DAX
support, we do not reject the mount as this change does.
dax=inode and dax=never are valid options on all configurations,
even those with without FSDAX support or have hardware that is not
capable of using DAX. dax=inode only affects how an inode is
instantiated in cache - if the inode has a flag that says "use DAX"
and dax is suppoortable by the hardware, then the turn on DAX for
that inode. Otherwise we just use the normal non-dax IO paths.
Again, we don't error out the filesystem if DAX is not supported,
we just don't turn it on. This check is done in
xfs_inode_should_enable_dax() and I think all you need to do is
replace the IS_ENABLED(CONFIG_FS_DAX) with a dax_is_supported()
call...
-Dave.
--
Dave Chinner
[email protected]
On 2024-01-29 16:22, Dan Williams wrote:
> Mathieu Desnoyers wrote:
>> This commit introduced in v5.13 prevents building FS_DAX on 32-bit ARM,
>> even on ARMv7 which does not have virtually aliased dcaches:
>>
>> commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
>>
>> It used to work fine before: I have customers using dax over pmem on
>> ARMv7, but this regression will likely prevent them from upgrading their
>> kernel.
>>
>> The root of the issue here is the fact that DAX was never designed to
>> handle virtually aliased dcache (VIVT and VIPT with aliased dcache). It
>> touches the pages through their linear mapping, which is not consistent
>> with the userspace mappings on virtually aliased dcaches.
>>
>> This patch series introduces cache_is_aliasing() with new Kconfig
>> options:
>>
>> * ARCH_HAS_CACHE_ALIASING
>> * ARCH_HAS_CACHE_ALIASING_DYNAMIC
>>
>> and implements it for all architectures. The "DYNAMIC" implementation
>> implements cache_is_aliasing() as a runtime check, which is what is
>> needed on architectures like 32-bit ARMV6 and ARMV6K.
>>
>> With this we can basically narrow down the list of architectures which
>> are unsupported by DAX to those which are really affected.
>>
>> Feedback is welcome,
>
> Hi Mathieu, this looks good overall, just some quibbling about the
> ordering.
Thanks for having a look !
>
> I would introduce dax_is_supported() with the current overly broad
> interpretation of "!(ARM || MIPS || SPARC)" using IS_ENABLED(), then
> fixup the filesystems to use the new helper, and finally go back and
> convert dax_is_supported() to use cache_is_aliasing() internally.
Will do.
>
> Separately, it is not clear to me why ARCH_HAS_CACHE_ALIASING_DYNAMIC
> needs to exist. As long as all paths that care are calling
> cache_is_aliasing() then whether it is dynamic or not is something only
> the compiler cares about. If those dynamic archs do not want to pay the
> .text size increase they can always do CONFIG_FS_DAX=n, right?
Good point. It will help reduce complexity and improve test coverage.
I also intend to rename "cache_is_aliasing()" to "dcache_is_aliasing()",
so if we introduce an "icache_is_aliasing()" in the future, it won't be
confusing. Having aliasing icache-dcache but not dcache-dcache seems to
be fairly common.
So basically:
If an arch selects ARCH_HAS_CACHE_ALIASING, it implements
dcache_is_aliasing() (for now), and eventually we can implement
icache_is_aliasing() as well.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
On 2024-01-29 21:38, Dave Chinner wrote:
> On Mon, Jan 29, 2024 at 04:06:31PM -0500, Mathieu Desnoyers wrote:
>> Use dax_is_supported() to validate whether the architecture has
>> virtually aliased caches at mount time.
>>
>> This is relevant for architectures which require a dynamic check
>> to validate whether they have virtually aliased data caches
>> (ARCH_HAS_CACHE_ALIASING_DYNAMIC=y).
>
> Where's the rest of this patchset? I have no idea what
> dax_is_supported() actually does, how it interacts with
> CONFIG_FS_DAX, etc.
>
> If you are changing anything to do with FSDAX, the cc-ing the
> -entire- patchset to linux-fsdevel is absolutely necessary so the
> entire patchset lands in our inboxes and not just a random patch
> from the middle of a bigger change.
Sorry, I will Cc linux-fsdevel on all patches for the next round.
Meanwhile you can find the whole series on lore:
https://lore.kernel.org/lkml/[email protected]/
[...]
>
> Assuming that I understand what dax_is_supported() is doing, this
> change isn't right. We're just setting the DAX configuration flags
> from the mount options here, we don't validate them until
> we've parsed all options and eliminated conflicts and rejected
> conflicting options. We validate whether the options are
> appropriate for the underlying hardware configuration later in the
> mount process.
>
> dax=always suitability is check in xfs_setup_dax_always() called
> later in the mount process when we have enough context and support
> to open storage devices and check them for DAX support. If the
> hardware does not support DAX then we simply we turn off DAX
> support, we do not reject the mount as this change does.
>
> dax=inode and dax=never are valid options on all configurations,
> even those with without FSDAX support or have hardware that is not
> capable of using DAX. dax=inode only affects how an inode is
> instantiated in cache - if the inode has a flag that says "use DAX"
> and dax is suppoortable by the hardware, then the turn on DAX for
> that inode. Otherwise we just use the normal non-dax IO paths.
>
> Again, we don't error out the filesystem if DAX is not supported,
> we just don't turn it on. This check is done in
> xfs_inode_should_enable_dax() and I think all you need to do is
> replace the IS_ENABLED(CONFIG_FS_DAX) with a dax_is_supported()
> call...
Thanks a lot for the detailed explanation. You are right, I will
move the dax_is_supported() check to xfs_inode_should_enable_dax().
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com