Re: svn commit: r312205 - in head/sys: kern net

[Hans Petter Selasky]

On 01/17/17 16:04, John Baldwin wrote:

Then it gives a much slower boot by

hanging for almost a minute waiting for USB or something.


Hi,

This might be related to a bug in the timer init which I've observed too.

Can you show output from:

vmstat -i

When the booting is slow?

--HPS
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r312205 - in head/sys: kern net

[John Baldwin]

On Tuesday, January 17, 2017 12:02:49 PM Bruce Evans wrote:
> On Mon, 16 Jan 2017, John Baldwin wrote:
> 
> > On Sunday, January 15, 2017 12:50:10 AM Sean Bruno wrote:
> >> ...
> >> Log:
> >>   Fix hangs in a uniprocessor configuration (qemu, virtualbox, real hw).
> >>
> >>   sys/net/iflib.c:
> >> Add ctx to filter_info and don't skpi interrupt early on unless we're 
> >> on an
> >> SMP system
> >
> > On an SMP system with EARLY_AP_STARTUP (default on x86) this code should
> > never see smp_started as false anyway, so these checks should likely just
> > be conditional on EARLY_AP_STARTUP (since that option will eventually go
> > away).
> 
> Ifdefs on EARLY_AP_STARTUP give strange results for the UP case (at least
> for SMP with 1 CPU).  First, the ifdef in sys/kernel.h moves SI_SUB_SMP
> from late to early, although the CPU initialization order has not changed
> when there is only 1 CPU.  Then everything that uses SI_SUB_SMP is
> reordered, and bugs in the new order show up even in the UP case.

Most of the things that used to use SI_SUB_SMP no longer do (there is now
a SI_SUB_LAST that many of those things use).

> sys/gtaskqueue.h uses a complicated combination of EARLY_AP_STARTUP and
> SI_SUB ifdefs to try to get the order right.  I think the last commit to
> it helps for the UP case but harms for the SMP case (both for the
> !EARLY_AP_STARTUP case).  Its code is simpler for the EARLY_AP_STARTUP
> case.  In the !EARLY_AP_STARTUP case, it has to split up the initialization
> to delay calculations depending on the number of CPUs until after SMP
> has started.  The last commit to it seems to have turned the split into
> nonsense by removing the delay.  But in the UP case, the split is not
> really necessary and removing the delay should help.
> 
> EARLY_AP_STARTUP works badly here.  It doesn't give a much faster startup
> for the early part of the boot.  Then it gives a much slower boot by
> hanging for almost a minute waiting for USB or something.  Starting all
> the CPUs early also unimproves debugging by interleaving the boot messages
> a bit.  I use colorized console messages (with the color indexed by the
> CPU).  This makes interleaved messages quite readable and gives interesting
> patterns when multiple CPUs are running.

The interleaving I am aware of is from USB which starts its own kthreads
during attach directly rather than using config_intrhook like other drivers
which do bus exploration later in boot (e.g. all the SCSI devices).  I tried
to describe this to Hans but will probably just come up with a patch to change
USB to use config_intrhook which might help.

-- 
John Baldwin
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r312205 - in head/sys: kern net

[Bruce Evans]

On Mon, 16 Jan 2017, John Baldwin wrote:


On Sunday, January 15, 2017 12:50:10 AM Sean Bruno wrote:

...
Log:
  Fix hangs in a uniprocessor configuration (qemu, virtualbox, real hw).

  sys/net/iflib.c:
Add ctx to filter_info and don't skpi interrupt early on unless we're on an
SMP system


On an SMP system with EARLY_AP_STARTUP (default on x86) this code should
never see smp_started as false anyway, so these checks should likely just
be conditional on EARLY_AP_STARTUP (since that option will eventually go
away).


Ifdefs on EARLY_AP_STARTUP give strange results for the UP case (at least
for SMP with 1 CPU).  First, the ifdef in sys/kernel.h moves SI_SUB_SMP
from late to early, although the CPU initialization order has not changed
when there is only 1 CPU.  Then everything that uses SI_SUB_SMP is
reordered, and bugs in the new order show up even in the UP case.

