linux-kernel - Re: [linus:master] [mseal] 8be7258aad: stress-ng.pagemove.page_remaps_per

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALmYWFuCvphvLQOuQHBbFq0G8Ekyze=q45Tt4dATOt-GhO2RGg@mail.gmail.com>
Date: Mon, 5 Aug 2024 12:37:59 -0700
From: Jeff Xu <jeffxu@...gle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, Pedro Falcato <pedro.falcato@...il.com>, 
	kernel test robot <oliver.sang@...el.com>, Jeff Xu <jeffxu@...omium.org>, oe-lkp@...ts.linux.dev, 
	lkp@...el.com, linux-kernel@...r.kernel.org, 
	Andrew Morton <akpm@...ux-foundation.org>, Kees Cook <keescook@...omium.org>, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Dave Hansen <dave.hansen@...el.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Guenter Roeck <groeck@...omium.org>, 
	Jann Horn <jannh@...gle.com>, Jonathan Corbet <corbet@....net>, 
	Jorge Lucangeli Obes <jorgelo@...omium.org>, Matthew Wilcox <willy@...radead.org>, 
	Muhammad Usama Anjum <usama.anjum@...labora.com>, Stephen Röttger <sroettger@...gle.com>, 
	Suren Baghdasaryan <surenb@...gle.com>, Amer Al Shanawany <amer.shanawany@...il.com>, 
	Javier Carrasco <javier.carrasco.cruz@...il.com>, Shuah Khan <shuah@...nel.org>, 
	linux-api@...r.kernel.org, linux-mm@...ck.org, ying.huang@...el.com, 
	feng.tang@...el.com, fengwei.yin@...el.com
Subject: Re: [linus:master] [mseal] 8be7258aad: stress-ng.pagemove.page_remaps_per_sec
 -4.4% regression

On Mon, Aug 5, 2024 at 12:01 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Mon, 5 Aug 2024 at 11:11, Jeff Xu <jeffxu@...gle.com> wrote:
> >
> > One thing that you can't walk around is that can_modify_mm must be
> > called prior to arch_unmap, that means in-place check for the munmap
> > is not possible.
>
> Actually, we should move 'arch_unmap()'.
>
I think you meant "remove"

> There is only one user of it, and it's pretty pointless.
>
> (Ok, there are two users - x86 also has an 'arch_unmap()', but it's empty).
>
> The reason I say that the current user of arch_unmap() is pointless is
> because this is what the powerpc user does:
>
>   static inline void arch_unmap(struct mm_struct *mm,
>                                 unsigned long start, unsigned long end)
>   {
>         unsigned long vdso_base = (unsigned long)mm->context.vdso;
>
>         if (start <= vdso_base && vdso_base < end)
>                 mm->context.vdso = NULL;
>   }
>
> and that would make sense if we didn't have an actual 'vma' that
> matched the vdso. But we do.
>
> I think this code may predate the whole "create a vma for the vdso"
> code. Or maybe it was just always confused.
>
Agree it is best to remove.

> Anyway, what the code *should* do is that we should just have a
> ->close() function for special mappings, and call that in
> special_mapping_close().
>
I'm curious, why does ppc need to unmap vdso ? ( other archs don't
have unmap logic.)

vdso has .remap, iiuc, that is for CHECKPOINT_RESTORE feature, i.e.
during restore, vdso might get relocated after taking from dump. [1]
IIUC, vdso mapping doesn't change during the lifetime of the process.
Or does it in some user cases ?

[1] https://lore.kernel.org/linux-mm/20161101172214.2938-1-dsafonov@virtuozzo.com/


> This is an ENTIRELY UNTESTED patch that gets rid of this horrendous wart.
>
> Michael / Nick / Christophe? Note that I didn't even compile-test this
> on x86-64, much less on powerpc.
>
> So please consider this a "maybe something like this" patch, but that
> 'arch_unmap()' really is pretty nasty.
>
> Oh, and there was a bug in the error path of the powerpc vdso setup
> code anyway. The patch fixes that too, although considering the
> entirely untested nature of it, the "fixes" is laughably optimistic.
>
>                  Linus