Page MenuHomeFreeBSD

thread: stash domain id to work around vtophys problems on ppc64
ClosedPublic

Authored by mjg on Thu, Nov 19, 7:04 PM.

Details

Summary

Adding to zombie list can be perfomed by idle threads, which on ppc64 leads to the following panic:

panic: mtx_lock() by idle thread 0xc008000006437100 on sleep mutex kernelpmap @ /usr/home/luporl/git/master/sys/powerpc/aim/mmu_oea64.c:2237
cpuid = 31
time = 1605806644
KDB: stack backtrace:
0xc0080000014d91e0: at kdb_backtrace+0x60
0xc0080000014d92f0: at vpanic+0x1e0
0xc0080000014d93a0: at panic+0x40
0xc0080000014d93d0: at __mtx_lock_flags+0x23c
0xc0080000014d9480: at moea64_kextract+0x68
0xc0080000014d9560: at thread_stash+0x48
0xc0080000014d95a0: at mi_switch+0x1fc
0xc0080000014d9620: at critical_exit_preempt+0x88
0xc0080000014d9650: at cpu_idle+0xd4
0xc0080000014d96c0: at sched_idletd+0x440
0xc0080000014d9820: at fork_exit+0xc4
0xc0080000014d98c0: at fork_trampoline+0x18
0xc0080000014d98f0: at cpu_reset_handler+0x64

Work around the problem by saving the id on allocation time.

The field is added into a 4 byte hole, so there is no growth on LP64. I can make it conditional on NUMA to spare others.

Another option is to recognize domain ids would easily fit in char, add a typedef and stash the one byte in a smaller hole elsewhere.

Test Plan

Reported and tested by luporl.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mjg requested review of this revision.Thu, Nov 19, 7:04 PM

On such arches, for idle threads, you can add to the numa domain 0 regardless of actual domain. Or process stash directly, after all we are in idle thread and running.

Processing the list can take even more sleepable locks for example to free TIDs. At the end of it uma is also going to make the call in item_domain.

I don't see a good reason to penalize the platform or to create differences to the rest. If 32-bit struct thread growth is a problem that can easily by combated by either ifdefing on NUMA or making the field smaller -- there are many sub 4 byte holes in the struct.

markj added inline comments.
sys/sys/proc.h
246 ↗(On Diff #79778)

It's safe to use a u_char for the domain, so the sizeof the structure remains the same on all platforms.

This revision is now accepted and ready to land.Fri, Nov 20, 7:24 PM
  • stuff the domain in a 3 byte hole with u_char
This revision now requires review to proceed.Fri, Nov 20, 8:07 PM

This is how it looks like on i386 (ptype /o struct thread):

/*  148      |     1 */    u_char td_lend_user_pri;
/*  149      |     1 */    u_char td_allocdomain;
/* XXX  2-byte hole  */
/*  152      |     4 */    int td_flags;
kib added inline comments.
sys/sys/proc.h
249 ↗(On Diff #79816)

This comment should be improved. It needs to be absolutely clear that it is about struct thread memory, not any thread' affinity policits. Basically, it is of no use for anything except thread_reap() hack.

No need to wait for me after you update the comment.

This revision is now accepted and ready to land.Sat, Nov 21, 12:29 AM