Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 12 Jan 2021 06:59:28 +0100
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Andrew Gallatin <gallatin@freebsd.org>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org,  dev-commits-src-main@freebsd.org
Subject:   Re: git: 7eaea04a5bb1 - main - amd64: compare TLB shootdown target to all_cpus
Message-ID:  <CAGudoHEmr%2BcLf3VH6-osjGqQKKvtghcOQ3cbKHakV6tovSwfqg@mail.gmail.com>
In-Reply-To: <202101120110.10C1A6mg069684@gitrepo.freebsd.org>
References:  <202101120110.10C1A6mg069684@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This makes my 2 core vm crash on boot:

Launching APs: 1
Timecounter "TSC-low" frequency 1346899854 Hz quality 1000
panic: IPI scoreboard is zero, initiator 1 target 1
cpuid = 1
time = 1
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xffffffff80fb7600
vpanic() at vpanic+0x181/frame 0xffffffff80fb7650
panic() at panic+0x43/frame 0xffffffff80fb76b0
smp_targeted_tlb_shootdown() at smp_targeted_tlb_shootdown+0x434/frame
0xffffffff80fb7730
pmap_invalidate_range() at pmap_invalidate_range+0x16f/frame 0xffffffff80fb77a0
vm_thread_stack_create() at vm_thread_stack_create+0x45/frame 0xffffffff80fb78e0
kstack_import() at kstack_import+0x4e/frame 0xffffffff80fb7910
cache_alloc() at cache_alloc+0x3bf/frame 0xffffffff80fb7970
cache_alloc_retry() at cache_alloc_retry+0x25/frame 0xffffffff80fb79b0
vm_thread_new() at vm_thread_new+0x3d/frame 0xffffffff80fb79d0
thread_alloc() at thread_alloc+0x50/frame 0xffffffff80fb7a00
kthread_add() at kthread_add+0x42/frame 0xffffffff80fb7aa0
kproc_kthread_add() at kproc_kthread_add+0xaf/frame 0xffffffff80fb7bc0
intr_event_add_handler() at intr_event_add_handler+0x227/frame
0xffffffff80fb7c30
start_softintr() at start_softintr+0xea/frame 0xffffffff80fb7c60
mi_startup() at mi_startup+0xec/frame 0xffffffff80fb7cb0
btext() at btext+0x2c
KDB: enter: panic


On 1/12/21, Andrew Gallatin <gallatin@freebsd.org> wrote:
> The branch main has been updated by gallatin:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=7eaea04a5bb1dc86c43ce046311e1c1a042994d3
>
> commit 7eaea04a5bb1dc86c43ce046311e1c1a042994d3
> Author:     Andrew Gallatin <gallatin@FreeBSD.org>
> AuthorDate: 2021-01-12 01:03:37 +0000
> Commit:     Andrew Gallatin <gallatin@FreeBSD.org>
> CommitDate: 2021-01-12 01:09:32 +0000
>
>     amd64: compare TLB shootdown target to all_cpus
>
>     On amd64, the pmap code passes all_cpus to
>     smp_targeted_tlb_shootdown() when unmapping from the
>     kernel pmap.  This function has an optimized path to send IPIs
>     to all but itself, which it intends to do when the target
>     is all cpus.   However, we need to compare the target cpu mask
>     with all_cpus, rather than using CPU_ISFULLSET().  Comparing with
>     CPU_ISFULLSET() will only work when we have MAXCPU cpus active in
>     the system, otherwise, we'll be sending repeated IPIs, rather than
>     a single IPI to all CPUs but ourself.
>
>     Fixing this should reduce the time spent in native_lapic_ipi_wait()
>     as we will be sending ipis in parallel, rather than one-by-one.
>     This is confirmed by dtrace.
>
>     Reviewed by: alc, jhb, kib, markj
>     Sponsored by:   Netflix
>     Differential Revision:  https://reviews.freebsd.org/D28102
> ---
>  sys/amd64/amd64/mp_machdep.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sys/amd64/amd64/mp_machdep.c b/sys/amd64/amd64/mp_machdep.c
> index 63777014e151..794a11bf1276 100644
> --- a/sys/amd64/amd64/mp_machdep.c
> +++ b/sys/amd64/amd64/mp_machdep.c
> @@ -673,7 +673,7 @@ smp_targeted_tlb_shootdown(cpuset_t mask, pmap_t pmap,
> vm_offset_t addr1,
>  	/*
>  	 * Check for other cpus.  Return if none.
>  	 */
> -	if (CPU_ISFULLSET(&mask)) {
> +	if (!CPU_CMP(&mask, &all_cpus)) {
>  		if (mp_ncpus <= 1)
>  			goto local_cb;
>  	} else {
> @@ -719,7 +719,7 @@ smp_targeted_tlb_shootdown(cpuset_t mask, pmap_t pmap,
> vm_offset_t addr1,
>  	 * (zeroing slot) and reading from it below (wait for
>  	 * acknowledgment).
>  	 */
> -	if (CPU_ISFULLSET(&mask)) {
> +	if (!CPU_CMP(&mask, &all_cpus)) {
>  		ipi_all_but_self(IPI_INVLOP);
>  		other_cpus = all_cpus;
>  		CPU_CLR(PCPU_GET(cpuid), &other_cpus);
>


-- 
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEmr%2BcLf3VH6-osjGqQKKvtghcOQ3cbKHakV6tovSwfqg>