2024-01-30 16:54:40

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 0/8] Introduce dcache_is_aliasing() to fix DAX regression

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 dcache_is_aliasing() with the new Kconfig
option ARCH_HAS_CACHE_ALIASING and implements it for all architectures.
The implementation of dcache_is_aliasing() is either evaluated to a
constant at compile-time or a runtime check, which is what is needed on
ARM.

With this we can basically narrow down the list of architectures which
are unsupported by DAX to those which are really affected.

Note that the order of the series was completely changed based on the
feedback received on v1, cache_is_aliasing() is renamed to
dcache_is_aliasing(), ARCH_HAS_CACHE_ALIASING_DYNAMIC is gone,
dcache_is_aliasing() vs ARCH_HAS_CACHE_ALIASING relationship is
simplified, and the dax_is_supported() check was moved to its rightful
place in all filesystems.

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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Mathieu Desnoyers (8):
dax: Introduce dax_is_supported()
erofs: Use dax_is_supported()
ext2: Use dax_is_supported()
ext4: Use dax_is_supported()
fuse: Use dax_is_supported()
xfs: Use dax_is_supported()
Introduce dcache_is_aliasing() across all architectures
dax: Fix incorrect list of dcache aliasing architectures

arch/arc/Kconfig | 1 +
arch/arc/include/asm/cachetype.h | 9 +++++++++
arch/arm/Kconfig | 1 +
arch/arm/include/asm/cachetype.h | 2 ++
arch/csky/Kconfig | 1 +
arch/csky/include/asm/cachetype.h | 9 +++++++++
arch/m68k/Kconfig | 1 +
arch/m68k/include/asm/cachetype.h | 9 +++++++++
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/parisc/include/asm/cachetype.h | 9 +++++++++
arch/sh/Kconfig | 1 +
arch/sh/include/asm/cachetype.h | 9 +++++++++
arch/sparc/Kconfig | 1 +
arch/sparc/include/asm/cachetype.h | 14 ++++++++++++++
arch/xtensa/Kconfig | 1 +
arch/xtensa/include/asm/cachetype.h | 10 ++++++++++
fs/Kconfig | 1 -
fs/erofs/super.c | 5 ++++-
fs/ext2/super.c | 6 +++++-
fs/ext4/super.c | 5 ++++-
fs/fuse/dax.c | 7 +++++++
fs/xfs/xfs_iops.c | 2 +-
include/linux/cacheinfo.h | 6 ++++++
include/linux/dax.h | 9 +++++++++
mm/Kconfig | 6 ++++++
29 files changed, 142 insertions(+), 5 deletions(-)
create mode 100644 arch/arc/include/asm/cachetype.h
create mode 100644 arch/csky/include/asm/cachetype.h
create mode 100644 arch/m68k/include/asm/cachetype.h
create mode 100644 arch/mips/include/asm/cachetype.h
create mode 100644 arch/nios2/include/asm/cachetype.h
create mode 100644 arch/parisc/include/asm/cachetype.h
create mode 100644 arch/sh/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



2024-01-30 16:54:43

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 1/8] dax: Introduce dax_is_supported()

Introduce a new dax_is_supported() static inline to check whether the
architecture supports DAX.

This replaces the following fs/Kconfig:FS_DAX dependency:

depends on !(ARM || MIPS || SPARC)

This is done in preparation for its use by each filesystem supporting
the dax mount option to validate whether dax is indeed supported.