sys/gtaskqueue.h uses a complicated combination of EARLY_AP_STARTUP and
SI_SUB ifdefs to try to get the order right.  I think the last commit to
it helps for the UP case but harms for the SMP case (both for the
!EARLY_AP_STARTUP case).  Its code is simpler for the EARLY_AP_STARTUP
case.  In the !EARLY_AP_STARTUP case, it has to split up the initialization
to delay calculations depending on the number of CPUs until after SMP
has started.  The last commit to it seems to have turned the split into
nonsense by removing the delay.  But in the UP case, the split is not
really necessary and removing the delay should help.

EARLY_AP_STARTUP works badly here.  It doesn't give a much faster startup
for the early part of the boot.  Then it gives a much slower boot by
hanging for almost a minute waiting for USB or something.  Starting all
the CPUs early also unimproves debugging by interleaving the boot messages
a bit.  I use colorized console messages (with the color indexed by the
CPU).  This makes interleaved messages quite readable and gives interesting
patterns when multiple CPUs are running.

Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r312205 - in head/sys: kern net

[John Baldwin]

On Sunday, January 15, 2017 12:50:10 AM Sean Bruno wrote:
> Author: sbruno
> Date: Sun Jan 15 00:50:10 2017
> New Revision: 312205
> URL: https://svnweb.freebsd.org/changeset/base/312205
> 
> Log:
>   Fix hangs in a uniprocessor configuration (qemu, virtualbox, real hw).
>   
>   sys/net/iflib.c:
> Add ctx to filter_info and don't skpi interrupt early on unless we're on 
> an
> SMP system

On an SMP system with EARLY_AP_STARTUP (default on x86) this code should
never see smp_started as false anyway, so these checks should likely just
be conditional on EARLY_AP_STARTUP (since that option will eventually go
away).

-- 
John Baldwin
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r312205 - in head/sys: kern net

[Ivan Klymenko]

On Sun, 15 Jan 2017 00:50:10 + (UTC)
Sean Bruno  wrote:

> Log:
>   Fix hangs in a uniprocessor configuration (qemu, virtualbox, real
> hw).

Hello.

MFC to STABLE?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r312205 - in head/sys: kern net

[Sean Bruno]

Author: sbruno
Date: Sun Jan 15 00:50:10 2017
New Revision: 312205
URL: https://svnweb.freebsd.org/changeset/base/312205

Log:
  Fix hangs in a uniprocessor configuration (qemu, virtualbox, real hw).
  
  sys/net/iflib.c:
Add ctx to filter_info and don't skpi interrupt early on unless we're on an
SMP system
  
  sys/kern/subr_gtaskqueue.c:
Skip smp check if we're running UP
  
  Submitted by: Matt Macy 
  Reported by:  emaste bde

Modified:
  head/sys/kern/subr_gtaskqueue.c
  head/sys/net/iflib.c

