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 <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: linux-cxl@vger.kernel.org
Cc: nvdimm@lists.linux.dev

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

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 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