This is done in preparation for using dcache_is_aliasing() in a
following change which will properly support architectures which detect
dcache aliasing at runtime.

Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
fs/Kconfig | 1 -
include/linux/dax.h | 10 ++++++++++
2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index 42837617a55b..e5efdb3b276b 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -56,7 +56,6 @@ endif # BLOCK
config FS_DAX
bool "File system based Direct Access (DAX) support"
depends on MMU
- depends on !(ARM || MIPS || SPARC)
depends on ZONE_DEVICE || FS_DAX_LIMITED
select FS_IOMAP
select DAX
diff --git a/include/linux/dax.h b/include/linux/dax.h
index b463502b16e1..cfc8cd4a3eae 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -78,6 +78,12 @@ static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
return false;
return dax_synchronous(dax_dev);
}
+static inline bool dax_is_supported(void)
+{
+ return !IS_ENABLED(CONFIG_ARM) &&
+ !IS_ENABLED(CONFIG_MIPS) &&
+ !IS_ENABLED(CONFIG_SPARC);
+}
#else
static inline void *dax_holder(struct dax_device *dax_dev)
{
@@ -122,6 +128,10 @@ static inline size_t dax_recovery_write(struct dax_device *dax_dev,
{
return 0;
}
+static inline bool dax_is_supported(void)
+{
+ return false;
+}
#endif

void set_dax_nocache(struct dax_device *dax_dev);
--
2.39.2


2024-01-30 16:55:20

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 8/8] dax: Fix incorrect list of dcache aliasing architectures

commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
prevents DAX from building on architectures with virtually aliased
dcache with:

depends on !(ARM || MIPS || SPARC)

This check is too broad (e.g. recent ARMv7 don't have virtually aliased
dcaches), and also misses many other architectures with virtually
aliased dcache.

This is a regression introduced in the v5.13 Linux kernel where the
dax mount option is removed for 32-bit ARMv7 boards which have no dcache
aliasing, and therefore should work fine with FS_DAX.

This was turned into the following implementation of dax_is_supported()
by a preparatory change:

return !IS_ENABLED(CONFIG_ARM) &&
!IS_ENABLED(CONFIG_MIPS) &&
!IS_ENABLED(CONFIG_SPARC);

Use dcache_is_aliasing() instead to figure out whether the environment
has aliasing dcaches.

Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
include/linux/dax.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/dax.h b/include/linux/dax.h
index cfc8cd4a3eae..f59e604662e4 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -5,6 +5,7 @@
#include <linux/fs.h>
#include <linux/mm.h>
#include <linux/radix-tree.h>
+#include <linux/cacheinfo.h>

typedef unsigned long dax_entry_t;

@@ -80,9 +81,7 @@ static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
}
static inline bool dax_is_supported(void)
{
- return !IS_ENABLED(CONFIG_ARM) &&
- !IS_ENABLED(CONFIG_MIPS) &&
- !IS_ENABLED(CONFIG_SPARC);
+ return !dcache_is_aliasing();
}
#else
static inline void *dax_holder(struct dax_device *dax_dev)
--
2.39.2


2024-01-30 16:55:27

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 7/8] Introduce dcache_is_aliasing() across all architectures

Introduce a generic way to query whether the dcache is virtually aliased
on all architectures. Its purpose is to ensure that subsystems which
are incompatible with virtually aliased data caches (e.g. FS_DAX) can
reliably query this.

For dcache aliasing, there are three scenarios dependending on the
architecture. Here is a breakdown based on my understanding:

A) The dcache is always aliasing:

* arc
* csky
* m68k (note: shared memory mappings are incoherent ? SHMLBA is missing there.)
* sh
* parisc

B) The dcache aliasing is statically known or depends on querying CPU
state at runtime:

* arm (cache_is_vivt() || cache_is_vipt_aliasing())
* mips (cpu_has_dc_aliases)
* nios2 (NIOS2_DCACHE_SIZE > PAGE_SIZE)
* sparc32 (vac_cache_size > PAGE_SIZE)
* sparc64 (L1DCACHE_SIZE > PAGE_SIZE)
* xtensa (DCACHE_WAY_SIZE > PAGE_SIZE)

C) The dcache is never aliasing:

* alpha
* arm64 (aarch64)
* hexagon
* loongarch (but with incoherent write buffers, which are disabled since
commit d23b7795 ("LoongArch: Change SHMLBA from SZ_64K to PAGE_SIZE"))
* microblaze
* openrisc
* powerpc
* riscv
* s390
* um
* x86

Require architectures in A) and B) to select ARCH_HAS_CACHE_ALIASING and
implement "dcache_is_aliasing()".

Architectures in C) don't select ARCH_HAS_CACHE_ALIASING, and thus
dcache_is_aliasing() simply evaluates to "false".

Note that this leaves "icache_is_aliasing()" to be implemented as future
work. This would be useful to gate features like XIP on architectures
which have aliasing dcache-icache but not dcache-dcache.

