[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f14a1ff2-6c25-4cf7-abf7-1428e62272b0@redhat.com>
Date: Tue, 25 Jun 2024 20:50:07 +0200
From: David Hildenbrand <david@...hat.com>
To: Gavin Shan <gshan@...hat.com>, linux-mm@...ck.org
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
djwong@...nel.org, willy@...radead.org, akpm@...ux-foundation.org,
hughd@...gle.com, torvalds@...ux-foundation.org, zhenyzha@...hat.com,
shan.gavin@...il.com, Ryan Roberts <ryan.roberts@....com>
Subject: Re: [PATCH 4/4] mm/shmem: Disable PMD-sized page cache if needed
On 25.06.24 11:06, Gavin Shan wrote:
> For shmem files, it's possible that PMD-sized page cache can't be
> supported by xarray. For example, 512MB page cache on ARM64 when
> the base page size is 64KB can't be supported by xarray. It leads
> to errors as the following messages indicate when this sort of xarray
> entry is split.
>
> WARNING: CPU: 34 PID: 7578 at lib/xarray.c:1025 xas_split_alloc+0xf8/0x128
> Modules linked in: binfmt_misc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 \
> nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject \
> nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 \
> ip_set rfkill nf_tables nfnetlink vfat fat virtio_balloon drm fuse xfs \
> libcrc32c crct10dif_ce ghash_ce sha2_ce sha256_arm64 sha1_ce virtio_net \
> net_failover virtio_console virtio_blk failover dimlib virtio_mmio
> CPU: 34 PID: 7578 Comm: test Kdump: loaded Tainted: G W 6.10.0-rc5-gavin+ #9
> Hardware name: QEMU KVM Virtual Machine, BIOS edk2-20240524-1.el9 05/24/2024
> pstate: 83400005 (Nzcv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> pc : xas_split_alloc+0xf8/0x128
> lr : split_huge_page_to_list_to_order+0x1c4/0x720
> sp : ffff8000882af5f0
> x29: ffff8000882af5f0 x28: ffff8000882af650 x27: ffff8000882af768
> x26: 0000000000000cc0 x25: 000000000000000d x24: ffff00010625b858
> x23: ffff8000882af650 x22: ffffffdfc0900000 x21: 0000000000000000
> x20: 0000000000000000 x19: ffffffdfc0900000 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000018000000000 x15: 52f8004000000000
> x14: 0000e00000000000 x13: 0000000000002000 x12: 0000000000000020
> x11: 52f8000000000000 x10: 52f8e1c0ffff6000 x9 : ffffbeb9619a681c
> x8 : 0000000000000003 x7 : 0000000000000000 x6 : ffff00010b02ddb0
> x5 : ffffbeb96395e378 x4 : 0000000000000000 x3 : 0000000000000cc0
> x2 : 000000000000000d x1 : 000000000000000c x0 : 0000000000000000
> Call trace:
> xas_split_alloc+0xf8/0x128
> split_huge_page_to_list_to_order+0x1c4/0x720
> truncate_inode_partial_folio+0xdc/0x160
> shmem_undo_range+0x2bc/0x6a8
> shmem_fallocate+0x134/0x430
> vfs_fallocate+0x124/0x2e8
> ksys_fallocate+0x4c/0xa0
> __arm64_sys_fallocate+0x24/0x38
> invoke_syscall.constprop.0+0x7c/0xd8
> do_el0_svc+0xb4/0xd0
> el0_svc+0x44/0x1d8
> el0t_64_sync_handler+0x134/0x150
> el0t_64_sync+0x17c/0x180
>
> Fix it by disabling PMD-sized page cache when HPAGE_PMD_ORDER is
> larger than MAX_PAGECACHE_ORDER.
>
> Suggested-by: David Hildenbrand <david@...hat.com>
> Signed-off-by: Gavin Shan <gshan@...hat.com>
> ---
> mm/shmem.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index a8b181a63402..5453875e3810 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -541,8 +541,9 @@ static bool shmem_confirm_swap(struct address_space *mapping,
>
> static int shmem_huge __read_mostly = SHMEM_HUGE_NEVER;
>
> -bool shmem_is_huge(struct inode *inode, pgoff_t index, bool shmem_huge_force,
> - struct mm_struct *mm, unsigned long vm_flags)
> +static bool __shmem_is_huge(struct inode *inode, pgoff_t index,
> + bool shmem_huge_force, struct mm_struct *mm,
> + unsigned long vm_flags)
> {
> loff_t i_size;
>
> @@ -573,6 +574,16 @@ bool shmem_is_huge(struct inode *inode, pgoff_t index, bool shmem_huge_force,
> }
> }
>
> +bool shmem_is_huge(struct inode *inode, pgoff_t index,
> + bool shmem_huge_force, struct mm_struct *mm,
> + unsigned long vm_flags)
> +{
> + if (!__shmem_is_huge(inode, index, shmem_huge_force, mm, vm_flags))
> + return false;
> +
> + return HPAGE_PMD_ORDER <= MAX_PAGECACHE_ORDER;
Why not check for that upfront?
> +}
> +
> #if defined(CONFIG_SYSFS)
> static int shmem_parse_huge(const char *str)
> {
This should make __thp_vma_allowable_orders() happy for shmem, and consequently, also khugepaged IIRC.
Acked-by: David Hildenbrand <david@...hat.com>
@Ryan,
should we do something like the following on top? The use of PUD_ORDER for ordinary pagecache is
wrong. Really only DAX is special and can support that in its own weird ways.
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 2aa986a5cd1b..ac63233fed6c 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -72,14 +72,25 @@ extern struct kobj_attribute shmem_enabled_attr;
#define THP_ORDERS_ALL_ANON ((BIT(PMD_ORDER + 1) - 1) & ~(BIT(0) | BIT(1)))
/*
- * Mask of all large folio orders supported for file THP.
+ * Mask of all large folio orders supported for FSDAX THP.
*/
-#define THP_ORDERS_ALL_FILE (BIT(PMD_ORDER) | BIT(PUD_ORDER))
+#define THP_ORDERS_ALL_DAX (BIT(PMD_ORDER) | BIT(PUD_ORDER))
+
+
+/*
+ * Mask of all large folio orders supported for ordinary pagecache (file/shmem)
+ * THP.
+ */
+#if PMD_ORDER <= MAX_PAGECACHE_ORDER
+#define THP_ORDERS_ALL_FILE 0
+#else
+#define THP_ORDERS_ALL_FILE (BIT(PMD_ORDER))
+#endif
/*
* Mask of all large folio orders supported for THP.
*/
-#define THP_ORDERS_ALL (THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE)
+#define THP_ORDERS_ALL (THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE )
#define TVA_SMAPS (1 << 0) /* Will be used for procfs */
#define TVA_IN_PF (1 << 1) /* Page fault handler */
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 89932fd0f62e..95d4a2edae39 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -88,9 +88,15 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
bool smaps = tva_flags & TVA_SMAPS;
bool in_pf = tva_flags & TVA_IN_PF;
bool enforce_sysfs = tva_flags & TVA_ENFORCE_SYSFS;
+
/* Check the intersection of requested and supported orders. */
- orders &= vma_is_anonymous(vma) ?
- THP_ORDERS_ALL_ANON : THP_ORDERS_ALL_FILE;
+ if (vma_is_anonymous(vma))
+ orders &= THP_ORDERS_ALL_ANON;
+ else if (vma_is_dax(vma))
+ orders &= THP_ORDERS_ALL_DAX;
+ else
+ orders &= THP_ORDERS_ALL_FILE;
+
if (!orders)
return 0;
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists