[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 06/12] mm: introduce generic lazy_mmu helpers



On Fri, Nov 07, 2025 at 03:22:54PM +0000, Ryan Roberts wrote:

Hi Ryan,

> On 07/11/2025 14:34, David Hildenbrand (Red Hat) wrote:
> >>>   #ifndef pte_batch_hint
> >>> diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
> >>> index 5d2a876035d6..c49b029d3593 100644
> >>> --- a/mm/kasan/shadow.c
> >>> +++ b/mm/kasan/shadow.c
> >>> @@ -305,7 +305,7 @@ static int kasan_populate_vmalloc_pte(pte_t *ptep,
> >>> unsigned long addr,
> >>>       pte_t pte;
> >>>       int index;
> >>>   -    arch_leave_lazy_mmu_mode();
> >>> +    lazy_mmu_mode_pause();
> >>
> >> I wonder if there really are use cases that *require* pause/resume? I think
> >> these kasan cases could be correctly implemented using a new nest level 
> >> instead?
> >> Are there cases where the effects really need to be immediate or do the 
> >> effects
> >> just need to be visible when you get to where the resume is?
> >>
> >> If the latter, that could just be turned into a nested disable (e.g. a 
> >> flush).
> >> In this case, there is only 1 PTE write so no benefit, but I wonder if 
> >> other
> >> cases may have more PTE writes that could then still be batched. It would 
> >> be
> >> nice to simplify the API by removing pause/resume if we can?
> > 
> > It has clear semantics, clearer than some nest-disable IMHO.
> > 
> > Maybe you can elaborate how you would change ("simplify") the API in that
> > regard? What would the API look like?
> 
> By simplify, I just meant can we remove lazy_mmu_mode_pause() and
> lazy_mmu_mode_resume() ?
> 
> 
> We currently have:
> 
> apply_to_page_range
>   lazy_mmu_mode_enable()
>     kasan_populate_vmalloc_pte()
>       lazy_mmu_mode_pause()
>       <code>
>       lazy_mmu_mode_resume()
>   lazy_mmu_mode_disable()
> 
> Where <code> is setting ptes. But if <code> doesn't need the effects to be
> visible until lazy_mmu_mode_resume(), then you could replace the block with:
> 
> apply_to_page_range
>   lazy_mmu_mode_enable()
>     kasan_populate_vmalloc_pte()
>       lazy_mmu_mode_enable()
>       <code>
>       lazy_mmu_mode_disable()
>   lazy_mmu_mode_disable()
> 
> However, looking at this more closely, I'm not really clear on why we need 
> *any*
> special attention to lazy mmu inside of kasan_populate_vmalloc_pte() and
> kasan_depopulate_vmalloc_pte().
> 
> I *think* that the original concern was that we were doing ptep_get(ptep) 
> inside
> of a lazy_mmu block? So we need to flush so that the getter returns the most
> recent value? But given we have never written to that particular ptep while in
> the lazy mmu block, there is surely no hazard in the first place?

There is, please see:
https://lore.kernel.org/linux-mm/cover.1755528662.git.agordeev@xxxxxxxxxxxxx/

> apply_to_existing_page_range() will only call kasan_depopulate_vmalloc_pte()
> once per pte, right? So given we read the ptep before writing it, there should
> be no hazard? If so we can remove pause/resume.

Unfortunately, we rather not, please see:
https://lore.kernel.org/linux-mm/d407a381-099b-4ec6-a20e-aeff4f3d750f@xxxxxxx/

The problem is kasan code invokes apply_to_page_range(), which enters lazy_mmu
mode unconditionally. I would claim that is rather an obstacle for the kasan
code, not a benefit. But it needs to be tackled.

Should apply_to_page_range() had an option not to enter the lazy_mmu mode
(e.g. an extra "bool skip_lazy" parameter) - the pause/resume could have
been avoided.

> Thanks,
> Ryan

Thanks!



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.