Modified: head/sys/kern/subr_gtaskqueue.c
==
--- head/sys/kern/subr_gtaskqueue.c Sat Jan 14 23:24:50 2017
(r312204)
+++ head/sys/kern/subr_gtaskqueue.c Sun Jan 15 00:50:10 2017
(r312205)
@@ -647,7 +647,7 @@ taskqgroup_attach(struct taskqgroup *qgr
qgroup->tqg_queue[qid].tgc_cnt++;
LIST_INSERT_HEAD(>tqg_queue[qid].tgc_tasks, gtask, gt_list);
gtask->gt_taskqueue = qgroup->tqg_queue[qid].tgc_taskq;
-   if (irq != -1 && smp_started) {
+   if (irq != -1 && (smp_started || mp_ncpus == 1)) {
gtask->gt_cpu = qgroup->tqg_queue[qid].tgc_cpu;
CPU_ZERO();
CPU_SET(qgroup->tqg_queue[qid].tgc_cpu, );
@@ -697,7 +697,7 @@ taskqgroup_attach_cpu(struct taskqgroup 
gtask->gt_irq = irq;
gtask->gt_cpu = cpu;
mtx_lock(>tqg_lock);
-   if (smp_started) {
+   if (smp_started || mp_ncpus == 1) {
for (i = 0; i < qgroup->tqg_cnt; i++)
if (qgroup->tqg_queue[i].tgc_cpu == cpu) {
qid = i;
@@ -717,7 +717,7 @@ taskqgroup_attach_cpu(struct taskqgroup 
 
CPU_ZERO();
CPU_SET(cpu, );
-   if (irq != -1 && smp_started)
+   if (irq != -1 && (smp_started || mp_ncpus == 1))
intr_setaffinity(irq, );
return (0);
 }
@@ -731,7 +731,7 @@ taskqgroup_attach_cpu_deferred(struct ta
qid = -1;
irq = gtask->gt_irq;
cpu = gtask->gt_cpu;
-   MPASS(smp_started);
+   MPASS(smp_started || mp_ncpus == 1);
mtx_lock(>tqg_lock);
for (i = 0; i < qgroup->tqg_cnt; i++)
if (qgroup->tqg_queue[i].tgc_cpu == cpu) {
@@ -824,7 +824,7 @@ _taskqgroup_adjust(struct taskqgroup *qg
 
mtx_assert(>tqg_lock, MA_OWNED);
 
-   if (cnt < 1 || cnt * stride > mp_ncpus || !smp_started) {
+   if (cnt < 1 || cnt * stride > mp_ncpus || (!smp_started && (mp_ncpus != 
1))) {
printf("taskqgroup_adjust failed cnt: %d stride: %d mp_ncpus: 
%d smp_started: %d\n",
   cnt, stride, mp_ncpus, smp_started);
return (EINVAL);

Modified: head/sys/net/iflib.c
==
--- head/sys/net/iflib.cSat Jan 14 23:24:50 2017(r312204)
+++ head/sys/net/iflib.cSun Jan 15 00:50:10 2017(r312205)
@@ -133,10 +133,13 @@ typedef struct iflib_rxq *iflib_rxq_t;
 struct iflib_fl;
 typedef struct iflib_fl *iflib_fl_t;
 
+struct iflib_ctx;
+
 typedef struct iflib_filter_info {
driver_filter_t *ifi_filter;
void *ifi_filter_arg;
struct grouptask *ifi_task;
+   struct iflib_ctx *ifi_ctx;
 } *iflib_filter_info_t;
 
 struct iflib_ctx {
@@ -300,6 +303,8 @@ typedef struct iflib_sw_tx_desc_array {
 #defineIFC_MULTISEG0x04
 #defineIFC_DMAR0x08
 #defineIFC_SC_ALLOCATED0x10
+#defineIFC_INIT_DONE   0x20
+
 
 #define CSUM_OFFLOAD   (CSUM_IP_TSO|CSUM_IP6_TSO|CSUM_IP| \
 CSUM_IP_UDP|CSUM_IP_TCP|CSUM_IP_SCTP| \
@@ -1194,7 +1199,7 @@ iflib_fast_intr(void *arg)
iflib_filter_info_t info = arg;
struct grouptask *gtask = info->ifi_task;
 
-   if (!smp_started)
+   if (!smp_started && mp_ncpus > 1)
return (FILTER_HANDLED);
 
DBG_COUNTER_INC(fast_intrs);
@@ -3753,6 +3758,7 @@ iflib_device_register(device_t dev, void
 
if_setgetcounterfn(ctx->ifc_ifp, iflib_if_get_counter);
iflib_add_device_sysctl_post(ctx);
+   ctx->ifc_flags |= IFC_INIT_DONE;
return (0);
 fail_detach:
ether_ifdetach(ctx->ifc_ifp);
@@ -4471,6 +4477,7 @@ iflib_irq_alloc_generic(if_ctx_t ctx, if
info->ifi_filter = filter;
info->ifi_filter_arg = filter_arg;
info->ifi_task = gtask;
+   info->ifi_ctx = ctx;
 
err = _iflib_irq_alloc(ctx, irq, rid, iflib_fast_intr, NULL, info,  
name);
if (err != 0) {
@@ -4567,6 +4574,7 @@ iflib_legacy_setup(if_ctx_t ctx, driver_
info->ifi_filter = filter;
info->ifi_filter_arg = filter_arg;
info->ifi_task = gtask;
+   info->ifi_ctx = ctx;
 
/* We allocate a single interrupt resource */
if ((err = _iflib_irq_alloc(ctx, irq, tqrid, iflib_fast_intr, NULL, 
info, name))