Hi Zi,
On Wed, Dec 4, 2024 at 5:58 PM Zi Yan <ziy@xxxxxxxxxx> wrote:
> On 4 Dec 2024, at 11:29, Matthew Wilcox wrote:
> > On Wed, Dec 04, 2024 at 11:16:51AM -0500, Zi Yan wrote:
> >>> So maybe the clearing done as part of page allocator isn't enough here.
> >>>
> >> Basically, mips needs to flush data cache if kmap address is aliased to
> >
> > People use "aliased" in contronym ways. Do you mean "has a
> > non-congruent alias" or "has a congruent alias"?
>
> I mean if kmap address goes into a different cache line than userspace
> address, a cache flush is needed to make sure data is visible to
> userspace.
>
> >
> >> userspace address. This means when mips has THP on, the patch below
> >> is not enough to fix the issue.
> >>
> >> In post_alloc_hook(), it does not make sense to pass userspace address
> >> in to determine whether to flush dcache or not.
> >>
> >> One way to fix it is to add something like arch_userpage_post_alloc()
> >> to flush dcache if kmap address is aliased to userspace address.
> >> But my questions are that
> >> 1) if kmap address will always be the same for two separate kmap_local() calls,
> >
> > No. It just takes the next address in the stack.
>
> So this fix will not work, since it is possible that first kmap and second
> kmap have different pages_do_alias() return values.
>
> Another way would be to make a special case for mips, like below.
> But that looks ugly, let me think about it more.
>
> diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> index bc3e3484c1bf..ef3c6f0b9159 100644
> --- a/arch/mips/include/asm/page.h
> +++ b/arch/mips/include/asm/page.h
> @@ -95,6 +95,19 @@ struct vm_area_struct;
> extern void copy_user_highpage(struct page *to, struct page *from,
> unsigned long vaddr, struct vm_area_struct *vma);
>
> +struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
> + unsigned long vaddr)
> + {
> + struct folio *folio;
> +
> + folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vaddr);
> + if (folio)
> + clear_user_highpage(&folio->page, vaddr);
> +
> + return folio;
> + }
> +#define vma_alloc_zeroed_movable_folio vma_alloc_zeroed_movable_folio
> +
> #define __HAVE_ARCH_COPY_USER_HIGHPAGE
>
> /*
> diff --git a/mm/internal.h b/mm/internal.h
> index cb8d8e8e3ffa..d513fa683aa3 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1287,7 +1287,8 @@ void touch_pmd(struct vm_area_struct *vma, unsigned long addr,
>
> static inline bool alloc_zeroed(void)
> {
> - return static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
> + return !IS_ENABLED(CONFIG_MIPS) &&
> + static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
> &init_on_alloc);
> }
After adding a missing static inline, #include <linux/gfp.h>, and still
getting compile failures, I gave up...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