Link: https://lore.kernel.org/lkml/[email protected]/
Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arc/Kconfig | 1 +
arch/arc/include/asm/cachetype.h | 9 +++++++++
arch/arm/Kconfig | 1 +
arch/arm/include/asm/cachetype.h | 2 ++
arch/csky/Kconfig | 1 +
arch/csky/include/asm/cachetype.h | 9 +++++++++
arch/m68k/Kconfig | 1 +
arch/m68k/include/asm/cachetype.h | 9 +++++++++
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/parisc/include/asm/cachetype.h | 9 +++++++++
arch/sh/Kconfig | 1 +
arch/sh/include/asm/cachetype.h | 9 +++++++++
arch/sparc/Kconfig | 1 +
arch/sparc/include/asm/cachetype.h | 14 ++++++++++++++
arch/xtensa/Kconfig | 1 +
arch/xtensa/include/asm/cachetype.h | 10 ++++++++++
include/linux/cacheinfo.h | 6 ++++++
mm/Kconfig | 6 ++++++
22 files changed, 112 insertions(+)
create mode 100644 arch/arc/include/asm/cachetype.h
create mode 100644 arch/csky/include/asm/cachetype.h
create mode 100644 arch/m68k/include/asm/cachetype.h
create mode 100644 arch/mips/include/asm/cachetype.h
create mode 100644 arch/nios2/include/asm/cachetype.h
create mode 100644 arch/parisc/include/asm/cachetype.h
create mode 100644 arch/sh/include/asm/cachetype.h
create mode 100644 arch/sparc/include/asm/cachetype.h
create mode 100644 arch/xtensa/include/asm/cachetype.h

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 1b0483c51cc1..969e6740bcf7 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -6,6 +6,7 @@
config ARC
def_bool y
select ARC_TIMERS
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_DEBUG_VM_PGTABLE
select ARCH_HAS_DMA_PREP_COHERENT
diff --git a/arch/arc/include/asm/cachetype.h b/arch/arc/include/asm/cachetype.h
new file mode 100644
index 000000000000..290e3cc85845
--- /dev/null
+++ b/arch/arc/include/asm/cachetype.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_ARC_CACHETYPE_H
+#define __ASM_ARC_CACHETYPE_H
+
+#include <linux/types.h>
+
+#define dcache_is_aliasing() true
+
+#endif
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f8567e95f98b..5adeee5e421f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -5,6 +5,7 @@ config ARM
select ARCH_32BIT_OFF_T
select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE if HAVE_KRETPROBES && FRAME_POINTER && !ARM_UNWIND
select ARCH_HAS_BINFMT_FLAT
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_CPU_FINALIZE_INIT if MMU
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL if MMU
diff --git a/arch/arm/include/asm/cachetype.h b/arch/arm/include/asm/cachetype.h
index e8c30430be33..18311570d4f0 100644
--- a/arch/arm/include/asm/cachetype.h
+++ b/arch/arm/include/asm/cachetype.h
@@ -20,6 +20,8 @@ extern unsigned int cacheid;
#define icache_is_vipt_aliasing() cacheid_is(CACHEID_VIPT_I_ALIASING)
#define icache_is_pipt() cacheid_is(CACHEID_PIPT)

