[PATCHv7 13/18] mm: Drop fake head checks
David Hildenbrand (Arm)
david at kernel.org
Mon Oct 26 16:37:13 PDT 2026
On 10/27/26 00:25, Kiryl Shutsemau wrote:
> On Mon, Oct 26, 2026 at 06:45:48PM +0100, David Hildenbrand (Arm) wrote:
>> On 2/27/26 20:42, Kiryl Shutsemau (Meta) wrote:
>>> From: Kiryl Shutsemau <kas at kernel.org>
>>>
>>> With fake head pages eliminated in the previous commit, remove the
>>> supporting infrastructure:
>>>
>>> - page_fixed_fake_head(): no longer needed to detect fake heads;
>>> - page_is_fake_head(): no longer needed;
>>> - page_count_writable(): no longer needed for RCU protection;
>>> - RCU read_lock in page_ref_add_unless(): no longer needed;
>>
>> I have an AI report that removing RCU read-lock was wrong.
>
> Ughh. Yes, it is a real bug.
>
> My reasoning in the commit message was wrong. Nothing stops a
> speculative get from writing to one after the remap. On x86 even a
> failing cmpxchg faults there.
>
> The fix I have in mind is to let the atomic fault.
>
> If the update faults, the struct page is a read-only HVO tail with a
> zero refcount, so the get just fails. An exception table entry on the
> cmpxchg handles it. No RCU on either side, and the fast path stays the
> same lock cmpxchg loop.
>
> Only x86, riscv and loongarch have HVO. Below is x86 only, to show the
> idea; riscv and loongarch need the same primitive in their LL/SC
> fetch_add_unless().
>
> Lightly tested on x86 by hitting a real HVO tail page.
>
> Does it look sane?
I'm sure x86 folks will have an opinion :) . Let me CC them whether something
like that would be acceptable.
>
> I can prepare a proper patchset if looks reasonable.
>
> diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
> index 75743f1dfd4e..5252b1382ed0 100644
> --- a/arch/x86/include/asm/atomic.h
> +++ b/arch/x86/include/asm/atomic.h
> @@ -108,6 +108,25 @@ static __always_inline bool arch_atomic_try_cmpxchg(atomic_t *v, int *old, int n
> }
> #define arch_atomic_try_cmpxchg arch_atomic_try_cmpxchg
>
> +/*
> + * arch_atomic_fetch_add_unless_nofault() is arch_atomic_fetch_add_unless(),
> + * except that a fault on the access jumps to @label instead of oopsing. It
> + * works in any context, including with IRQs disabled.
> + */
> +#define arch_atomic_fetch_add_unless_nofault(_v, _a, _u, label) \
> +({ \
> + atomic_t *__v = (_v); \
> + int __a = (_a), __u = (_u); \
> + int __c = arch_atomic_read(__v); \
> + \
> + do { \
> + if (unlikely(__c == __u)) \
> + break; \
> + } while (!__try_cmpxchg_nofault(&__v->counter, &__c, \
> + __c + __a, label)); \
> + __c; \
> +})
> +
> static __always_inline int arch_atomic_xchg(atomic_t *v, int new)
> {
> return arch_xchg(&v->counter, new);
> diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
> index a88b06f1c35e..685aab3edd3f 100644
> --- a/arch/x86/include/asm/cmpxchg.h
> +++ b/arch/x86/include/asm/cmpxchg.h
> @@ -232,6 +232,63 @@ extern void __add_wrong_size(void)
> #define arch_try_cmpxchg_local(ptr, pold, new) \
> __try_cmpxchg_local((ptr), (pold), (new), sizeof(*(ptr)))
>
> +/*
> + * __try_cmpxchg_nofault() is arch_try_cmpxchg() on a 4-byte kernel variable,
> + * except that a fault on the access jumps to @label instead of oopsing.
> + */
> +#ifdef CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
> +#define __try_cmpxchg_nofault(_ptr, _pold, _new, label) \
> +({ \
> + bool success; \
> + __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
> + __typeof__(*(_ptr)) __old = *_old; \
> + __typeof__(*(_ptr)) __new = (_new); \
> + \
> + if (sizeof(*(_ptr)) != 4) \
> + __cmpxchg_wrong_size(); \
> + asm_goto_output("\n" \
> + "1: " LOCK_PREFIX "cmpxchgl %[new], %[ptr]\n" \
> + _ASM_EXTABLE(1b, %l[label]) \
> + : "=@ccz" (success), \
> + [ptr] "+m" (*(_ptr)), \
> + [old] "+a" (__old) \
> + : [new] "r" (__new) \
> + : "memory" \
> + : label); \
> + if (unlikely(!success)) \
> + *_old = __old; \
> + likely(success); \
> +})
> +#else
> +#define __try_cmpxchg_nofault(_ptr, _pold, _new, label) \
> +({ \
> + int __err = 0; \
> + bool success; \
> + __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
> + __typeof__(*(_ptr)) __old = *_old; \
> + __typeof__(*(_ptr)) __new = (_new); \
> + \
> + if (sizeof(*(_ptr)) != 4) \
> + __cmpxchg_wrong_size(); \
> + asm volatile("\n" \
> + "1: " LOCK_PREFIX "cmpxchgl %[new], %[ptr]\n" \
> + "2:\n" \
> + _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG, \
> + %[errout]) \
> + : "=@ccz" (success), \
> + [errout] "+r" (__err), \
> + [ptr] "+m" (*(_ptr)), \
> + [old] "+a" (__old) \
> + : [new] "r" (__new) \
> + : "memory"); \
> + if (unlikely(__err)) \
> + goto label; \
> + if (unlikely(!success)) \
> + *_old = __old; \
> + likely(success); \
> +})
> +#endif
> +
> /*
> * xadd() adds "inc" to "*ptr" and atomically returns the previous
> * value of "*ptr".
> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> index 9f5c75d06f76..82a2fc445ce6 100644
> --- a/include/linux/page_ref.h
> +++ b/include/linux/page_ref.h
> @@ -246,9 +246,31 @@ static inline int folio_ref_dec_return(struct folio *folio)
> return page_ref_dec_return(&folio->page);
> }
>
> +#ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
> +/*
> + * HVO maps the tail struct pages of a hugetlb folio read-only. A speculative
> + * caller can read a non-zero refcount, then the page gets freed, becomes a
> + * tail page of a hugetlb folio and is remapped before the update. The
> + * atomic then faults. A read-only struct page is a hugetlb tail page with a
> + * zero refcount, so the fault means no reference can be taken.
> + */
> +static __always_inline bool __page_ref_add_unless_zero(atomic_t *v, int nr)
> +{
> + instrument_atomic_read_write(v, sizeof(*v));
> + return arch_atomic_fetch_add_unless_nofault(v, nr, 0, fault) != 0;
> +fault:
> + return false;
> +}
> +#else
> +static __always_inline bool __page_ref_add_unless_zero(atomic_t *v, int nr)
> +{
> + return atomic_add_unless(v, nr, 0);
> +}
> +#endif
> +
> static inline bool page_ref_add_unless_zero(struct page *page, int nr)
> {
> - bool ret = atomic_add_unless(&page->_refcount, nr, 0);
> + bool ret = __page_ref_add_unless_zero(&page->_refcount, nr);
>
> if (page_ref_tracepoint_active(page_ref_mod_unless))
> __page_ref_mod_unless(page, nr, ret);
--
Cheers,
David
More information about the linux-riscv
mailing list