> Date: Sat, 25 May 2019 15:57:44 -0300
> From: Martin Pieuchot <m...@openbsd.org>
>
> On 12/05/19(Sun) 18:17, Martin Pieuchot wrote:
> > People started complaining that the SCHED_LOCK() is contended. Here's a
> > first round at reducing its scope.
> >
> > Diff below introduces a per-process mutex to protect time accounting
> > fields accessed in tuagg(). tuagg() is principally called in mi_switch()
> > where the SCHED_LOCK() is currently held. Moving these fields out of
> > its scope allows us to drop some SCHED_LOCK/UNLOCK dances in accounting
> > path.
> >
> > Note that hardclock(9) still increments p_{u,s,i}ticks without holding a
> > lock. I doubt it's worth doing anything so this diff doesn't change
> > anything in that regard.
>
> Updated diff:
>
> - Use struct assignment instead of timespecadd() to initialize
> `tu_runtime', pointed out by visa@.
> - Do not use mtx_enter(9)/leave(9) in libkvm's version of FILL_PROC(),
> pointed out by guenther@ and lteo@.
>
> Oks?
I fear that what was committed isn't quite right:
witness: lock order reversal:
1st 0xffff800022dce100 &pr->ps_mtx (&pr->ps_mtx)
2nd 0xffffffff81e8d4a0 &sched_lock (&sched_lock)
lock order "&sched_lock"(sched_lock) -> "&pr->ps_mtx"(mutex) first seen at:
#0 witness_checkorder+0x449
#1 mtx_enter+0x34
#2 tuagg+0x27
#3 mi_switch+0x10f
#4 sleep_finish+0x81
#5 tsleep+0xc7
#6 main+0x5c0
#7 longmode_hi+0x95
lock order "&pr->ps_mtx"(mutex) -> "&sched_lock"(sched_lock) first seen at:
#0 witness_checkorder+0x449
#1 __mp_lock+0x5f
#2 schedclock+0x69
#3 hardclock+0xe5
#4 lapic_clockintr+0x3f
#5 Xresume_lapic_ltimer+0x26
#6 sched_exit+0xc2
#7 exit1+0x563
#8 sys_exit+0x17
#9 syscall+0x2d5
#10 Xsyscall+0x128
I suspect this could be fixed by marking the mutex as IPL_SCHED? Or
is this particular one harmless because schedlock is special?
