Re: xcall while cold == 1
On 2017/12/27 1:37, Martin Husemann wrote: On Tue, Dec 26, 2017 at 07:05:38PM +0900, Masanobu SAITOH wrote: so checking mp_online is the best, right? I wonder if we should additionaly check for ncpu >= 2 (or ncpuonline >= 2), but that is probably overoptimization. I think doing optimization for single CPU machine with options MULTIPROCESSOR kernel is not so important. It would rather do optimize for non-MULTIPROCESSOR kernel. Martin -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: xcall while cold == 1
On Tue, Dec 26, 2017 at 07:05:38PM +0900, Masanobu SAITOH wrote: > > so checking mp_online is the best, right? I wonder if we should additionaly check for ncpu >= 2 (or ncpuonline >= 2), but that is probably overoptimization. Martin
Re: xcall while cold == 1
On 2017/12/26 11:59, Masanobu SAITOH wrote:
On 2017/12/25 20:26, Martin Husemann wrote:
On Mon, Dec 25, 2017 at 03:42:06PM +0900, Masanobu SAITOH wrote:
Is this intended behavior? Is it possible to use xcall while
cold==1?
Cold is (as you noted) not the right condition (but pretty close).
Xcalls don't really make any sense before cpus have been spun up.
In your case it might be good to do the loop checking for SPCF_RUNNING
and if <= 1 is found, use the code path for single cpu systems (the
current else statatement).
In init_main.c::configure2():
cold = 0; /* clocks are running, we're warm now! */
s = splsched();
curcpu()->ci_schedstate.spc_flags |= SPCF_RUNNING;
splx(s);
/* Boot the secondary processors. */
for (CPU_INFO_FOREACH(cii, ci)) {
uvm_cpu_attach(ci);
}
mp_online = true;
so checking mp_online is the best, right?
Martin
Updated diff:
-
Index: kern_softint.c
===
RCS file: /cvsroot/src/sys/kern/kern_softint.c,v
retrieving revision 1.44
diff -u -p -r1.44 kern_softint.c
--- kern_softint.c 22 Nov 2017 02:20:21 - 1.44
+++ kern_softint.c 26 Dec 2017 07:47:57 -
@@ -177,6 +177,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_softint
#include
#include
#include
+#include
#include
#include
#include
@@ -430,8 +431,10 @@ softint_disestablish(void *arg)
* it again. So, we are only looking for handler records with
* SOFTINT_ACTIVE already set.
*/
- where = xc_broadcast(0, (xcfunc_t)nullop, NULL, NULL);
- xc_wait(where);
+ if (__predict_true(mp_online)) {
+ where = xc_broadcast(0, (xcfunc_t)nullop, NULL, NULL);
+ xc_wait(where);
+ }
for (;;) {
/* Collect flag values from each CPU. */
Index: subr_pserialize.c
===
RCS file: /cvsroot/src/sys/kern/subr_pserialize.c,v
retrieving revision 1.9
diff -u -p -r1.9 subr_pserialize.c
--- subr_pserialize.c 21 Nov 2017 08:49:14 - 1.9
+++ subr_pserialize.c 26 Dec 2017 07:47:57 -
@@ -157,6 +157,11 @@ pserialize_perform(pserialize_t psz)
KASSERT(psz->psz_owner == NULL);
KASSERT(ncpu > 0);
+ if (__predict_false(mp_online == false)) {
+ psz_ev_excl.ev_count++;
+ return;
+ }
+
/*
* Set up the object and put it onto the queue. The lock
* activity here provides the necessary memory barrier to
Index: subr_psref.c
===
RCS file: /cvsroot/src/sys/kern/subr_psref.c,v
retrieving revision 1.9
diff -u -p -r1.9 subr_psref.c
--- subr_psref.c14 Dec 2017 05:45:55 - 1.9
+++ subr_psref.c26 Dec 2017 07:47:57 -
@@ -429,8 +429,14 @@ psreffed_p(struct psref_target *target,
.ret = false,
};
- /* Ask all CPUs to say whether they hold a psref to the target. */
- xc_wait(xc_broadcast(0, _p_xc, , NULL));
+ if (__predict_true(mp_online)) {
+ /*
+* Ask all CPUs to say whether they hold a psref to the
+* target.
+*/
+ xc_wait(xc_broadcast(0, _p_xc, , NULL));
+ } else
+ psreffed_p_xc(, NULL);
return P.ret;
}
-
It might not be required to increment psz_ev_excl evcnt in
softint_disestablish()
when mp_online == false.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: xcall while cold == 1
On 2017/12/25 20:26, Martin Husemann wrote:
On Mon, Dec 25, 2017 at 03:42:06PM +0900, Masanobu SAITOH wrote:
Is this intended behavior? Is it possible to use xcall while
cold==1?
Cold is (as you noted) not the right condition (but pretty close).
Xcalls don't really make any sense before cpus have been spun up.
In your case it might be good to do the loop checking for SPCF_RUNNING
and if <= 1 is found, use the code path for single cpu systems (the
current else statatement).
In init_main.c::configure2():
cold = 0; /* clocks are running, we're warm now! */
s = splsched();
curcpu()->ci_schedstate.spc_flags |= SPCF_RUNNING;
splx(s);
/* Boot the secondary processors. */
for (CPU_INFO_FOREACH(cii, ci)) {
uvm_cpu_attach(ci);
}
mp_online = true;
so checking mp_online is the best, right?
Martin
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
xcall while cold == 1
Hi.
While debugging PR#52820 ("boot -1" panics on systems with ixgX interfaces),
I've noticed that xcall doesn't work while cold == 1.
When I added softint_disestablish() near the end of the ixgbe_attach().
The following panic occured:
panic: kernel diagnostic assertion "xc->xc_donep < xc->xc_headp" failed: file
"../../../../kern/subr_xcall.c", line 278
This KASSERT is:
static inline uint64_t
xc_lowpri(xcfunc_t func, void *arg1, void *arg2, struct cpu_info *ci)
{
xc_state_t *xc = _low_pri;
CPU_INFO_ITERATOR cii;
uint64_t where;
mutex_enter(>xc_lock);
while (xc->xc_headp != xc->xc_donep) {
cv_wait(>xc_busy, >xc_lock);
}
xc->xc_arg1 = arg1;
xc->xc_arg2 = arg2;
xc->xc_func = func;
if (ci == NULL) {
xc_broadcast_ev.ev_count++;
for (CPU_INFO_FOREACH(cii, ci)) {
if ((ci->ci_schedstate.spc_flags & SPCF_RUNNING) == 0)
continue;
xc->xc_headp += 1;
ci->ci_data.cpu_xcall_pending = true;
cv_signal(>ci_data.cpu_xcall);
}
} else {
xc_unicast_ev.ev_count++;
xc->xc_headp += 1;
ci->ci_data.cpu_xcall_pending = true;
cv_signal(>ci_data.cpu_xcall);
}
KASSERT(xc->xc_donep < xc->xc_headp);<= Here!
where = xc->xc_headp;
mutex_exit(>xc_lock);
/* Return a low priority ticket. */
KASSERT((where & XC_PRI_BIT) == 0);
return where;
}
So I added the following debug printf:
-
@@ -252,6 +253,7 @@ xc_lowpri(xcfunc_t func, void *arg1, voi
xc_state_t *xc = _low_pri;
CPU_INFO_ITERATOR cii;
uint64_t where;
+ int i = -1;
mutex_enter(>xc_lock);
while (xc->xc_headp != xc->xc_donep) {
@@ -263,8 +265,11 @@ xc_lowpri(xcfunc_t func, void *arg1, voi
if (ci == NULL) {
xc_broadcast_ev.ev_count++;
for (CPU_INFO_FOREACH(cii, ci)) {
- if ((ci->ci_schedstate.spc_flags & SPCF_RUNNING) == 0)
+ i++;
+ if ((ci->ci_schedstate.spc_flags & SPCF_RUNNING) == 0) {
+ printf("cpu %d: XXX not running\n", i);
continue;
+ }
xc->xc_headp += 1;
ci->ci_data.cpu_xcall_pending = true;
cv_signal(>ci_data.cpu_xcall);
@@ -275,6 +280,9 @@ xc_lowpri(xcfunc_t func, void *arg1, voi
ci->ci_data.cpu_xcall_pending = true;
cv_signal(>ci_data.cpu_xcall);
}
+ if (xc->xc_donep >= xc->xc_headp)
+ printf("XXX donep = %" PRIu64 ", headp = %" PRIu64
+ ", ci = %p\n", xc->xc_donep, xc->xc_headp, ci);
KASSERT(xc->xc_donep < xc->xc_headp);
where = xc->xc_headp;
mutex_exit(>xc_lock);
-
The output says
cpu 0: XXX not running
cpu 1: XXX not running
cpu 2: XXX not running
cpu 3: XXX not running
XXX donep = 0, headp = 0, ci = 0x0
panic: kernel diagnostic assertion "xc->xc_donep < xc->xc_headp" failed: file
"../../../../kern/subr_xcall.c", line 286
(Yes, the exact reason is not cold==1 but all CPU's SPCF_RUNNING is not set)
Is this intended behavior? Is it possible to use xcall while
cold==1?
For softint_establish(), the following diff avoid panic:
Index: kern_softint.c
===
RCS file: /cvsroot/src/sys/kern/kern_softint.c,v
retrieving revision 1.44
diff -u -p -r1.44 kern_softint.c
--- kern_softint.c 22 Nov 2017 02:20:21 - 1.44
+++ kern_softint.c 25 Dec 2017 06:31:57 -
@@ -177,6 +177,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_softint
#include
#include
#include
+#include
#include
#include
#include
@@ -430,8 +431,10 @@ softint_disestablish(void *arg)
* it again. So, we are only looking for handler records with
* SOFTINT_ACTIVE already set.
*/
- where = xc_broadcast(0, (xcfunc_t)nullop, NULL, NULL);
- xc_wait(where);
+ if (__predict_true(cold == 0)) {
+ where = xc_broadcast(0, (xcfunc_t)nullop, NULL, NULL);
+ xc_wait(where);
+ }
for (;;) {
/* Collect flag values from each CPU. */
I don't know whether this is