+#define dcache_is_aliasing() (cache_is_vivt() || cache_is_vipt_aliasing())
+
/*
* __LINUX_ARM_ARCH__ is the minimum supported CPU architecture
* Mask out support which will never be present on newer CPUs.
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index cf2a6fd7dff8..439d7640deb8 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -2,6 +2,7 @@
config CSKY
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_HAS_SYNC_DMA_FOR_CPU
diff --git a/arch/csky/include/asm/cachetype.h b/arch/csky/include/asm/cachetype.h
new file mode 100644
index 000000000000..fd21c815a8e4
--- /dev/null
+++ b/arch/csky/include/asm/cachetype.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_CSKY_CACHETYPE_H
+#define __ASM_CSKY_CACHETYPE_H
+
+#include <linux/types.h>
+
+#define dcache_is_aliasing() true
+
+#endif
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 4b3e93cac723..216338704f0a 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -3,6 +3,7 @@ config M68K
bool
default y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_BINFMT_FLAT
select ARCH_HAS_CPU_FINALIZE_INIT if MMU
select ARCH_HAS_CURRENT_STACK_POINTER
diff --git a/arch/m68k/include/asm/cachetype.h b/arch/m68k/include/asm/cachetype.h
new file mode 100644
index 000000000000..24298a45b215
--- /dev/null
+++ b/arch/m68k/include/asm/cachetype.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_M68K_CACHETYPE_H
+#define __ASM_M68K_CACHETYPE_H
+
+#include <linux/types.h>
+
+#define dcache_is_aliasing() true
+
+#endif
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 797ae590ebdb..cc21e88b2dc4 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -4,6 +4,7 @@ config MIPS
default y
select ARCH_32BIT_OFF_T if !64BIT
select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_CPU_FINALIZE_INIT
select ARCH_HAS_CURRENT_STACK_POINTER if !CC_IS_CLANG || CLANG_VERSION >= 140000
select ARCH_HAS_DEBUG_VIRTUAL if !64BIT
diff --git a/arch/mips/include/asm/cachetype.h b/arch/mips/include/asm/cachetype.h
new file mode 100644
index 000000000000..b967c4219c31
--- /dev/null
+++ b/arch/mips/include/asm/cachetype.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_MIPS_CACHETYPE_H
+#define __ASM_MIPS_CACHETYPE_H
+
+#include <asm/cpu-features.h>
+
+#define dcache_is_aliasing() cpu_has_dc_aliases
+
+#endif
diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
index d54464021a61..af3a0631f4f1 100644
--- a/arch/nios2/Kconfig
+++ b/arch/nios2/Kconfig
@@ -2,6 +2,7 @@
config NIOS2
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_SYNC_DMA_FOR_CPU
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
diff --git a/arch/nios2/include/asm/cachetype.h b/arch/nios2/include/asm/cachetype.h
new file mode 100644
index 000000000000..3027ba4e75ab
--- /dev/null
+++ b/arch/nios2/include/asm/cachetype.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_NIOS2_CACHETYPE_H
+#define __ASM_NIOS2_CACHETYPE_H
+
+#include <asm/page.h>
+#include <asm/cache.h>
+
+#define dcache_is_aliasing() (NIOS2_DCACHE_SIZE > PAGE_SIZE)
+
+#endif
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index d14ccc948a29..e8c217744d83 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -8,6 +8,7 @@ config PARISC
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_SYSCALL_TRACEPOINTS
select ARCH_WANT_FRAME_POINTERS
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_DMA_ALLOC if PA11
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_STRICT_KERNEL_RWX
diff --git a/arch/parisc/include/asm/cachetype.h b/arch/parisc/include/asm/cachetype.h
new file mode 100644
index 000000000000..a7f49ff26e3f
--- /dev/null
+++ b/arch/parisc/include/asm/cachetype.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_PARISC_CACHETYPE_H
+#define __ASM_PARISC_CACHETYPE_H
+
+#include <linux/types.h>
+
+#define dcache_is_aliasing() true
+
+#endif
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 7500521b2b98..6465ef80c055 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -2,6 +2,7 @@
config SUPERH
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM && MMU
select ARCH_ENABLE_MEMORY_HOTREMOVE if SPARSEMEM && MMU
select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
diff --git a/arch/sh/include/asm/cachetype.h b/arch/sh/include/asm/cachetype.h
new file mode 100644
index 000000000000..55619e60c55f
--- /dev/null
+++ b/arch/sh/include/asm/cachetype.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_SH_CACHETYPE_H
+#define __ASM_SH_CACHETYPE_H
+
+#include <linux/types.h>
+
+#define dcache_is_aliasing() true
+
+#endif
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49849790e66d..8794e0d0920f 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -13,6 +13,7 @@ config 64BIT
config SPARC
bool
default y
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_MIGHT_HAVE_PC_PARPORT if SPARC64 && PCI
select ARCH_MIGHT_HAVE_PC_SERIO
select DMA_OPS
diff --git a/arch/sparc/include/asm/cachetype.h b/arch/sparc/include/asm/cachetype.h
new file mode 100644
index 000000000000..3ebc0e08a298
--- /dev/null
+++ b/arch/sparc/include/asm/cachetype.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_SPARC_CACHETYPE_H
+#define __ASM_SPARC_CACHETYPE_H
+
+#include <asm/page.h>
+
+#ifdef CONFIG_SPARC32
+extern int vac_cache_size;
+#define dcache_is_aliasing() (vac_cache_size > PAGE_SIZE)
+#else
+#define dcache_is_aliasing() (L1DCACHE_SIZE > PAGE_SIZE)
+#endif
+
+#endif
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 7d792077e5fd..54dbe9f42ec0 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -2,6 +2,7 @@
config XTENSA
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_CACHE_ALIASING
select ARCH_HAS_BINFMT_FLAT if !MMU
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VM_PGTABLE
diff --git a/arch/xtensa/include/asm/cachetype.h b/arch/xtensa/include/asm/cachetype.h
new file mode 100644
index 000000000000..1de9337e9dbd
--- /dev/null
+++ b/arch/xtensa/include/asm/cachetype.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_XTENSA_CACHETYPE_H
+#define __ASM_XTENSA_CACHETYPE_H
+
+#include <asm/cache.h>
+#include <asm/page.h>
+
+#define dcache_is_aliasing() (DCACHE_WAY_SIZE > PAGE_SIZE)
+
+#endif
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index d504eb4b49ab..a307916fac40 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -138,4 +138,10 @@ static inline int get_cpu_cacheinfo_id(int cpu, int level)
#define use_arch_cache_info() (false)
#endif

+#ifndef CONFIG_ARCH_HAS_CACHE_ALIASING
+#define dcache_is_aliasing() false
+#else
+#include <asm/cachetype.h>
+#endif
+
#endif /* _LINUX_CACHEINFO_H */
diff --git a/mm/Kconfig b/mm/Kconfig
index 57cd378c73d6..3fa87e45883d 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1016,6 +1016,12 @@ config IDLE_PAGE_TRACKING
See Documentation/admin-guide/mm/idle_page_tracking.rst for
more details.

+# Architectures which implement dcache_is_aliasing() to query whether
+# the data caches are aliased (VIVT or VIPT with dcache aliasing) need
+# to select this.
+config ARCH_HAS_CACHE_ALIASING
+ bool
+
config ARCH_HAS_CACHE_LINE_SIZE
bool

--
2.39.2


2024-01-30 16:56:20

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 6/8] xfs: Use dax_is_supported()

Use dax_is_supported() to validate whether the architecture has
virtually aliased data caches at mount time. Silently disable
DAX if dax=always is requested as a mount option on an architecture
which does not support DAX.

