linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Roberts <ryan.roberts@arm.com>
To: David Hildenbrand <david@redhat.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Uladzislau Rezki <urezki@gmail.com>,
	Christoph Hellwig <hch@infradead.org>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Alexandre Ghiti <alexghiti@rivosinc.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	syzbot+5c0d9392e042f41d45c5@syzkaller.appspotmail.com
Subject: Re: [PATCH] arm64/mm: Disable barrier batching in interrupt contexts
Date: Mon, 12 May 2025 13:33:28 +0100	[thread overview]
Message-ID: <20e6ac75-6c4b-499b-ba26-cc1a2110509e@arm.com> (raw)
In-Reply-To: <836f2574-cb60-44c5-865c-7f13a90779ec@redhat.com>

On 12/05/2025 13:05, David Hildenbrand wrote:
> 
>>>>    static inline void arch_leave_lazy_mmu_mode(void)
>>>>    {
>>>> +    if (in_interrupt())
>>>> +        return;
>>>> +
>>>>        arch_flush_lazy_mmu_mode();
>>>>        clear_thread_flag(TIF_LAZY_MMU);
>>>>    }
>>>
>>> I guess in all cases we could optimize out the in_interrupt() check on !debug
>>> configs.
>>
>> I think that assumes we can easily and accurately identify all configs that
>> cause this? We've identified 2 but I'm not confident that it's a full list.
> 
> Agreed. I was wondering if we could convert the ones to use different pte
> helpers, whereby these helpers would not be available without CONFIG_WHATEVER.
> Then, make these features select CONFIG_WHATEVER.

Trouble is, in KFENCE's case, there is quite a bit of code between the general
API it calls and the pte manipulations:

 arch_enter_lazy_mmu_mode arch/arm64/include/asm/pgtable.h:82 [inline] (P)
 apply_to_pte_range mm/memory.c:2936 [inline] (P)
 apply_to_pmd_range mm/memory.c:2985 [inline] (P)
 apply_to_pud_range mm/memory.c:3021 [inline] (P)
 apply_to_p4d_range mm/memory.c:3057 [inline] (P)
 __apply_to_page_range+0xdb4/0x13e4 mm/memory.c:3093 (P)
 apply_to_page_range+0x4c/0x64 mm/memory.c:3112
 __change_memory_common+0xac/0x3f8 arch/arm64/mm/pageattr.c:64
 set_memory_valid+0x68/0x7c arch/arm64/mm/pageattr.c:-1
 kfence_protect_page arch/arm64/include/asm/kfence.h:17 [inline]
 kfence_protect mm/kfence/core.c:247 [inline]
 kfence_guarded_free+0x278/0x5a8 mm/kfence/core.c:565
 __kfence_free+0x104/0x198 mm/kfence/core.c:1187
 kfence_free include/linux/kfence.h:187 [inline]
 slab_free_hook mm/slub.c:2318 [inline]
 slab_free mm/slub.c:4642 [inline]
 kfree+0x268/0x474 mm/slub.c:4841
 slab_free_after_rcu_debug+0x78/0x2f4 mm/slub.c:4679
 rcu_do_batch kernel/rcu/tree.c:2568 [inline]
 rcu_core+0x848/0x17a4 kernel/rcu/tree.c:2824
 rcu_core_si+0x10/0x1c kernel/rcu/tree.c:2841
 handle_softirqs+0x328/0xc88 kernel/softirq.c:579
 __do_softirq+0x14/0x20 kernel/softirq.c:613

> 
> VM_WARN_ON_* would be used to catch any violations / wrong use of pte helpers.
> 
>> Also, KFENCE isn't really a debug config (despite me calling it that in the
>> commit log) - it's supposed to be something that can be enabled in production
>> builds.
> 
> Agreed. Even Fedora has it.
> 
>>
>>>
>>> Hm, maybe there is an elegant way to catch all of these "problematic" users?
>>
>> I'm all ears if you have any suggestions? :)
>>
>>
>> It actaully looks like x86/XEN tries to solves this problem in a similar way:
> 
> Heh, yes. Good old xen ...
> 
>>
>> enum xen_lazy_mode xen_get_lazy_mode(void)
>> {
>>     if (in_interrupt())
>>         return XEN_LAZY_NONE;
>>
>>     return this_cpu_read(xen_lazy_mode);
>> }
>>
>> Although I'm not convinced it's fully robust. It also has:
>>
>> static inline void enter_lazy(enum xen_lazy_mode mode)
>> {
>>     BUG_ON(this_cpu_read(xen_lazy_mode) != XEN_LAZY_NONE);
>>
>>     this_cpu_write(xen_lazy_mode, mode);
>> }
>>
>> which is called as part of its arch_enter_lazy_mmu_mode() implementation. If a
>> task was already in lazy mmu mode when an interrupt comes in and causes the
>> nested arch_enter_lazy_mmu_mode() that we saw in this bug report, surely that
>> BUG_ON() should trigger?
> 
> Hm, good point. But that code is old, so probably something seems to be
> preventing that?
> 
> 
> In any case, just a thought on the in_interrupt() check, I think this commit is
> good enough as is.

OK thanks. I'll leave it as-is then in the absence of other feedback :)



  reply	other threads:[~2025-05-12 12:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-12 10:22 Ryan Roberts
2025-05-12 11:00 ` Catalin Marinas
2025-05-12 11:03   ` Ryan Roberts
2025-05-12 11:07 ` David Hildenbrand
2025-05-12 12:00   ` Ryan Roberts
2025-05-12 12:05     ` David Hildenbrand
2025-05-12 12:33       ` Ryan Roberts [this message]
2025-05-12 13:14 ` Catalin Marinas
2025-05-12 13:53   ` Ryan Roberts
2025-05-12 14:14     ` Ryan Roberts
2025-05-13 20:46 ` Will Deacon
2025-05-14  9:29   ` Ryan Roberts
2025-05-14 15:13     ` Will Deacon
2025-05-14 15:14 ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20e6ac75-6c4b-499b-ba26-cc1a2110509e@arm.com \
    --to=ryan.roberts@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexghiti@rivosinc.com \
    --cc=anshuman.khandual@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=david@redhat.com \
    --cc=hch@infradead.org \
    --cc=kevin.brodsky@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=syzbot+5c0d9392e042f41d45c5@syzkaller.appspotmail.com \
    --cc=urezki@gmail.com \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox