Date: Mon, 11 Jan 2021 22:22:49 -0800 From: Ryan Libby <rlibby@freebsd.org> To: Mateusz Guzik <mjguzik@gmail.com>, Andrew Gallatin <gallatin@freebsd.org> Cc: src-committers <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: <CAHgpiFw2UBUj2Hp9FRTO_AEy422o1=D-N6ZZa5N_g_AFYvqwzA@mail.gmail.com> In-Reply-To: <CAGudoHEmr%2BcLf3VH6-osjGqQKKvtghcOQ3cbKHakV6tovSwfqg@mail.gmail.com> References: <202101120110.10C1A6mg069684@gitrepo.freebsd.org> <CAGudoHEmr%2BcLf3VH6-osjGqQKKvtghcOQ3cbKHakV6tovSwfqg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jan 11, 2021 at 9:59 PM Mateusz Guzik <mjguzik@gmail.com> wrote: > > This makes my 2 core vm crash on boot: There's some known broken logic in bitset(9) for comparisons and some other operations. I started some tests and a patch to fix it and clean up some stuff, but I haven't polished it off: https://github.com/rlibby/freebsd/commits/bitset Long story short, it isn't careful not to examine bits in the last word that are past the end of the set. > > 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?CAHgpiFw2UBUj2Hp9FRTO_AEy422o1=D-N6ZZa5N_g_AFYvqwzA>