This is relevant for architectures which require a dynamic check
to validate whether they have virtually aliased data caches.

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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
fs/xfs/xfs_iops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index a0d77f5f512e..360f640159b0 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1208,7 +1208,7 @@ static bool
xfs_inode_should_enable_dax(
struct xfs_inode *ip)
{
- if (!IS_ENABLED(CONFIG_FS_DAX))
+ if (!dax_is_supported())
return false;
if (xfs_has_dax_never(ip->i_mount))
return false;
--
2.39.2


2024-01-30 16:56:20

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 5/8] fuse: Use dax_is_supported()

Use dax_is_supported() to validate whether the architecture has
virtually aliased data caches at mount time. Silently disable
DAX if dax=always is requested as a mount option on an architecture
which does not support DAX.

This is relevant for architectures which require a dynamic check
to validate whether they have virtually aliased data caches.

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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
fs/fuse/dax.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 12ef91d170bb..36e1c1abbf8e 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1336,6 +1336,13 @@ static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
if (dax_mode == FUSE_DAX_NEVER)
return false;

+ /*
+ * Silently fallback to 'never' mode if the architecture does
+ * not support DAX.
+ */
+ if (!dax_is_supported())
+ return false;
+
/*
* fc->dax may be NULL in 'inode' mode when filesystem device doesn't
* support DAX, in which case it will silently fallback to 'never' mode.
--
2.39.2


2024-01-30 17:16:04

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC PATCH v2 2/8] erofs: Use dax_is_supported()

Use dax_is_supported() to validate whether the architecture has
virtually aliased data caches at mount time. Print an error and disable
DAX if dax=always is requested as a mount option on an architecture
which does not support DAX.

This is relevant for architectures which require a dynamic check
to validate whether they have virtually aliased data caches.

Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
Signed-off-by: Mathieu Desnoyers <[email protected]>
Cc: Gao Xiang <[email protected]>
Cc: Chao Yu <[email protected]>
Cc: Yue Hu <[email protected]>
Cc: Jeffle Xu <[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: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
fs/erofs/super.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3789d6224513..82e569bd5889 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -644,7 +644,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
}

if (test_opt(&sbi->opt, DAX_ALWAYS)) {
- if (!sbi->dax_dev) {
+ if (!dax_is_supported()) {
+ errorfc(fc, "DAX unsupported by architecture. Turning off DAX.");
+ clear_opt(&sbi->opt, DAX_ALWAYS);
+ } else if (!sbi->dax_dev) {
errorfc(fc, "DAX unsupported by block device. Turning off DAX.");
clear_opt(&sbi->opt, DAX_ALWAYS);
} else if (sbi->blkszbits != PAGE_SHIFT) {
--
2.39.2


2024-01-31 02:40:36

by Dave Chinner

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/8] dax: Introduce dax_is_supported()

On Tue, Jan 30, 2024 at 11:52:48AM -0500, Mathieu Desnoyers wrote:
> Introduce a new dax_is_supported() static inline to check whether the
> architecture supports DAX.
>
> This replaces the following fs/Kconfig:FS_DAX dependency:
>
> depends on !(ARM || MIPS || SPARC)
>
> This is done in preparation for its use by each filesystem supporting
> the dax mount option to validate whether dax is indeed supported.
>
> This is done in preparation for using dcache_is_aliasing() in a
> following change which will properly support architectures which detect
> dcache aliasing at runtime.
>
> Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> fs/Kconfig | 1 -
> include/linux/dax.h | 10 ++++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 42837617a55b..e5efdb3b276b 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -56,7 +56,6 @@ endif # BLOCK
> config FS_DAX
> bool "File system based Direct Access (DAX) support"
> depends on MMU
> - depends on !(ARM || MIPS || SPARC)
> depends on ZONE_DEVICE || FS_DAX_LIMITED
> select FS_IOMAP
> select DAX
> diff --git a/include/linux/dax.h b/include/linux/dax.h
> index b463502b16e1..cfc8cd4a3eae 100644
> --- a/include/linux/dax.h
> +++ b/include/linux/dax.h
> @@ -78,6 +78,12 @@ static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
> return false;
> return dax_synchronous(dax_dev);
> }
> +static inline bool dax_is_supported(void)
> +{
> + return !IS_ENABLED(CONFIG_ARM) &&
> + !IS_ENABLED(CONFIG_MIPS) &&
> + !IS_ENABLED(CONFIG_SPARC);
> +}

Uh, ok. Now I see what dax_is_supported() does.

I think this should be folded into fs_dax_get_by_bdev(), which
currently returns NULL if CONFIG_FS_DAX=n and so should be cahnged
to return NULL if any of these platform configs is enabled.

Then I don't think you need to change a single line of filesystem
code - they'll all just do what they do now if the block device
doesn't support DAX....

-Dave.
--
Dave Chinner
[email protected]

2024-01-31 02:48:56

by Dave Chinner

[permalink] [raw]
Subject: Re: [RFC PATCH v2 7/8] Introduce dcache_is_aliasing() across all architectures

On Tue, Jan 30, 2024 at 11:52:54AM -0500, Mathieu Desnoyers wrote:
> Introduce a generic way to query whether the dcache is virtually aliased
> on all architectures. Its purpose is to ensure that subsystems which
> are incompatible with virtually aliased data caches (e.g. FS_DAX) can
> reliably query this.
>
> For dcache aliasing, there are three scenarios dependending on the
> architecture. Here is a breakdown based on my understanding:
>
> A) The dcache is always aliasing:
>
> * arc
> * csky
> * m68k (note: shared memory mappings are incoherent ? SHMLBA is missing there.)
> * sh
> * parisc

/me wonders why the dentry cache aliasing has problems on these
systems.

Oh, dcache != fs/dcache.c (the VFS dentry cache).

Can you please rename this function appropriately so us dumb
filesystem people don't confuse cpu data cache configurations with
the VFS dentry cache aliasing when we read this code? Something like
cpu_dcache_is_aliased(), perhaps?

-Dave.
--
Dave Chinner
[email protected]

2024-01-31 02:56:15

by Dave Chinner

[permalink] [raw]
Subject: Re: [RFC PATCH v2 8/8] dax: Fix incorrect list of dcache aliasing architectures

On Tue, Jan 30, 2024 at 11:52:55AM -0500, Mathieu Desnoyers wrote:
> commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> prevents DAX from building on architectures with virtually aliased
> dcache with:
>
> depends on !(ARM || MIPS || SPARC)
>
> This check is too broad (e.g. recent ARMv7 don't have virtually aliased
> dcaches), and also misses many other architectures with virtually
> aliased dcache.
>
> This is a regression introduced in the v5.13 Linux kernel where the
> dax mount option is removed for 32-bit ARMv7 boards which have no dcache
> aliasing, and therefore should work fine with FS_DAX.
>
> This was turned into the following implementation of dax_is_supported()
> by a preparatory change:
>
> return !IS_ENABLED(CONFIG_ARM) &&
> !IS_ENABLED(CONFIG_MIPS) &&
> !IS_ENABLED(CONFIG_SPARC);
>
> Use dcache_is_aliasing() instead to figure out whether the environment
> has aliasing dcaches.
>
> Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> include/linux/dax.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/dax.h b/include/linux/dax.h
> index cfc8cd4a3eae..f59e604662e4 100644
> --- a/include/linux/dax.h
> +++ b/include/linux/dax.h
> @@ -5,6 +5,7 @@
> #include <linux/fs.h>
> #include <linux/mm.h>
> #include <linux/radix-tree.h>
> +#include <linux/cacheinfo.h>
>
> typedef unsigned long dax_entry_t;
>
> @@ -80,9 +81,7 @@ static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
> }
> static inline bool dax_is_supported(void)
> {
> - return !IS_ENABLED(CONFIG_ARM) &&
> - !IS_ENABLED(CONFIG_MIPS) &&
> - !IS_ENABLED(CONFIG_SPARC);
> + return !dcache_is_aliasing();

Yeah, if this is just a one liner should go into
fs_dax_get_by_bdev(), similar to the blk_queue_dax() check at the
start of the function.

I also noticed that device mapper uses fs_dax_get_by_bdev() to
determine if it can support DAX, but this patch set does not address
that case. Hence it really seems to me like fs_dax_get_by_bdev() is
the right place to put this check.

-Dave.
--
Dave Chinner
[email protected]

2024-01-31 03:13:30

by Dan Williams

[permalink] [raw]
Subject: Re: [RFC PATCH v2 8/8] dax: Fix incorrect list of dcache aliasing architectures

Dave Chinner wrote:
> On Tue, Jan 30, 2024 at 11:52:55AM -0500, Mathieu Desnoyers wrote:
> > commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> > prevents DAX from building on architectures with virtually aliased
> > dcache with:
> >
> > depends on !(ARM || MIPS || SPARC)
> >
> > This check is too broad (e.g. recent ARMv7 don't have virtually aliased
> > dcaches), and also misses many other architectures with virtually
> > aliased dcache.
> >
> > This is a regression introduced in the v5.13 Linux kernel where the
> > dax mount option is removed for 32-bit ARMv7 boards which have no dcache
> > aliasing, and therefore should work fine with FS_DAX.
> >
> > This was turned into the following implementation of dax_is_supported()
> > by a preparatory change:
> >
> > return !IS_ENABLED(CONFIG_ARM) &&
> > !IS_ENABLED(CONFIG_MIPS) &&
> > !IS_ENABLED(CONFIG_SPARC);
> >
> > Use dcache_is_aliasing() instead to figure out whether the environment
> > has aliasing dcaches.
> >
> > Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> > Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
> > Cc: Russell King <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > include/linux/dax.h | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/dax.h b/include/linux/dax.h
> > index cfc8cd4a3eae..f59e604662e4 100644
> > --- a/include/linux/dax.h
> > +++ b/include/linux/dax.h
> > @@ -5,6 +5,7 @@
> > #include <linux/fs.h>
> > #include <linux/mm.h>
> > #include <linux/radix-tree.h>
> > +#include <linux/cacheinfo.h>
> >
> > typedef unsigned long dax_entry_t;
> >
> > @@ -80,9 +81,7 @@ static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
> > }
> > static inline bool dax_is_supported(void)
> > {
> > - return !IS_ENABLED(CONFIG_ARM) &&
> > - !IS_ENABLED(CONFIG_MIPS) &&
> > - !IS_ENABLED(CONFIG_SPARC);
> > + return !dcache_is_aliasing();
>
> Yeah, if this is just a one liner should go into
> fs_dax_get_by_bdev(), similar to the blk_queue_dax() check at the
> start of the function.
>
> I also noticed that device mapper uses fs_dax_get_by_bdev() to
> determine if it can support DAX, but this patch set does not address
> that case. Hence it really seems to me like fs_dax_get_by_bdev() is
> the right place to put this check.

Oh, good catch. Yes, I agree this can definitely be pushed down, but
then I think it should be pushed down all the way to make alloc_dax()
fail. That will need some additional fixups like:

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8dcabf84d866..a35e60e62440 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2126,12 +2126,12 @@ static struct mapped_device *alloc_dev(int minor)
md->dax_dev = alloc_dax(md, &dm_dax_ops);
if (IS_ERR(md->dax_dev)) {
md->dax_dev = NULL;
- goto bad;
+ } else {
+ set_dax_nocache(md->dax_dev);
+ set_dax_nomc(md->dax_dev);
+ if (dax_add_host(md->dax_dev, md->disk))
+ goto bad;
}
- set_dax_nocache(md->dax_dev);
- set_dax_nomc(md->dax_dev);
- if (dax_add_host(md->dax_dev, md->disk))
- goto bad;
}

format_dev_t(md->name, MKDEV(_major, minor));

..to make it not fatal to fail to register the dax_dev.

2024-01-31 15:22:27

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [RFC PATCH v2 8/8] dax: Fix incorrect list of dcache aliasing architectures

On 2024-01-30 22:13, Dan Williams wrote:
> Dave Chinner wrote:
>> On Tue, Jan 30, 2024 at 11:52:55AM -0500, Mathieu Desnoyers wrote:
>>> commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
>>> prevents DAX from building on architectures with virtually aliased
>>> dcache with:
>>>
>>> depends on !(ARM || MIPS || SPARC)
>>>
>>> This check is too broad (e.g. recent ARMv7 don't have virtually aliased
>>> dcaches), and also misses many other architectures with virtually
>>> aliased dcache.
>>>
>>> This is a regression introduced in the v5.13 Linux kernel where the
>>> dax mount option is removed for 32-bit ARMv7 boards which have no dcache
>>> aliasing, and therefore should work fine with FS_DAX.
>>>
>>> This was turned into the following implementation of dax_is_supported()
>>> by a preparatory change:
>>>
>>> return !IS_ENABLED(CONFIG_ARM) &&
>>> !IS_ENABLED(CONFIG_MIPS) &&
>>> !IS_ENABLED(CONFIG_SPARC);
>>>
>>> Use dcache_is_aliasing() instead to figure out whether the environment
>>> has aliasing dcaches.
>>>
>>> Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
>>> Signed-off-by: Mathieu Desnoyers <[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: Arnd Bergmann <[email protected]>
>>> Cc: Russell King <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> ---
>>> include/linux/dax.h | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/linux/dax.h b/include/linux/dax.h
>>> index cfc8cd4a3eae..f59e604662e4 100644
>>> --- a/include/linux/dax.h
>>> +++ b/include/linux/dax.h
>>> @@ -5,6 +5,7 @@
>>> #include <linux/fs.h>
>>> #include <linux/mm.h>
>>> #include <linux/radix-tree.h>
>>> +#include <linux/cacheinfo.h>
>>>
>>> typedef unsigned long dax_entry_t;
>>>
>>> @@ -80,9 +81,7 @@ static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
>>> }
>>> static inline bool dax_is_supported(void)
>>> {
>>> - return !IS_ENABLED(CONFIG_ARM) &&
>>> - !IS_ENABLED(CONFIG_MIPS) &&
>>> - !IS_ENABLED(CONFIG_SPARC);
>>> + return !dcache_is_aliasing();
>>
>> Yeah, if this is just a one liner should go into
>> fs_dax_get_by_bdev(), similar to the blk_queue_dax() check at the
>> start of the function.
>>
>> I also noticed that device mapper uses fs_dax_get_by_bdev() to
>> determine if it can support DAX, but this patch set does not address
>> that case. Hence it really seems to me like fs_dax_get_by_bdev() is
>> the right place to put this check.
>
> Oh, good catch. Yes, I agree this can definitely be pushed down, but
> then I think it should be pushed down all the way to make alloc_dax()
> fail. That will need some additional fixups like:
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 8dcabf84d866..a35e60e62440 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2126,12 +2126,12 @@ static struct mapped_device *alloc_dev(int minor)
> md->dax_dev = alloc_dax(md, &dm_dax_ops);
> if (IS_ERR(md->dax_dev)) {
> md->dax_dev = NULL;
> - goto bad;
> + } else {
> + set_dax_nocache(md->dax_dev);
> + set_dax_nomc(md->dax_dev);
> + if (dax_add_host(md->dax_dev, md->disk))
> + goto bad;
> }
> - set_dax_nocache(md->dax_dev);
> - set_dax_nomc(md->dax_dev);
> - if (dax_add_host(md->dax_dev, md->disk))
> - goto bad;
> }
>
> format_dev_t(md->name, MKDEV(_major, minor));
>
> ...to make it not fatal to fail to register the dax_dev.

I've had a quick look at other users of alloc_dax() and
alloc_dax_region(), and so far I figure that all of those
really want to bail out on alloc_dax failure. Is dm.c the
only special-case we need to fix to make it non-fatal ?

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com


2024-01-31 15:46:12

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [RFC PATCH v2 7/8] Introduce dcache_is_aliasing() across all architectures

On 2024-01-30 21:48, Dave Chinner wrote:
> On Tue, Jan 30, 2024 at 11:52:54AM -0500, Mathieu Desnoyers wrote:
>> Introduce a generic way to query whether the dcache is virtually aliased
>> on all architectures. Its purpose is to ensure that subsystems which
>> are incompatible with virtually aliased data caches (e.g. FS_DAX) can
>> reliably query this.
>>
>> For dcache aliasing, there are three scenarios dependending on the
>> architecture. Here is a breakdown based on my understanding:
>>
>> A) The dcache is always aliasing:
>>
>> * arc
>> * csky
>> * m68k (note: shared memory mappings are incoherent ? SHMLBA is missing there.)
>> * sh
>> * parisc
>
> /me wonders why the dentry cache aliasing has problems on these
> systems.
>
> Oh, dcache != fs/dcache.c (the VFS dentry cache).
>
> Can you please rename this function appropriately so us dumb
> filesystem people don't confuse cpu data cache configurations with
> the VFS dentry cache aliasing when we read this code? Something like
> cpu_dcache_is_aliased(), perhaps?

Good point, will do. I'm planning go rename as follows for v3 to
eliminate confusion with dentry cache (and with "page cache" in
general):

ARCH_HAS_CACHE_ALIASING -> ARCH_HAS_CPU_CACHE_ALIASING
dcache_is_aliasing() -> cpu_dcache_is_aliasing()

I noticed that you suggested "aliased" rather than "aliasing",
but I followed what arm64 did for icache_is_aliasing(). Do you
have a strong preference one way or another ?

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com


2024-01-31 20:43:07

by Dave Chinner

[permalink] [raw]
Subject: Re: [RFC PATCH v2 7/8] Introduce dcache_is_aliasing() across all architectures

On Wed, Jan 31, 2024 at 09:58:21AM -0500, Mathieu Desnoyers wrote:
> On 2024-01-30 21:48, Dave Chinner wrote:
> > On Tue, Jan 30, 2024 at 11:52:54AM -0500, Mathieu Desnoyers wrote:
> > > Introduce a generic way to query whether the dcache is virtually aliased
> > > on all architectures. Its purpose is to ensure that subsystems which
> > > are incompatible with virtually aliased data caches (e.g. FS_DAX) can
> > > reliably query this.
> > >
> > > For dcache aliasing, there are three scenarios dependending on the
> > > architecture. Here is a breakdown based on my understanding:
> > >
> > > A) The dcache is always aliasing:
> > >
> > > * arc
> > > * csky
> > > * m68k (note: shared memory mappings are incoherent ? SHMLBA is missing there.)
> > > * sh
> > > * parisc
> >
> > /me wonders why the dentry cache aliasing has problems on these
> > systems.
> >
> > Oh, dcache != fs/dcache.c (the VFS dentry cache).
> >
> > Can you please rename this function appropriately so us dumb
> > filesystem people don't confuse cpu data cache configurations with
> > the VFS dentry cache aliasing when we read this code? Something like
> > cpu_dcache_is_aliased(), perhaps?
>
> Good point, will do. I'm planning go rename as follows for v3 to
> eliminate confusion with dentry cache (and with "page cache" in
> general):
>
> ARCH_HAS_CACHE_ALIASING -> ARCH_HAS_CPU_CACHE_ALIASING
> dcache_is_aliasing() -> cpu_dcache_is_aliasing()
>
> I noticed that you suggested "aliased" rather than "aliasing",
> but I followed what arm64 did for icache_is_aliasing(). Do you
> have a strong preference one way or another ?

Not really.

-Dave.
--
Dave Chinner
[email protected]