svn commit: r360233 - in head: contrib/jemalloc . . . : This partially breaks a 2-socket 32-bit powerpc (old PowerMac G4) based on head -r360311

[Mark Millard]

[I found a cause of the crash problem for the patched code,
or so I expect. More missing code.]

On 2020-Jun-16, at 20:23, Mark Millard <marklmi at yahoo.com> wrote:
> 
> On 2020-Jun-16, at 19:32, Justin Hibbits <chmeeedalf at gmail.com> wrote:
> 
>> (Removing hackers and current, too many cross-lists already, and those
>> interested in reading this are probably already on ppc@)
>> 
>> Mark,
>> 
>> Can you try this updated patch?  Again, I've only compiled it, I
>> haven't tested it, so it may also explode.  However, it more closely
>> mimics exactly what moea64 does.
> 
> Sure . . . But no luck.
> 
> Same crash, same backtrace related information,
> other than the "in page" figure and the "time"
> figure:
> 
> panic: vm_page_free_prep: mapping flags set in page 0xd0300fc0
> cpuid = 0
> time = 1592362496
> KDB: stack backtrace:
> 0xd2dc4340: at kdb_backtrace+0x64
> 0xd2dc43a0: at vpanic+0x208
> 0xd2dc4410: at panic+0x64
> 0xd2dc4450: at vm_page_free_prep+0x348
> 0xd2dc4470: at vm_page_free_toq+0x3c
> 0xd2dc4490: at vm_page_free+0x20
> 0xd2dc44a0: at vm_object_collapse+0x4ac
> 0xd2dc4510: at vm_object_deallocate+0x430
> 0xd2dc4550: at vm_map_process_deferred+0xec
> 0xd2dc4570: at vm_map_remove+0x12c
> 0xd2dc4590: at exec_new_vmspace+0x20c
> 0xd2dc45f0: at exec_elf32_imgact+0xa70
> 0xd2dc46a0: at kern_execve+0x600
> 0xd2dc4910: at sys_execve+0x84
> 0xd2dc4970: at trap+0x748
> 0xd2dc4a10: at powerpc_interrupt+0x178
> 0xd2dc4a40: user SC trap by 0x100d71f8: srr1=0xf032
>            r1=0xffffd810 cr=0x82000280 xer=0 ctr=0x10173810 frame=0xd2dc4a48
> KDB: enter: panic
> 
> /wrkdirs/usr/ports/devel/gdb/work-py37/gdb-9.1/gdb/inferior.c:283: internal-error: struct inferior *find_inferior_pid(int): Assertion `pid != 0' failed.
> 
> 
> FYI . . .
> 
> (m->a.flags & (PGA_EXECUTABLE | PGA_WRITEABLE)) == 0
> is failing when (m->oflags & VPO_UNMANAGED) == 0 holds in
> vm_page_free_prep. See the last KASSERT in the code
> quoted below. Does this suggest the lack of someplace
> not clearing some flags in m->a.flags that should be
> doing so?
> 
> static bool
> vm_page_free_prep(vm_page_t m)
> {
> 
>        /*
>         * Synchronize with threads that have dropped a reference to this
>         * page.
>         */
>        atomic_thread_fence_acq();
> 
> #if defined(DIAGNOSTIC) && defined(PHYS_TO_DMAP)
>        if (PMAP_HAS_DMAP && (m->flags & PG_ZERO) != 0) {
>                uint64_t *p;
>                int i;
>                p = (uint64_t *)PHYS_TO_DMAP(VM_PAGE_TO_PHYS(m));
>                for (i = 0; i < PAGE_SIZE / sizeof(uint64_t); i++, p++)
>                        KASSERT(*p == 0, ("vm_page_free_prep %p PG_ZERO %d %jx",
>                            m, i, (uintmax_t)*p));
>        }
> #endif
>        if ((m->oflags & VPO_UNMANAGED) == 0) {
>                KASSERT(!pmap_page_is_mapped(m),
>                    ("vm_page_free_prep: freeing mapped page %p", m));
>                KASSERT((m->a.flags & (PGA_EXECUTABLE | PGA_WRITEABLE)) == 0,
>                    ("vm_page_free_prep: mapping flags set in page %p", m));
>        } else {
> . . .
> 


From what I can tell there is another 32-bit vs. 64 bit difference
for the following code that involves 32-bit not using
vm_page_aflag_clear(???,PGA_WRITEABLE | PGA_EXECUTABLE) where
64-bit does involve such.

Starting a trace of the issue at exec_new_vmspace . . .

int
exec_new_vmspace(struct image_params *imgp, struct sysentvec *sv)
{
        . . .
        if (vmspace->vm_refcnt == 1 && vm_map_min(map) == sv_minuser &&
            vm_map_max(map) == sv->sv_maxuser &&
            cpu_exec_vmspace_reuse(p, map)) {
                shmexit(vmspace);
                pmap_remove_pages(vmspace_pmap(vmspace));
                vm_map_remove(map, vm_map_min(map), vm_map_max(map));
. . .

The "pmap_remove_pages(vmspace_pmap(vmspace))" before the
vm_map_remove use has a very different handling for
64-bit (does something) vs. 32-bit (no-op) . . .

moea64_remove_pages is (and eventually involves vm_page_aflag_clear):

void
moea64_remove_pages(mmu_t mmu, pmap_t pm)
{
        . . .
        while (!SLIST_EMPTY(&tofree)) {
                pvo = SLIST_FIRST(&tofree);
                SLIST_REMOVE_HEAD(&tofree, pvo_dlink);
                moea64_pvo_remove_from_page(mmu, pvo);
                free_pvo_entry(pvo);
        }
}

where moea64_pvo_remove_from_page involves
vm_page_aflag_clear(????,PGA_WRITEABLE | PGA_EXECUTABLE) via:

static inline void
moea64_pvo_remove_from_page_locked(mmu_t mmu, struct pvo_entry *pvo,
    vm_page_t m)
{
                
        . . . 
        /*
         * Update vm about page writeability/executability if managed
         */
        PV_LOCKASSERT(pvo->pvo_pte.pa & LPTE_RPGN);
        if (pvo->pvo_vaddr & PVO_MANAGED) {
                if (m != NULL) {
                        LIST_REMOVE(pvo, pvo_vlink);
                        if (LIST_EMPTY(vm_page_to_pvoh(m)))
                                vm_page_aflag_clear(m,
                                    PGA_WRITEABLE | PGA_EXECUTABLE);
                }
        }
        . . .
}

But 32-bit has/uses:

        static void mmu_null_remove_pages(mmu_t mmu, pmap_t pmap)
        {
                return;
        }


so it does not involve:

    vm_page_aflag_clear(????,PGA_WRITEABLE | PGA_EXECUTABLE)

but apparently should involve such in order to pass:

               KASSERT((m->a.flags & (PGA_EXECUTABLE | PGA_WRITEABLE)) == 0,
                   ("vm_page_free_prep: mapping flags set in page %p", m));

===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)