Re: [PATCH v2 09/15] riscv/mm: avoid Svnapot consistency checks in ptep_get()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded `goto retry` loop in `napotpte_ptep_get_lockless()` causes a hard lockup if it interrupts a thread on the same CPU that is concurrently modifying the NAPOT block.
--

--- Patch [9]: [PATCH v2 09/15] riscv/mm: avoid Svnapot consistency checks in ptep_get() ---
commit 6e778d5fb0d9c3254546937eeef1badca713e19a
Author: Yunhui Cui <cuiyunhui@bytedance.com>

riscv/mm: avoid Svnapot consistency checks in ptep_get()

This commit removes range consistency checks from the generic ptep_get() path,
leaving them only in napotpte_ptep_get_lockless(). It also adds comments
explaining the lockless retry behavior.

> diff --git a/arch/riscv/mm/contpte.c b/arch/riscv/mm/contpte.c
> index 70f9a0668b50c..0f66426b0ecfd 100644
> --- a/arch/riscv/mm/contpte.c
> +++ b/arch/riscv/mm/contpte.c
[ ... ]
> @@ -345,6 +346,12 @@ EXPORT_SYMBOL(napotpte_ptep_get);
>  
>  pte_t napotpte_ptep_get_lockless(pte_t *orig_ptep)
>  {
> +	/*
> +	 * ptep_get_lockless() must return a self-consistent PTE without the
> +	 * PTL. Recheck that the whole NAPOT range still describes the same
> +	 * mapping, ignoring A/D bits, and retry if a concurrent update tears
> +	 * the range while A/D state is being gathered.
> +	 */
>  	pte_t orig_pte, pte;
>  	pte_t *ptep;
>  	unsigned int i, nr;

[Severity: High]
Could the unbounded retry loop in napotpte_ptep_get_lockless() cause a hard
lockup regression if it interrupts a modifying thread on the same CPU?

If a thread modifying the NAPOT block is preempted by an NMI (such as a PMU
sampling interrupt) exactly in the middle of an update, and the NMI handler
calls ptep_get_lockless():

arch/riscv/mm/contpte.c:napotpte_ptep_get_lockless() {
    ...
retry:
	orig_pte = READ_ONCE(*orig_ptep);
	if (!pte_present_napot(orig_pte))
		return orig_pte;

	ptep = napot_align_ptep(orig_ptep);

	for (i = 0; i < nr; i++, ptep++) {
		pte = READ_ONCE(*ptep);

		if (!napotpte_is_consistent(pte, orig_pte))
			goto retry;
    ...
}

Will this partially updated block cause napotpte_is_consistent() to fail and
jump to retry?

Since the modifying thread is suspended on the same CPU, it would never be able
to resume and finish the update, resulting in an infinite spin loop.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784201104.git.cuiyunhui@bytedance.com?part=9